Skip to content

feat(fluent): add Stream::filter_map, the Option-shaped map_filter - #873

Merged
0-jake-0 merged 1 commit into
mainfrom
stream-filter-map
Aug 23, 2026
Merged

feat(fluent): add Stream::filter_map, the Option-shaped map_filter#873
0-jake-0 merged 1 commit into
mainfrom
stream-filter-map

Conversation

@0-jake-0

@0-jake-0 0-jake-0 commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

What this changes

StreamOps gains filter_map(f) taking Fn(&T) -> Option<B>: tick the returned Some, stay quiet on None.

#[must_use = "a dropped stream stays wired and cycles every tick, producing an unread value"]
fn filter_map<B, F>(&self, f: F) -> Stream<B>
where
    B: Clone + Default + 'static,
    F: Fn(&T) -> Option<B> + 'static;

It is fluent sugar over the existing MapFilter op — the same single node, differing only in how the call site spells the emit decision — not a new catalog entry. map_filter is untouched: neither replaced nor deprecated.

Why — and the premise has changed since the issue was filed

Read this part before the rest. #831 argued a parity gap: Signal::filter_map existed, Stream had no twin. #887 then deleted the Signal facade outright (deviation D29), so that premise is gone — nothing in the tree has an Option-shaped filter-map any more, and this PR no longer closes a gap between two surfaces. The Signal::filter_map delegation that was part of the original branch has been dropped in the rebase; it had nowhere to land.

The change is now a fresh API addition, and it stands on a narrower but still real argument:

map_filter's (value, emit?) convention inverts Iterator::filter_map, which is the shape every Rust programmer already reaches for. Under the current API a Stream user has three options and all three are worse than the one they expect:

  • discover map_filter and fabricate a discarded B on the don't-emit branch ((Default::default(), false)) — a value the engine never reads, written only to satisfy the tuple;
  • chain two nodes via .map(f).filter_none(), paying a node per cycle for a spelling;
  • or reach for filter_map by name, find it resolves on Stream… nowhere, because it didn't exist.

The third is the tell: filter_none already exists for the already-Option case, which means the Option convention is one this API teaches — it just had no entry point when the Option comes out of a closure.

Why sugar and not an op. The behaviour is the same single MapFilter node, so promoting it would add a catalog entry that removes no node from any graph. The bound that looks like the sugar's cost isn't one: a dedicated FilterMap op could not have shed B: Default, because the #[op] forwarders add Out: Default to every op for value-slot seeding (wingfoil-derive/src/lib.rs). A new op would have bought nothing and cost a catalog entry.

If you don't buy that argument, this is the PR to close rather than the one to trim. With the parity premise gone the whole change is an ergonomics call, and the reviewer's call — not something to land on momentum. The counter-case is legitimate: it is a second spelling of an existing node, and the catalog is smaller for not having it.

Naming, under the #884 ruling

#884 ruled the fallible-twin convention: try_ + the exact base name, so map_filtertry_map_filter. It renamed try_filter_map specifically to avoid a false infallible/fallible pair with an Option-shaped filter_map. That ruling anticipated exactly this method, and the shapes land correctly beside each other:

method closure pairs with
map_filter Fn(&T) -> (B, bool) try_map_filter
try_map_filter Fn(&T) -> Result<(B, bool)> map_filter
filter_map (this PR) Fn(&T) -> Option<B> — deliberately no try_ twin

filter_map and try_map_filter are neighbours, not a pair — different base names, so the trap #884 removed does not come back. The rustdoc on the new method now says so explicitly, replacing the disambiguating note #884 put on Signal::filter_map (which died with the facade). enumerate (#892) and pairwise (#879) were checked for overlap: neither touches this shape.

What the rebase onto main changed

