From 1841b6012efb68160e0e3c12ebb2201d88b6738b Mon Sep 17 00:00:00 2001 From: alvvm Date: Tue, 1 Sep 2026 08:19:23 +0200 Subject: [PATCH] feat(sdk): dataset-qualified series name contract + shared split helper (0.26.0) pj.data_processors.v1 addressed input series by bare topic/field names and never said what a name means when several loaded datasets share topic names, so conforming hosts filled the gap incompatibly (PJ4's transform path refused duplicates while its marker path silently bound the first-loaded dataset). The contract is now explicit in the PJ_data_processors_host_vtable_t doc block (DATASET-QUALIFIED NAMES), and the new installed header pj_base/sdk/dataset_qualified_name.hpp ships the shared parser/composer (splitDatasetQualifier / qualifiedSeriesName) so hosts and plugins use one implementation instead of the two copies that exist today. MINOR bump to 0.26.0: new installed public header, no ABI change. --- CHANGELOG.md | 29 ++++++++ VERSION | 2 +- pj_base/CMakeLists.txt | 1 + pj_base/include/pj_base/plugin_data_api.h | 20 +++++- .../pj_base/sdk/dataset_qualified_name.hpp | 65 +++++++++++++++++ .../include/pj_base/sdk/plugin_data_api.hpp | 9 ++- pj_base/tests/dataset_qualified_name_test.cpp | 71 +++++++++++++++++++ pj_plugins/docs/ARCHITECTURE.md | 6 +- 8 files changed, 198 insertions(+), 5 deletions(-) create mode 100644 pj_base/include/pj_base/sdk/dataset_qualified_name.hpp create mode 100644 pj_base/tests/dataset_qualified_name_test.cpp diff --git a/CHANGELOG.md b/CHANGELOG.md index 97d7d09b..4ba8c2cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,35 @@ All notable changes to `plotjuggler_sdk` are recorded here. Versioning policy is in [`CLAUDE.md`](./CLAUDE.md) → "Release Versioning". +## [0.26.0] + +### Feature: dataset-qualified series names — contract and shared helper (MINOR) + +`pj.data_processors.v1` addressed input series by bare `topic/field` names and never said +what a name means when several loaded datasets share topic names — so conforming hosts +filled the gap incompatibly (PJ4's transform path refused duplicates while its marker path +silently bound the first-loaded dataset; fixed by PlotJuggler/PJ4#619). The contract is now +explicit in the `PJ_data_processors_host_vtable_t` doc block (DATASET-QUALIFIED NAMES): + +- A series' full identity is (dataset, topic, field); a bare name is an abbreviation. An + input MAY carry the qualifier `dataset_source:topic/field` — the same form hosts print + as a series identity, so displayed names round-trip as inputs. +- The qualifier is matched against the **loaded** source names (longest match wins), never + split blindly at `:` — stream-style source names like `[stream] UDP Server` need no + escaping. +- A qualifier matching no loaded dataset is an error (no fallback to the bare reading); a + bare name that exists in several datasets MUST be refused with the qualified candidates, + never resolved by load order; qualified inputs of one processor must agree on a single + dataset. Marker per-series output keys accept the qualifier the same way. + +New installed header `pj_base/sdk/dataset_qualified_name.hpp` ships the shared +parser/composer (`splitDatasetQualifier` / `qualifiedSeriesName`, header-only) so hosts +and plugins use one implementation instead of the two copies that exist today. + +Plugins that emit qualified names pin `plotjuggler_sdk/[>=0.26.0 <1.0.0]`. No ABI change; +`abi/baseline.abi` is untouched. Addressing two datasets that share one source name stays +out of contract (ambiguous → error); a typed dataset id would be a tail-appended addition. + ## [0.25.0] ### Feature: plugin-authoring CMake helpers ship with the SDK (MINOR) diff --git a/VERSION b/VERSION index d21d277b..4e8f395f 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.25.0 +0.26.0 diff --git a/pj_base/CMakeLists.txt b/pj_base/CMakeLists.txt index 6603178f..10debf26 100644 --- a/pj_base/CMakeLists.txt +++ b/pj_base/CMakeLists.txt @@ -151,6 +151,7 @@ if(PJ_BUILD_TESTS) tests/parser_module_abi_test.cpp tests/parser_module_manifest_test.cpp tests/data_processors_api_test.cpp + tests/dataset_qualified_name_test.cpp tests/settings_store_host_test.cpp tests/parser_runtime_host_test.cpp tests/data_source_protocol_test.cpp diff --git a/pj_base/include/pj_base/plugin_data_api.h b/pj_base/include/pj_base/plugin_data_api.h index 93e302f0..822ddae0 100644 --- a/pj_base/include/pj_base/plugin_data_api.h +++ b/pj_base/include/pj_base/plugin_data_api.h @@ -913,7 +913,8 @@ typedef struct { * - language : script backend, "luau" today; the host rejects anything else. * - inputs : topic OR topic-field names ("pose/orientation" or * "pose/orientation/x") the script reads; the host resolves them - * and exact-joins co-timestamped inputs. + * and exact-joins co-timestamped inputs. A name MAY carry the + * dataset qualifier, see DATASET-QUALIFIED NAMES below. * - outputs : target topic key(s). MAY be empty for an ephemeral preview * (flags & PJ_DATA_PROCESSOR_FLAG_EPHEMERAL), in which case the host * names the sink(s) and returns the resolved names in out_topics. @@ -931,6 +932,23 @@ typedef struct { * - flags : bitset; PJ_DATA_PROCESSOR_FLAG_EPHEMERAL marks a preview (never * persisted, dropped on remove). Reserved bits must be 0. * + * DATASET-QUALIFIED NAMES — a series' full identity is (dataset, topic, field); a + * bare "topic/field" name is an abbreviation that stops being unique the moment + * two loaded datasets share topic names. An input MAY therefore carry the + * qualifier "dataset_source:topic/field" — the same form hosts print as a series + * identity (shared parser/composer: sdk/dataset_qualified_name.hpp). The qualifier + * is matched against the LOADED source names (longest match wins), never split + * blindly at ':', so source names need no escaping. The host MUST enforce: + * - a qualifier matching no loaded dataset is an error — no fallback to reading + * the whole string as a bare name; + * - a bare name that exists in SEVERAL loaded datasets is refused, reporting the + * qualified candidates — NEVER resolved by load order; + * - all qualified inputs of one processor agree on a single dataset. + * Marker output keys (per-series, see markerSeriesKey) accept the qualifier the + * same way; transform outputs name NEW topics and are never qualified. Addressing + * two datasets that share one source name is outside this contract (ambiguous -> + * error); that needs a typed dataset id, which would be a tail-appended addition. + * * FORWARD-COMPAT — the native door is WASM, not a C++ kernel. Because the script * slot is a binary-safe blob the host owns and runs (today Luau; tomorrow a * host-owned WASM/Python backend), a native processor needs NO new ABI: it ships diff --git a/pj_base/include/pj_base/sdk/dataset_qualified_name.hpp b/pj_base/include/pj_base/sdk/dataset_qualified_name.hpp new file mode 100644 index 00000000..44c43265 --- /dev/null +++ b/pj_base/include/pj_base/sdk/dataset_qualified_name.hpp @@ -0,0 +1,65 @@ +#pragma once +// Copyright 2026 Davide Faconti +// SPDX-License-Identifier: Apache-2.0 + +// Dataset-qualified series names — the naming contract for series addressed by +// string across the plugin boundary (normative statement in plugin_data_api.h, +// DATASET-QUALIFIED NAMES). A series' full identity is (dataset, topic, field); +// a bare "topic/field" name is an abbreviation that stops being unique the +// moment two loaded datasets share topic names. The canonical serialized +// identity is "dataset_source:topic/field" — the same form hosts print — and +// this header is the shared parser/composer for it, so hosts and plugins never +// drift. + +#include +#include +#include + +#include "pj_base/span.hpp" + +namespace PJ::sdk { + +/// Result of splitting a series name that may carry the dataset qualifier, +/// "dataset_source:topic/field". +/// @since 0.26.0 +struct DatasetQualifierSplit { + bool qualified = false; + std::string dataset_source; ///< the matched source name; empty when unqualified + std::string bare; ///< the name with the qualifier removed +}; + +/// Split `name` against the KNOWN dataset source names (longest match wins). +/// Matching against known names instead of parsing at ':' means source names +/// need no escaping — "[stream] UDP Server:/udp/data" works as-is — and a ':' +/// inside an ordinary name can never be misread as a qualifier. +/// @since 0.26.0 +[[nodiscard]] inline DatasetQualifierSplit splitDatasetQualifier( + std::string_view name, Span source_names) { + DatasetQualifierSplit out; + std::size_t best = 0; + for (const std::string& src : source_names) { + if (src.empty() || src.size() <= best || name.size() <= src.size() || name[src.size()] != ':' || + name.compare(0, src.size(), src) != 0) { + continue; + } + best = src.size(); + out.dataset_source = src; + } + out.qualified = best != 0; + out.bare = std::string(out.qualified ? name.substr(best + 1) : name); + return out; +} + +/// The canonical dataset-qualified form, "dataset_source:bare" — the inverse of +/// splitDatasetQualifier for a loaded source name. +/// @since 0.26.0 +[[nodiscard]] inline std::string qualifiedSeriesName(std::string_view dataset_source, std::string_view bare) { + std::string out; + out.reserve(dataset_source.size() + 1 + bare.size()); + out.append(dataset_source); + out.push_back(':'); + out.append(bare); + return out; +} + +} // namespace PJ::sdk diff --git a/pj_base/include/pj_base/sdk/plugin_data_api.hpp b/pj_base/include/pj_base/sdk/plugin_data_api.hpp index a6cb86e6..ee53eee2 100644 --- a/pj_base/include/pj_base/sdk/plugin_data_api.hpp +++ b/pj_base/include/pj_base/sdk/plugin_data_api.hpp @@ -1490,7 +1490,10 @@ class DataProcessorsHostView { /// PJ_DATA_PROCESSOR_FLAG_EPHEMERAL), in which case the host names the sink(s). /// Returns the resolved physical output topic name(s) (owned copies) so the caller /// can read results back through the kind's read surface. `params_json` is forwarded - /// verbatim to the script. + /// verbatim to the script. Input names MAY carry the dataset qualifier + /// "dataset_source:topic/field" (see DATASET-QUALIFIED NAMES in plugin_data_api.h; + /// shared parser/composer in sdk/dataset_qualified_name.hpp) — required to address a + /// series whose bare name exists in several loaded datasets. [[nodiscard]] Expected> create( std::string_view id, std::string_view kind, std::string_view language, Span inputs, Span outputs, std::string_view script, std::string_view params_json, @@ -1546,7 +1549,8 @@ class DataProcessorsHostView { /// Convenience: create a kind="transform" node (DerivedEngine timeseries). `outputs` /// must be non-empty. Discards the resolved topic names (the caller supplied them); - /// use create() directly if you need them back. + /// use create() directly if you need them back. Inputs MAY be dataset-qualified (see + /// create()); outputs name NEW topics and are never qualified. [[nodiscard]] Status createTransform( std::string_view id, Span inputs, Span outputs, std::string_view script, std::string_view params_json, uint32_t flags = 0) const { @@ -1572,6 +1576,7 @@ class DataProcessorsHostView { /// key (kGlobalMarkerTopic or markerSeriesKey). Pass /// flags=PJ_DATA_PROCESSOR_FLAG_EPHEMERAL for a live preview (output may be left empty /// to let the host name the preview topic). Returns the resolved object topic(s). + /// Inputs and per-series output keys MAY be dataset-qualified (see create()). [[nodiscard]] Expected> createMarkers( std::string_view id, Span inputs, std::string_view output_marker_topic, std::string_view script, std::string_view params_json, uint32_t flags = 0) const { diff --git a/pj_base/tests/dataset_qualified_name_test.cpp b/pj_base/tests/dataset_qualified_name_test.cpp new file mode 100644 index 00000000..5fda7311 --- /dev/null +++ b/pj_base/tests/dataset_qualified_name_test.cpp @@ -0,0 +1,71 @@ +// Copyright 2026 Davide Faconti +// SPDX-License-Identifier: Apache-2.0 + +#include "pj_base/sdk/dataset_qualified_name.hpp" + +#include + +#include +#include + +namespace PJ::sdk { +namespace { + +TEST(SplitDatasetQualifier, MatchesOnlyKnownSourceNames) { + const std::vector sources = {"run_a.mcap", "run_b.mcap"}; + const auto hit = splitDatasetQualifier("run_a.mcap:/speed/value", sources); + EXPECT_TRUE(hit.qualified); + EXPECT_EQ(hit.dataset_source, "run_a.mcap"); + EXPECT_EQ(hit.bare, "/speed/value"); + + // A ':' whose prefix is no loaded source is part of the name, not a qualifier. + const std::vector other = {"other.mcap"}; + const auto miss = splitDatasetQualifier("run_a.mcap:/speed/value", other); + EXPECT_FALSE(miss.qualified); + EXPECT_TRUE(miss.dataset_source.empty()); + EXPECT_EQ(miss.bare, "run_a.mcap:/speed/value"); +} + +TEST(SplitDatasetQualifier, StreamStyleNamesNeedNoEscaping) { + const std::vector sources = {"[stream] UDP Server"}; + const auto split = splitDatasetQualifier("[stream] UDP Server:/udp/data/value", sources); + EXPECT_TRUE(split.qualified); + EXPECT_EQ(split.dataset_source, "[stream] UDP Server"); + EXPECT_EQ(split.bare, "/udp/data/value"); +} + +TEST(SplitDatasetQualifier, LongestKnownNameWins) { + const std::vector sources = {"a", "a:b"}; + const auto split = splitDatasetQualifier("a:b:/t/f", sources); + EXPECT_TRUE(split.qualified); + EXPECT_EQ(split.dataset_source, "a:b"); + EXPECT_EQ(split.bare, "/t/f"); +} + +TEST(SplitDatasetQualifier, EmptyAndDegenerateInputs) { + const std::vector sources = {"", "run_a.mcap"}; + // An empty source name never qualifies anything. + EXPECT_FALSE(splitDatasetQualifier(":/speed/value", sources).qualified); + // The bare name alone (no ':' after a known source) stays bare. + EXPECT_FALSE(splitDatasetQualifier("run_a.mcap", sources).qualified); + // A qualifier with nothing after the ':' splits to an empty bare name. + const auto empty_bare = splitDatasetQualifier("run_a.mcap:", sources); + EXPECT_TRUE(empty_bare.qualified); + EXPECT_EQ(empty_bare.bare, ""); + // No datasets loaded: nothing can qualify. + EXPECT_FALSE(splitDatasetQualifier("run_a.mcap:/speed/value", {}).qualified); +} + +TEST(QualifiedSeriesName, RoundTripsThroughSplit) { + const std::vector sources = {"run_a.mcap", "session 12.mcap"}; + const std::string qualified = qualifiedSeriesName("session 12.mcap", "/imu/accel/x"); + EXPECT_EQ(qualified, "session 12.mcap:/imu/accel/x"); + + const auto split = splitDatasetQualifier(qualified, sources); + EXPECT_TRUE(split.qualified); + EXPECT_EQ(split.dataset_source, "session 12.mcap"); + EXPECT_EQ(split.bare, "/imu/accel/x"); +} + +} // namespace +} // namespace PJ::sdk diff --git a/pj_plugins/docs/ARCHITECTURE.md b/pj_plugins/docs/ARCHITECTURE.md index 0aa335e5..e2445203 100644 --- a/pj_plugins/docs/ARCHITECTURE.md +++ b/pj_plugins/docs/ARCHITECTURE.md @@ -239,7 +239,11 @@ service registry, error out-params, and typed borrowed-dialog patterns): `pj_base/descriptor_import_protocol.h`. `"pj.data_processors.v1"` (optional) lets a toolbox create catalog-resident transform nodes in the host by data — a script plus input/output names and a params JSON blob; nothing executable crosses the - boundary (the host owns execution). The script payload is **binary-safe** + boundary (the host owns execution). Input names may carry the dataset + qualifier `dataset_source:topic/field`, so a name several loaded datasets + share is addressed rather than guessed — rules are normative in + `plugin_data_api.h` (DATASET-QUALIFIED NAMES), shared parser/composer in + `pj_base/sdk/dataset_qualified_name.hpp`. The script payload is **binary-safe** (`PJ_string_view_t {data,size}`), so the native "door" is WASM bytes through this same data-only surface (a future host-owned WASM/Python backend is purely additive and survives plugin unload) — deliberately *not* a C++ kernel vtable