docs: rewrite README, refresh godoc, redesign examples, and fix interceptor wrapper inheritance#75
Merged
Merged
Conversation
Make the README significantly tighter and pivot the example to a more realistic snippet (HTTP fetch → file write) that highlights data flow between steps via Input. Drop the Nested Workflows section to keep the landing-page focused on the 80% use case. Refresh godoc comments across all non-test files so they match actual runtime behavior. Notable corrections: - step.go: AfterStep doc said "called before Do"; corrected to "after Do". - step.go: Retry example used opt.MaxAttempts; the real field is Attempts. - workflow.go: expand the worker / tick / interceptor inheritance docs with the rationale already captured in code comments. - condition.go: clarify that Skipped/Canceled are settled inline by the scheduler and never enter the interceptor chain. - interceptor.go, retry.go, error.go, wrap.go, branch.go, state.go, build_step.go, mock.go, func.go, noop.go, name.go: fill in missing doc comments and tighten existing ones. Deprecate SubWorkflow in favor of embedding *Workflow directly. No code behavior changes; verified with go build and go vet. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
executeWithRetry previously located the InterceptorReceiver on the running step via a direct type assertion. That silently breaks parent → child interceptor inheritance whenever the sub-workflow is wrapped in a Steper-only wrapper such as flow.Name / NamedStep — the embedded Steper interface does not promote PrependInterceptors, so the assertion fails and the parent's chain is never installed on the child. Walk the Step tree via Unwrap (the same protocol used by As[T] / Has[T]) and use the first InterceptorReceiver found in pre-order. This is a pure bug-fix: existing inheritance scenarios are unchanged; previously broken wrapper scenarios now work. Update the step-interceptor spec to document the resolution rule and add a scenario covering NamedStep inheritance. Add a regression test TestWorkflow_AsStep_InheritsThroughNamedStep that asserts the parent's interceptor fires for both the wrapping NamedStep and the inner step. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add example/14_interceptor_test.go with three runnable godoc examples:
- ExampleStepInterceptor — minimal logger wrapping each Step's full
lifetime (across retries).
- ExampleAttemptInterceptor — per-attempt observation alongside Retry,
making the two-layer model concrete.
- ExampleInterceptorReceiver — parent → child inheritance and opt-out
via IsolateInterceptors. Uses flow.Name
to label sub-workflows; works thanks to
the Unwrap-chain receiver lookup.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- 03_io_data_flow_test.go: use a typed ctxKey instead of a bare string for context.WithValue (vet-clean and a better Go example). - 05_branch_if_switch_test.go: discard error return from w.Do via '_ = w.Do(...)' for consistency with the rest of the suite. - 07_timeout_test.go: explain why testTimer.Start ignores the requested duration. - 08_workflow_option_test.go: fix stale doc — the field is SkipAsError, not OKToSkip. - 12_composite_step_test.go: drop ExampleCompositeViaWorkflow; it relied on the BuildStep / Reset implicit machinery that is being removed in a separate branch, and the surviving ExampleCompositeStep already covers the composite-vs-workflow story. - Renumber 10..14 → 09..13 to close the gap left by the removal of an earlier example file. The new interceptor example lands at 14. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace the 14-file API-clinic layout with a 12-file path organised by the
question a reader brings: get the mental model, move data, decide what
runs, build bigger workflows, then operate / debug / test.
01_quickstart — end-to-end 3-minute tour (parallel fetch + merge)
02_steps_and_deps — Step / Steps / DependsOn / Pipe / BatchPipe / Func / Name
03_data_flow — typed Input / Output, Func / FuncIO / FuncI / FuncO
04_callbacks — BeforeStep / AfterStep, where they sit in the stack
05_conditions — Condition / When / Skip / Cancel from inside Do
06_branching — If / Switch with runtime data
07_retry_and_timeout — Retry, per-attempt and per-step timeouts (mock clock)
08_workflow_in_workflow — sub-workflow as a Step + the composite-Step antipattern
09_workflow_options — MaxConcurrency, DontPanic
10_observability — Step / Attempt interceptors (renamed from 14_interceptor)
11_debugging — ErrWorkflow + Workflow.StateOf
12_testing_workflows — flow.Mock (with both an Example and a real Test)
Each file follows a consistent template: top-of-file doc explains
"What you'll learn" + mental model + when to reach for it; body has
small focused Example* functions. Add example/README.md as a navigation
landing page.
What got dropped:
- The four ExampleWorkflow_* sub-cases in old 08 (DefaultOption,
SkipAsError) — moved out of the path; their behaviour is documented on
the Workflow type's godoc.
- The Wrap / MultiWrap / Update demonstrations in old 09 — those were
really godoc material for Steper / Unwrap, not reader-path examples.
- The dedicated CompositeStep example in old 12 — folded into 08 as a
deliberate antipattern alongside the recommended sub-workflow form.
Verified with `go test -race -count=5 ./example/...` and `go vet ./...`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…pters Reframe 01_quickstart and 03_data_flow to put the central pitch up front: anything with a Do(context.Context) error method is a Step. There's no interface to embed, no generics, no decorators — your business types ARE the workflow. 01_quickstart now defines FetchUser, FetchPosts, and BuildProfile as plain structs. BuildProfile holds *FetchUser and *FetchPosts pointers directly, so data flows through fields rather than callbacks. 02_steps_and_deps switches to a tiny shared `stage` struct (vs the previous flow.Func) so the wiring chapters still focus on dependency shapes while staying honest about how Steps are defined. 03_data_flow now leads with the recommended struct-with-pointer-fields pattern (ExampleSteper_directFields) and demotes Input / Output callbacks to a clearly-marked alternative (ExampleAddStep_Input) for runtime-wired or generic helpers. 04_callbacks switches its example Steps from flow.Func to small structs (greeter, lookupItem) so the BeforeStep / AfterStep mechanics aren't mixed up with "what is a Step". example/README.md updated: add a Conventions note explaining that struct-based Steps are the production form and Func variants are convenience helpers used from 05 onward when the focus is something other than the Step body. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
In 01_quickstart, BuildProfile now exposes Name and Posts as plain
input fields and uses Step(...).Input(...) to copy upstream outputs
into them, rather than holding *FetchUser / *FetchPosts pointers and
reaching into upstream objects from Do.
This is the more idiomatic pattern: each Step is self-contained
(testable in isolation by setting input fields and calling Do
directly), and the data wiring is declared next to DependsOn so the
dependency and the data flow live in one place.
In 03_data_flow:
- ExampleAddStep_Input is now the leading example: plain user-defined
structs (fetchFeed / countItems / announceCount) with input/output
fields, wired with Input callbacks.
- ExampleFunction_inputOutput follows it as the convenience variant
using flow.Func / FuncIO / FuncI / FuncO when you don't want to
declare a struct per Step. The mechanics are identical.
example/README.md updated to match the new framing.
Verified with `go test -race -count=3 ./example/...` and `go vet ./...`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The earlier deprecation rationale ("SubWorkflow is unnecessary; embed
*Workflow directly") was wrong. The two patterns are NOT equivalent:
- Embedding `flow.SubWorkflow` keeps the inner Workflow unexported (`w
Workflow`, value type) and exposes a tight surface (Add / Do / Reset /
PrependInterceptors). The zero value works.
- Embedding `*flow.Workflow` promotes every public field of Workflow
(MaxConcurrency, DontPanic, SkipAsError, Clock, …) onto the outer
struct. It also requires the embedded pointer to be initialised
before use (typically inside BuildStep), so the zero value would
panic.
Both are valid; pick by trade-off. Drop the Deprecated marker so the
godoc no longer pushes users toward a strictly less-encapsulated
alternative; keep the example template intact.
Co-Authored-By: Claude Opus 4.7 (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
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
This PR bundles three threads of documentation work plus one related bug fix.
1. Documentation cleanup
2. Interceptor wrapper-inheritance fix
3. Example suite redesign
Test plan