Rebased from 932107e onto f794b07. Two conflicts, both deliberate resolutions rather than mechanical ones:

  • signal.rs (modify/delete) — took main's deletion. The Signal::filter_map delegation is gone with it. A tests/signal.rs cross-reference in one of the new test doc comments was dropped for the same reason (that file no longer exists).
  • fluent.rs — kept both sides: try_map_filter (refactor(ops)!: rename try_filter_map to try_map_filter #884) and filter_map sit adjacent in the trait and the impl, and filter_none keeps main's #[must_use] alongside this branch's added doc line.

Picked up from what landed underneath:

  • #[must_use] (Add #[must_use] to the transform and source combinators #871) with that PR's message verbatim — it was already on this branch — and now pinned in tests/trybuild/must_use_combinators.rs + .stderr, which is what /new-op step 4c asks for. The fixture confirms the attribute fires at the call site, i.e. it is on the hand-written trait declaration and not lost in an impl.
  • /new-op step 1b lost its reference to the deleted step 4b and to Signal delegation; it now points at step 4c.

How it was verified

Toolchain: 1.98.0, which is what dtolnay/rust-toolchain@stable resolves to today — so the .stderr fixtures were generated and checked against the same rustc CI will use. fluent_only_sugar.stderr needed no regeneration after the rebase; must_use_combinators.stderr was regenerated (TRYBUILD=overwrite) for the added case and the line shifts it causes below it.

  • cargo fmt --all -- --check
  • cargo lint and cargo lint-all
  • cargo test -p wingfoil --all-features --no-fail-fast
  • cargo test --doc -p wingfoil --all-features
  • cargo test -p wingfoil-derive
  • scripts/check-example-docs.sh

Green except the nine *_integration suites (aeron, etcd, fluvio, kafka, otlp, postgres, redis, zmq_cross_lang, zmq_etcd) that fail with failed to initialize a docker client: Socket not found: /var/run/docker.sock — no Docker in that environment, pre-existing and unrelated.

Three tests in tests/catalog_flow.rs, all RunMode::HistoricalFrom(NanoTime::ZERO):

  • filter_map_emits_some_and_stays_quiet_on_nonewith_time().accumulate() pins values and stamps: a 10ns ticker counts 1..=4 at t = 0/10/20/30, only the odd counts survive, and the two even instants being absent is the filtering half of the contract.
  • filter_map_all_none_never_ticks_downstream — an all-None closure with a counting for_each behind it: source ticks four times, sink fires zero. The "a filtered-out tick does not tick downstream" case stated directly rather than inferred from a gap in a Vec.
  • filter_map_matches_map_filter — sugar and primitive wired side by side in one graph, asserted equal on values and stamps, so the two cannot drift.

Plus a doc fence on the method and the two trybuild cases.

Notes for the reviewer

nitro! diagnostics are the non-obvious cost of sugar, and they are here. Sugar composes out of what already exists and needs no forwarder — but that is true of the emission, not of the diagnostics. A fluent method with no op behind it has no __wf_op_* forwarders yet still resolves on Stream, so inside a nitro! block the expansion dies on leaked __WF_OP_FILTER_MAP_… internals carrying nonsense "a constant with a similar name exists" suggestions, with no no method named error pointing at the real problem. filter_none, collapse_accumulate and split each have an arm in non_op_method_advice (wingfoil-derive) that turns that into one message naming the primitive to spell instead; filter_map now has one too, pinned by a case in tests/trybuild/fluent_only_sugar.rs and recorded in the fluent-only allowlist in tests/op_completeness.rs (category 2).

Worth flagging: filter_map is the first of that set on StreamOps rather than on a specialised receiver (Stream<Option<T>>, Stream<Burst<T>>). The other two are out of reach of most nitro! blocks by their receiver type; this one is in reach of all of them, so the arm is not optional for it.

Python is unaffected. PyStream already exposes filter_map (wingfoil-python/src/graph.rs, python.rs), so this closes a Rust-side gap Python never had. No binding changes.

Living-document update. Per CLAUDE.md, the decision this turned on — sugar over an existing op vs. a new catalog op — was not written down anywhere, which is why the issue had to argue it from first principles. It is now step 1b of /new-op and a matching section in docs/adding-an-op.md: the node-count test that answers it (sugar wires exactly the node the primitive would; a two-node "sugar" method is a promotion candidate — the argument that promoted not, collapse, count, accumulate and merge_all), the warning not to let a trait bound argue for a new op when the forwarders impose it anyway, and the three macro-crate touch-points sugar owes — including that adding a trybuild block shifts the line numbers of the existing cases in the shared .stderr.

Unrelated gap noticed while rebasing, not fixed here. take_while (#886) and enumerate (#892) landed on StreamOps without #[must_use], against fluent.rs's own module doc ("Every transform and source declaration here therefore carries #[must_use]") and /new-op step 4c. must_use_combinators.rs does not catch it because it is a representative sample, not an exhaustive one. Deliberately left alone so it does not ride along in this PR — worth its own issue.

Closes

Closes #831but see "Why" above: the issue's parity framing no longer describes the tree, and the issue text should be read as superseded by this PR's argument rather than as the case for merging.

`StreamOps` gains `filter_map(f)` taking `Fn(&T) -> Option<B>`: tick the
returned `Some`, stay quiet on `None`. It is fluent sugar over the existing
`MapFilter` op — the same single node, differing only in how the call site
spells the emit decision — so it is not a catalog entry.

`map_filter`'s `(value, emit?)` convention inverts `Iterator::filter_map`,
which is the shape every Rust programmer already knows. Without this, a
Stream user either fabricates a discarded `B` on the don't-emit branch or
chains two nodes via `.map(f).filter_none()`.

`B: Default` is not a cost of the sugar: the `#[op]` forwarders add
`Out: Default` to every op for value-slot seeding, so a dedicated `FilterMap`
op could not have shed it either.

Sugar is not free in the macro crate. A fluent method with no op behind it
has no `__wf_op_*` forwarders but still resolves on `Stream`, so inside a
`nitro!` block the expansion dies on leaked `__WF_OP_FILTER_MAP_*` internals
with no `no method named` error to explain them. `filter_map` therefore gets
an arm in `non_op_method_advice` naming the primitive to spell instead — and
it is the first of that set on `StreamOps` rather than a specialised receiver,
so unlike `filter_none` / `collapse_accumulate` it is in reach of every
`nitro!` block.

- `#[must_use]` with the #871 message, pinned in
  `tests/trybuild/must_use_combinators.rs` + `.stderr`
- rejected by name in `nitro!`, pinned in `tests/trybuild/fluent_only_sugar.rs`
  + `.stderr`, and recorded in the `tests/op_completeness.rs` allowlist
- three tests in `tests/catalog_flow.rs` asserting values *and* tick times:
  `Some`/`None` behaviour, an all-`None` closure never ticking its sink, and
  the sugar tied out against `map_filter` side by side in one graph
- rustdoc notes that `try_map_filter` is the `try_` twin of `map_filter`, not
  of this spelling — the distinction #884 ruled on
- `/new-op` step 1b and a matching section in `docs/adding-an-op.md`: the
  node-count test for sugar vs. a catalog op, the warning not to let a trait
  bound argue for an op the forwarders would bound anyway, and the three
  macro-crate touch-points sugar owes
@0-jake-0
0-jake-0 merged commit c3f7060 into main Aug 23, 2026
7 checks passed
@0-jake-0
0-jake-0 deleted the stream-filter-map branch August 23, 2026 10:06
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.

Stream lacks filter_map — Signal already has it (API parity gap)

2 participants