diff --git a/README.md b/README.md index aac00ae..608c6f5 100644 --- a/README.md +++ b/README.md @@ -575,15 +575,24 @@ Run the bundled example directly after building: ## Roadmap -Planned improvements, not yet implemented: - -- **Track which stage short-circuited.** When a graph drops a message, neither - `DslFilterGraph` nor `AnyFilterChain` currently tells the caller *which* - stage returned `std::nullopt`. They should record the stage (name and - location) and expose it, e.g. via an accessor or a richer result type, so - callers can diagnose where a message was filtered out. (The `Void` type - already distinguishes an *intentional* dead-end from a dropped message; this - item covers observing *unintentional* drops.) +Planned improvements, not yet implemented, live in [TODO.md](TODO.md) — each +with what the library does today, the gap, an API sketch and the workaround +available in the meantime. The current list: + +- **More examples** for `GraphContext`, `finish()`, stateful merges, + `JoinFilter`, nested graphs and the in-band tick pattern. +- **Named merge slots**, so a fan-in group matches by name, not position. +- **Several named graph inputs** (`in.orders`, `in.quotes`), mirroring + `GraphOutputs`. +- **Stage labels and typed access** (`Summarize@stats`, `graph.stage("stats")`). +- **Track which stage short-circuited**, so a caller can see where a message + was dropped rather than only that it was. +- **Config-aware `registerMergeFilter`**, a fresh combiner per instance. +- **Injectable registry with duplicate detection**, instead of a singleton that + silently overwrites. +- **Documentation** of 0..n outputs, fan-out copies and stateful stages. +- **Lifting the current limitations**: nested stage arguments in the DSL, and + type checking inside `GraphOutputs`. ## License diff --git a/TODO.md b/TODO.md new file mode 100644 index 0000000..3e3e9e4 --- /dev/null +++ b/TODO.md @@ -0,0 +1,291 @@ +# filterGraph — TODO + +Planned work, in one place. Each item states what the library does **today**, +the **gap**, a **proposal** with an API sketch, its **compatibility** impact, +and the **workaround** that exists in the meantime. + +The list started as a set of extension ideas that came up while designing +stateful fan-out/fan-in graphs (several parallel stages whose results a +stateful merge combines, plus a result-checking stage), so most items are about +merges, stage state and the end of a run. They are written for the library in +general, and the API sketches are sketches: nothing here is implemented. + +Examples use the generic stages of the [README](README.md) (`Parse`, +`Validate`, `Summarize`, …). Shipped features are documented in the +[README](README.md) and [CHANGELOG.md](CHANGELOG.md), not here. + +## Overview + +| # | Item | Impact | Compatibility | Workaround today | +|---|------|--------|---------------|------------------| +| 1 | [More examples for the newer features](#1-more-examples-for-the-newer-features) | **high**: several features have no runnable example | additive | read the tests | +| 2 | [Named merge slots](#2-named-merge-slots) | medium | additive if done carefully | rely on group order | +| 3 | [Several named graph inputs](#3-several-named-graph-inputs) | medium | additive (new graph shape) | `std::variant` input + select stages | +| 4 | [Stage labels and typed access](#4-stage-labels-and-typed-access) | medium | additive (DSL syntax) | side registry filled by creator lambdas | +| 5 | [Track which stage short-circuited](#5-track-which-stage-short-circuited) | medium | additive | tap the edge, or log in the stage | +| 6 | [Config-aware `registerMergeFilter`](#6-config-aware-registermergefilter) | low | additive | subclass + `FilterRegistrar` creator | +| 7 | [Injectable registry, duplicate detection](#7-injectable-registry-duplicate-detection) | low | mostly additive | unique names | +| 8 | [Documentation: 0..n outputs, large messages](#8-documentation-0n-outputs-large-messages) | doc only | — | — | +| 9 | [Known limitations to lift](#9-known-limitations-to-lift) | low–medium | additive | JSON config; read `GraphOutputs` carefully | + +Two earlier items of the same list are done and therefore not repeated here: +**type-checked merge inputs** (`TypedMergeFilter` / `UniformMergeFilter` / +`registerTypedMergeFilter`) and the **end-of-stream hook** +(`MessageFilter::finish()`). Both are described in the README. + +--- + +## 1. More examples for the newer features + +**Today.** [`apps/textPipeline`](apps/textPipeline/main.cpp) and +[EXAMPLE.md](EXAMPLE.md) cover the compile-time `FilterGraph`, registering +stages, a DSL graph with a tap, fan-in with an untyped and a typed merge, +several named outputs, `validateDslGraph` and the JSON format. Several features +that shipped since have no runnable example: `GraphContext`, +`MessageFilter::finish()`, a merge with per-instance state, `JoinFilter`, a +nested graph used as a stage, a `Void` sink, and the in-band tick pattern that +stands in for a `tick()` hook. + +**Gap.** Those are exactly the features that come up once stages hold state, +and the hardest to get right from a reference description alone. Their only +executable documentation today is the test suite +([`GraphContextTests.cpp`](tests/FilterGraphTests/GraphContextTests.cpp), +[`LifecycleTests.cpp`](tests/FilterGraphTests/LifecycleTests.cpp)), which reads +as assertions rather than as a walkthrough. + +**Proposal.** One runnable app per theme, each in the style of `textPipeline` +(numbered blocks, comments that explain the *why*), plus a matching EXAMPLE.md +section per app: + +- `apps/statefulPipeline` — a stage that accumulates across messages, flushes + in `finish()` and publishes its result through the `GraphContext`; a merge + with per-instance state; a derived application context recovered with + `as()`. +- `apps/compositePipeline` — `JoinFilter` from JSON next to the equivalent DSL + merge; a `DslFilterGraph` registered as a stage of an outer graph (showing + that `finish()` and the context propagate into it); a `Void`-terminated sink; + an in-band `Tick` alternative in a variant input type. + +**Compatibility.** Additive: new targets under `apps/`, no library change. + +**Workaround today.** The tests, plus the README sections +[Graph context](README.md#graph-context) and +[Ending a run](README.md#ending-a-run). + +## 2. Named merge slots + +**Today.** Slots are positional, in the order the DSL group lists them. A merge +stage can only know which upstream a slot came from by convention. Since +type-checked merges shipped, a mis-*typed* slot is caught when the graph is +built — but two slots of the *same* type in the wrong order still pass +validation and then run silently wrong. + +**Proposal.** Optional slot names in the group, checked against the names the +stage declares: + +```text +(raw: msg, checked: valid) -> Summarize -> out.stats +``` + +```cpp +// empty = positional, today's behaviour +virtual std::vector mergeInputNames() const { return {}; } +``` + +Behaviour: + +- **Stage declares names, group uses names:** slots are matched by name, so the + order in the group no longer matters. Unknown or missing names produce + diagnostics. +- **Stage declares names, group is positional:** allowed, matched by position + (keeps today's graphs valid). +- **Stage declares nothing:** names in the group are an error (*'Summarize' has + no named slots*). + +Together with the slot types a typed merge already declares, this gives every +slot a name *and* a type, which also makes `dsl::toMermaid` output +self-explanatory. + +**Compatibility.** Keep `MergeInputs` a `std::vector` and reorder the +values into declared order before calling the stage, so stage code does not +change. Making `MergeInputs` a struct with names would be a breaking change and +is not needed. + +**Workaround today.** Rely on group order, and give same-typed slots distinct +wrapper types so that the existing slot-type check can tell them apart. + +## 3. Several named graph inputs + +**Today.** A graph has exactly one input edge `in`, with one type. A graph that +consumes several kinds of message needs a single `std::variant` input, and then +every stage has to unpack it, or one "select" stage per alternative drops the +others. + +**Proposal.** A graph with named inputs, mirroring `GraphOutputs`: + +```text +in.orders -> ParseOrder -> order +in.quotes -> ParseQuote -> quote +(order, quote) -> Match -> out +``` + +```cpp +DslFilterGraph graph(text); +graph.push("quotes", Quote{...}); // runs only the stages reachable from in.quotes +``` + +Semantics: + +- One `push` is one run. +- Input edges not fed in this run are **holes**, exactly like dropped paths + today, so merges and outputs need no new rules. +- Validation checks each named input's type at construction, which needs a way + to declare them, e.g. `GraphInputs::of("orders", "quotes")`. +- `filter(GraphInputs&&)` stays available, for "push several inputs in one run". + +**Compatibility.** Additive: `DslFilterGraph` with a plain input type +keeps the single `in` edge. + +**Workaround today.** A `std::variant` input plus one select stage per +alternative, each dropping the alternatives it does not handle. + +## 4. Stage labels and typed access + +**Today.** The owner of a `DslFilterGraph` cannot reach a stage instance. The +DSL has no labels (`#` starts a comment), and the compiled plan is private. +Tests, diagnostics and statistics therefore have to go through edges or global +state. + +**Proposal.** An optional label per stage, and typed lookup: + +```text +(msg, valid) -> Summarize@stats -> out.stats +``` + +```cpp +auto& stats = graph.stage("stats"); // throws if unknown or the type differs +``` + +- Labels are unique per graph, which is checked at construction. +- `dsl::toMermaid` can show them, and diagnostics can say `stats` instead of + "stage 'Summarize' at 3:17". +- Lookup needs `AnyMessageFilterAdapter` to expose the wrapped filter + (`std::shared_ptr` + `std::type_index`, or a virtual + `target(std::type_index)` in the style of `std::function::target`). + +**Compatibility.** Additive. The `@` character is unused in the DSL today. + +**Workaround today.** Register the stage with a creator lambda that records +every instance it builds in a side registry, or have the stage publish what the +owner needs through the `GraphContext`. + +## 5. Track which stage short-circuited + +**Today.** When a graph drops a message, neither `DslFilterGraph` nor +`AnyFilterChain` tells the caller *which* stage returned `std::nullopt`. + +**Gap.** A graph that silently produces nothing is hard to diagnose: the caller +sees an empty `std::optional` and has to bisect the graph to find the stage that +dropped the message. (`Void` already distinguishes an *intentional* dead end +from a dropped message; this item is about observing *unintentional* drops.) + +**Proposal.** Record the stage that short-circuited — its name, and its location +in the DSL text — and expose it, e.g. through an accessor valid after a run, or +a richer result type. Whatever the shape, it should stay allocation-free on the +happy path and say something useful for a drop inside a nested graph. + +**Compatibility.** Additive as an accessor; a new result type would be breaking, +so that variant needs an overload or an opt-in. + +**Workaround today.** Tap the suspect edge with a logging stage ending in `end`, +or log inside the stage that decides to drop. + +## 6. Config-aware `registerMergeFilter` + +**Today.** `registerMergeFilter(name, combiner)` ignores the DSL config and +**copies one combiner into every instance**. A combiner lambda that captures a +`shared_ptr` therefore **shares state across all instances and graphs**. The +header says so; the README and EXAMPLE.md do not. + +A merge with config and per-instance state is already possible: derive from +`MessageFilter` (or from `TypedMergeFilter`) and register it +with `FilterRegistrar(name, creator)`. The gap is ergonomics and +documentation, not capability. + +**Proposal.** + +```cpp +// A fresh combiner per instance, built from the stage's DSL config. +template +void registerMergeFilter(const std::string& name, + std::function::Combiner(const nlohmann::json& config)> factory); +``` + +Plus a short EXAMPLE.md section on "stateful merges: subclass + +`FilterRegistrar`" (see item 1), and a note in the README on the copy semantics +of the existing overload. + +**Compatibility.** Additive overload. + +**Workaround today.** Subclass and register with a creator lambda. + +## 7. Injectable registry, duplicate detection + +**Today.** `FilterRegistry::instance()` is a process-wide singleton, and +`registerFilter` **silently overwrites** an existing name. + +**Gap.** + +- Two libraries registering the same name shadow each other without notice. +- A test that wants to swap a stage for a fake has to mutate global state and + restore it. +- Graphs cannot be built against different stage sets in one process. + +**Proposal.** + +- `registerFilter` reports a duplicate name (throw, or return `false`), with an + explicit `replaceFilter(name, creator)` for intentional overrides. +- `DslFilterGraph(text, const FilterRegistry& registry = FilterRegistry::instance())`, + and the same parameter on `validateDslGraph`, `validateGraph` and + `JsonFilterGraph`. +- A copyable `FilterRegistry` (or `FilterRegistry::derive()`), so a test can + start from the global set and replace single stages. + +**Compatibility.** Turning an overwrite into an error is behaviour-breaking. +Ship it behind a transition: warn first, or add +`registerFilter(..., OnDuplicate)`. + +**Workaround today.** Keep names unique, and register test fakes under their own +names. + +## 8. Documentation: 0..n outputs, large messages + +No API change. These points belong in the README / EXAMPLE.md, because they come +up as soon as stages carry state: + +- **0..n outputs per input.** A stage emits at most one value per run. A general + multi-output stage would conflict with merge semantics (which output pairs + with which slot?), so the recommendation is **not** to add one. Document the + convention instead: a stage that can produce several results has a collection + as its output type, and downstream stages iterate over it. +- **Fan-out copies.** Every reader of an edge except the last gets a copy + (`GraphPlan::read`). Cheap values are fine; large or shared payloads should + travel as `std::shared_ptr`. +- **Stateful stages.** A stage instance lives as long as its graph, and state in + its members persists across messages. Say explicitly that this is supported + and intended, and that each graph construction creates fresh instances. + +## 9. Known limitations to lift + +The [current limitations](README.md#current-limitations) the README lists, as +work items: + +- **Nested stage arguments in the DSL.** Arguments are flat `key=value` pairs; + nested objects and lists are not expressible, so a stage that needs them has + to be configured in JSON. Lifting this means a value grammar for objects and + arrays, plus diagnostics for it. +- **Type checking inside `GraphOutputs`.** Only the graph's input type and the + single `out` type are checked against the C++ template parameters. The types + behind `out.` are checked when they are read (`get` throws + `std::bad_any_cast`), not when the graph is built. Checking them up front + needs a way to declare the expected type per key.