Skip to content

refactor[next]: replace workflow combinators with explicit typed pipelines - #2743

Open
egparedes wants to merge 1 commit into
otf-split-3-observabilityfrom
otf-split-4-pipeline
Open

refactor[next]: replace workflow combinators with explicit typed pipelines#2743
egparedes wants to merge 1 commit into
otf-split-3-observabilityfrom
otf-split-4-pipeline

Conversation

@egparedes

@egparedes egparedes commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Description

Final PR of the otf-toolchain-split stack (on top of #2742). It implements the "Pipeline, not combinators" decision recorded in ADR 0027, and lands the OTFCompileWorkflowCompilePipeline rename that ADR deferred to this point.

The combinator framework from ADR 0011 had grown to a dozen abstractions to express function composition. Only CachedStep was a deep module; the rest were shallow wrappers over Callable[[S], T], and NamedStepSequence.__call__'s reflection loop was Any-typed — defeating the static typing ADR 0011 was after in the first place. Net: +500 / −887 lines.

Pipelines are dataclasses with an explicit __call__

  • backend.Transforms keeps its input-dependent step selection: the match that lived in step_order now lives in __call__, where the order is literally readable top to bottom. The step_order method is deleted.
  • backend.CompilePipeline (ex recipes.OTFCompileWorkflow) spells out translation → bindings → compilation.
  • Both take over emitting stage_hook from refactor[next]: add stage observability and Toolchain.translate #2742. This is the part worth reviewing closely: names, order, count and artifact values are unchanged, and the two instrumentation tests that assert the exact stage sequences (test_hooks.py::test_stage_hook and test_stage_dump.py's EXPECTED_STAGES) pass unmodified — that is the identity proof.

Steps are plain callables

workflow.Step[S, T] is just Callable[[S], T]. Customization stays composition-time via dataclasses.replace, per ADR 0011's own rule. Deleted: Workflow, both mixins, NamedStepSequence, MultiWorkflow, StepSequence, make_step, .chain, the three otf.toolchain adapters, and the five adapted_*_factory wrappers that existed only to wrap a function into an adapter. CachedStep's body is character-for-character unchanged — it loses only the mixin bases (and with them .replace/.chain, see breaking changes) and gets three retyped annotations. otf.recipes and otf.toolchain are deleted, with no re-exports left behind.

Because steps no longer have to be adapter objects, the seven ffront factories collapse to returning either the bare function or a CachedStep around it, and the three per-step callers in decorator.py lose their wrap/unwrap dance.

What ADR 0011's requirements become

ADR 0011 requirement Kept by
Named steps, order visible dataclass fields + explicit __call__ (order literally visible)
Statically typed composition explicit __call__, checked end-to-end — unlike the Any-typed reflection loop
Customization at composition, not via flags dataclasses.replace on frozen pipelines
Steps compose across backends Step[S, T] is a Callable; every existing step already satisfies it
Linear workflows unchanged (Transforms keeps its input-dependent selection)

Cache keys: nothing rotates

Unlike the naming PR, no persistent key changes. Verified empirically rather than assumed: fingerprinting tags a dataclass with its fully-qualified name plus its fields and never touches __bases__/__mro__ (two otherwise-identical classes, one with Protocol bases, fingerprint equal), and neither CompilePipeline nor Transforms is reachable from a persistent cache's fingerprint graph — the persistent CachedStep wraps the bare translator directly. Some in-memory step fingerprints move (unwrapped steps), which is invisible: those dicts are per-process and never persisted.

Breaking changes for downstream

Please read this list rather than skimming it — the first two are the ones most likely to bite.

  • Custom Transforms steps change signature. func_to_foast, func_to_past, past_lint and foast_to_itir go from pair-in/pair-out to bare-stage-in/bare-stage-out. This matters because dataclasses.replace(DEFAULT_TRANSFORMS, past_lint=…) is the sanctioned customization mechanism and HackTheToolchain.md shipped exactly that recipe, so a step written against the current release fails with AttributeError: 'PASTProgramDef' object has no attribute 'definition' on first call.
  • .replace() and .chain() disappear from classes this PR otherwise leaves aloneCachedStep, GTFNTranslationStep, CPPCompiler, DaCeTranslator, DaCeCompiler, and both pipelines. They came from the deleted mixins. Note ADR 0012 documents workflow.replace(run_gtfn.workflow, <name>=<instance>) as the substep-configuration idiom, so this is a live pattern; the replacement is dataclasses.replace.
  • Subclassing any deleted combinator. Overriding Transforms.step_order now raises TypeError — the method is retained purely to fail loudly, since __call__ no longer consults it and a silent override would have quietly re-enabled skipped steps.
  • stages.TranslationStep / stages.CompilationStep change from Protocol classes to Callable type aliases, so they can no longer be subclassed or used with isinstance.
  • Workflow is gone — annotate with Step[S, T]. It was a Protocol, so isinstance/subclass uses break loudly, which is intended.
  • gt4py.next.otf.recipes and gt4py.next.otf.toolchain no longer exist: OTFCompileWorkflow is now gt4py.next.backend.CompilePipeline, and the pair type lives in gt4py.next.otf.workflow.
  • roundtrip.foast_to_gtir_step silently changes signature (pair → bare FOASTOperatorDef). No in-repo users.
  • linter_factory(adapter=...) raises TypeError; the parameter was accepted and ignored before.
  • ItirShim.foast_to_itir is now data-only; anyone constructing an ItirShim with a pair-typed step breaks.

Docs

Both executed notebooks are rewritten. WorkflowPatterns.md was largely a tour of the deleted framework; it now covers Step, CachedStep, dataclasses.replace, and observing stages via stage_hook / GT4PY_DUMP_STAGES. HackTheToolchain.md's "skip a step" recipe becomes a dataclasses.replace with an identity step instead of a step_order override. Three cells in these notebooks referenced names that no longer exist (DEFAULT_PROG_TRANSFORMS, LinterFactory, .steps.inner[0]) and only "passed" CI because IPython's ?? reports a lookup failure without raising — those are fixed, but note nbmake does not guard against that class of regression.

Requirements

  • All fixes and/or new features come with corresponding tests. (Combinator tests deleted with their subjects; new Transforms/CompilePipeline tests assert step order, hook emission, passthrough, dataclasses.replace customization, and error ordering. The two stage-sequence oracles are unmodified.)
  • Important design decisions have been documented in the appropriate ADR inside the docs/development/ADRs/ folder. (ADR 0027; its phasing paragraph is updated to past tense now that the whole stack has landed.)

https://claude.ai/code/session_01R8zRtFMhdJ8c96XJYCXkRk

@egparedes
egparedes marked this pull request as ready for review July 30, 2026 17:58
@egparedes
egparedes force-pushed the otf-split-4-pipeline branch from 55f9fcb to 706dcba Compare July 30, 2026 17:58
…lines

The workflow-combinator framework introduced by ADR 0011 had grown to a dozen
abstractions to express what is, in the end, function composition. Measured
against actual use, only `CachedStep` was a deep module; the rest were shallow
wrappers around `Callable[[S], T]`, and the reflection loop in
`NamedStepSequence.__call__` was `Any`-typed, defeating the static typing
ADR 0011 prized.

The named pipelines become plain frozen dataclasses with an explicit, fully
typed `__call__`:

- `backend.Transforms` keeps its input-dependent step *selection* -- the `match`
  that used to live in `step_order` now lives in `__call__`, where the order is
  literally readable -- and the `step_order` method is retained only to raise,
  so a downstream override fails loudly instead of being silently ignored.
- `recipes.OTFCompileWorkflow` becomes `backend.CompilePipeline` and spells out
  its three steps; `otf.recipes` and `otf.toolchain` are deleted.
- Both take over emitting the `stage_hook` added in the previous PR. Names,
  order, count and artifacts are unchanged; the two instrumentation tests that
  assert the exact stage sequences pass unmodified, which is the proof.

Steps are now plain callables, named by the `workflow.Step[S, T]` alias, and
customization stays composition-time via `dataclasses.replace`. Deleted:
`Workflow`, `ChainableWorkflowMixin`, `ReplaceEnabledWorkflowMixin`,
`NamedStepSequence`, `MultiWorkflow`, `StepSequence`, `make_step`, `.chain`,
the three adapters in `otf.toolchain`, and the five `adapted_*_factory`
wrappers whose only job was to wrap a function into an adapter. `CachedStep`'s
body is unchanged; it loses only the mixin bases, and with them `.replace` and
`.chain`.

Because steps no longer need to be adapter objects, the seven ffront factories
collapse to returning either the bare function or a `CachedStep` around it, and
the three per-step callers in `decorator.py` lose their wrap/unwrap dance.

What ADR 0011's decisions become: named steps with a visible order are now
dataclass fields plus an explicit `__call__`; statically typed composition is
checked end-to-end instead of through an `Any`-typed reflection loop;
customization at composition time is `dataclasses.replace`; and steps still
compose across backends because every existing step already satisfies
`Step[S, T]`.

No behavior change, and -- unlike the naming PR -- no persistent cache key
rotates: fingerprints embed a class's qualified name and fields but never its
bases, and neither renamed pipeline is reachable from a persistent cache's
fingerprint graph.

Breaking, with no compatibility aliases: the deleted combinators and the
`otf.recipes` / `otf.toolchain` modules, `.replace()` / `.chain()` on the
classes that kept them via the mixins, overriding `Transforms.step_order`,
`roundtrip.foast_to_gtir_step` (now a data-only step), and
`linter_factory(adapter=...)` (the parameter was accepted and ignored).

Claude-Session: https://claude.ai/code/session_01R8zRtFMhdJ8c96XJYCXkRk
@egparedes
egparedes force-pushed the otf-split-4-pipeline branch from 706dcba to 605676d Compare July 31, 2026 16:15
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.

1 participant