Plugin hardening (#85)

* fix: retain plugin library lifetime

* fix: harden plugin host boundaries

* docs: clarify plugin author guidance

* fix: avoid dialog instantiation during catalog scans

Add a static manifest tail slot to dialog vtables and teach catalog discovery to use it when present, while preserving the legacy create/get_manifest fallback for existing plugins.

Update SDK examples and docs for the two-argument dialog export, cover the static-manifest path in tests, and simplify redundant unexpected(std::string(...)) error construction.

* fix: fallback for null dialog static manifests

Use the dialog static manifest tail slot only when it is present and non-null. New-header plugins using the legacy one-argument dialog macro now correctly fall back to runtime manifest discovery.

* refactor: move plugin vtable validation out of header

Keep the loader required-slot checks in a private implementation unit and wire them through an internal detail library. Also remove the dialog protocol subproject's global C++ standard override so it no longer downgrades the root build setting.

* formatting

* feat: add parser arrow stream writes

* fix: harden dialog plugin loading

* fix: enable MSVC conformant preprocessor

PJ_DIALOG_PLUGIN's overload-by-arg-count idiom relies on __VA_ARGS__
splitting on commas inside nested macro calls. MSVC's traditional
preprocessor passes __VA_ARGS__ as a single token, causing the
2-argument form to dispatch to the legacy 1-argument macro and
generate a bogus dialogVtableFor<Class, kManifest> specialisation.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix: update missing slot fixture for parser tail slot

* fix: export MSVC preprocessor mode for dialog SDK

---------

Co-authored-by: Davide Faconti <dfaconti@aurynrobotics.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: 3 people

CMakeLists.txt

@@ -32,7 +32,10 @@ option(PJ_ENABLE_ABI_CHECK "Enable abidiff-based ABI drift gate (requires libabi
 # ---------------------------------------------------------------------------
 
 if(MSVC)
-  set(PJ_WARNING_FLAGS /W4 /WX /permissive-)
+  # /Zc:preprocessor: conformant preprocessor — required so __VA_ARGS__ inside
+  # nested macro calls (e.g. PJ_DIALOG_PLUGIN's overload-by-arg-count idiom)
+  # splits on commas instead of being passed as a single token.
+  set(PJ_WARNING_FLAGS /W4 /WX /permissive- /Zc:preprocessor)
 else()
   set(PJ_WARNING_FLAGS
     -Wall -Wextra -Werror -Wshadow -Wnon-virtual-dtor -Wold-style-cast

docs/dialog-sdk-reference.md

@@ -53,9 +53,10 @@ For the full tutorial, see [dialog-plugin-guide.md](../pj_plugins/docs/dialog-pl
 | Method | Description |
 |--------|-------------|
 | `setButtonText(name, text)` | Set button label |
-| `setShortcut(name, key_sequence)` | Assign keyboard shortcut (e.g. `"Ctrl+A"`) **NEW** |
+| `setButtonIcon(name, svg_data)` | Set an inline SVG icon |
+| `setShortcut(name, key_sequence)` | Assign keyboard shortcut (e.g. `"Ctrl+A"`) |
 | `setFilePicker(name, text, filter, title)` | Turn into file picker |
-| `setFolderPicker(name, text, title)` | Turn into folder picker **NEW** |
+| `setFolderPicker(name, text, title)` | Turn into folder picker |
 
 ### QListWidget
 
@@ -71,13 +72,23 @@ For the full tutorial, see [dialog-plugin-guide.md](../pj_plugins/docs/dialog-pl
 | `setTableHeaders(name, vector<string>)` | Set column headers |
 | `setTableRows(name, vector<vector<string>>)` | Set row data |
 | `setSelectedRows(name, vector<int>)` | Set selected row indices |
-| `setDisabledRows(name, vector<int>)` | Grey out rows (non-selectable) **NEW** |
+| `setDisabledRows(name, vector<int>)` | Grey out rows (non-selectable) |
+
+### QFrame Chart Container
+
+| Method | Description |
+|--------|-------------|
+| `setChartSeries(name, vector<ChartSeries>)` | Create/update chart series inside a QFrame |
+| `clearChart(name)` | Remove chart series |
+| `setChartZoomEnabled(name, bool)` | Enable chart zoom/pan events |
 
 ### QPlainTextEdit
 
 | Method | Description |
 |--------|-------------|
-| `setPlainText(name, text)` | Set plain text content **NEW** |
+| `setPlainText(name, text)` | Set plain text content |
+| `setCodeContent(name, code)` | Set editable code content |
+| `setCodeLanguage(name, lang)` | Set syntax highlighting language such as `"lua"` or `"python"` |
 
 ### QTabWidget
 
@@ -98,6 +109,7 @@ For the full tutorial, see [dialog-plugin-guide.md](../pj_plugins/docs/dialog-pl
 |--------|-------------|
 | `setEnabled(name, bool)` | Enable/disable widget |
 | `setVisible(name, bool)` | Show/hide widget |
+| `setDropTarget(name, bool)` | Accept dropped item labels and emit `onItemsDropped` |
 
 ### Dialog-level Commands
 
@@ -121,9 +133,12 @@ Override these in your `DialogPluginTyped` subclass. Return `true` when state ch
 | `onValueChanged(name, double)` | QDoubleSpinBox | New double value |
 | `onClicked(name)` | QPushButton | (no payload) |
 | `onFileSelected(name, path)` | QPushButton (file picker) | Selected file path |
-| `onFolderSelected(name, path)` | QPushButton (folder picker) | Selected folder path **NEW** |
+| `onFolderSelected(name, path)` | QPushButton (folder picker) | Selected folder path |
 | `onSelectionChanged(name, items)` | QListWidget, QTableWidget | Vector of selected item texts |
 | `onItemDoubleClicked(name, index)` | QListWidget, QTableWidget | Row index of double-clicked item |
+| `onCodeChanged(name, code)` | QPlainTextEdit code editor | Edited code |
+| `onItemsDropped(name, items)` | Any widget with `setDropTarget` | Dropped item labels |
+| `onChartViewChanged(name, x_min, x_max, y_min, y_max)` | QFrame chart container | Visible chart range |
 | `onTabChanged(name, index)` | QTabWidget | New tab index |
 
 ---
@@ -137,7 +152,6 @@ Override these in your `DialogPluginTyped` subclass. Return `true` when state ch
 | `onRejected()` | User clicked Cancel | void |
 | `saveConfig()` | Host persisting state | JSON string |
 | `loadConfig(json)` | Host restoring state | `true` if state changed |
-| `lastError()` | Host checking for errors | Error string or empty |
 
 ---
 

docs/toolbox-porting-gap-analysis.md

@@ -1,17 +1,18 @@
 # Toolbox Porting — SDK Gap Analysis
 
-This document provides an exhaustive comparison of PlotJuggler 3.x toolbox plugin
-features against the current PJ 4.x SDK capabilities. It identifies what Dialog SDK
-extensions are required to port the existing toolboxes with full feature parity.
+This historical document compares PlotJuggler 3.x toolbox plugin features
+against the PJ 4.x SDK capabilities available when the porting work began.
+Some gaps listed below have since been closed in the Dialog SDK.
 
 **Scope:** `plotjuggler_core` (Dialog SDK, `ToolboxPluginBase`) + `pj-official-plugins`
 (Quaternion to RPY port as the reference implementation).
 
-**Summary:** The SDK is sufficient for simple, headless processing toolboxes (Quaternion
-reaches ~80% feature parity). FFT drops to ~60%. The Lua Reactive Script Editor cannot
-be ported without major SDK extensions. Six gaps are blocking and require new
-infrastructure: embedded chart widget, zoom event, drag-drop on chart, editable code
-editor, reactive time-tick, and ScatterXY output type.
+**Current status:** Dialog SDK support now exists for chart containers,
+chart zoom/pan events, drop targets, editable `QPlainTextEdit` code editors,
+and periodic ticks. Treat the original gap analysis below as historical
+context for porting priorities, not as a current reference. Remaining
+porting risks should be revalidated against the current SDK and datastore
+surface; ScatterXY-style output remains a separate datastore concern.
 
 ---
 
@@ -51,7 +52,10 @@ In the current port:
 - No preview — the transform runs directly on OK
 - Modal dialog that opens and closes
 
-This is not a failure of the port — it is a limitation of the current SDK: `DialogPluginTyped` does not support preview widgets (charts) or drag-and-drop. Those features require Dialog SDK extensions.
+At the time of the original port, `DialogPluginTyped` did not support preview
+widgets or drag-and-drop. Current Dialog SDKs provide chart containers,
+chart range events, and generic drop targets; any toolbox port should be
+revalidated against those APIs before treating this section as blocking.
 
 The end-to-end wiring is not yet confirmed at 100%. The plugin loads, the dialog opens with combo boxes, but we have not verified that the transform produces data visible in the plots. There may be a mismatch in catalog field names.
 
@@ -74,11 +78,11 @@ Three toolbox plugins exist in `plotjuggler/plotjuggler_plugins/`:
 | Aspect | PJ 3.x | New SDK |
 |--------|--------|---------|
 | Plugin owns its UI | Yes — full `QWidget` with arbitrary children | No — `.ui` file + host-rendered dialog runtime |
-| Data access | Direct reference to `PlotDataMapRef` | Via handles: `catalogSnapshot`, `readSeries`, `appendRecord` |
+| Data access | Direct reference to `PlotDataMapRef` | Via handles: `catalogSnapshot`, `readSeriesArrow`, `appendRecord` |
 | Communication | Qt signals/slots | C ABI vtables + JSON config |
 | Qt dependency | Required | None in core/plugin SDK; GUI hosts supply their toolkit runtime |
-| Embedded chart preview | Integrated (`PlotWidgetBase`) | Not available in the SDK |
-| Drag-and-drop | Via `eventFilter` in the plugin | Not supported by the dialog protocol |
+| Embedded chart preview | Integrated (`PlotWidgetBase`) | Available through chart data on QFrame containers |
+| Drag-and-drop | Via `eventFilter` in the plugin | Available through dialog drop targets and `onItemsDropped` |
 | Output type | `PlotData` (time series) **or** `PlotDataXY` (scatter) | Time-indexed series only |
 | Transform registry | Yes — re-applied on layout reload | No — outputs are static data |
 | Reactive execution | `ReactiveLuaFunction` re-runs on every slider tick | `onTick()` exists but cannot write to datastore or access current timestamp |
@@ -250,7 +254,10 @@ The Lua editor uses `QCodeEditor` (external library) with:
 | Instances | 3: global code, function body, library |
 | Font size | Ctrl+wheel: 8–14 pt range, persisted to `QSettings` |
 
-The current SDK has `setPlainText()` for **read-only** text display only. The dialog docs explicitly state that `QTextEdit` and `QPlainTextEdit` are **not supported** by the widget binding system. Without an editable code widget, the Lua editor cannot exist.
+The current Dialog SDK has `setPlainText()`, editable `setCodeContent()`,
+`setCodeLanguage()`, and `onCodeChanged()` for `QPlainTextEdit`-based code
+editing. The Lua editor port still needs product-level validation, but an
+editable code widget is no longer a missing SDK primitive.
 
 **SDK equivalent needed:**
 ```cpp
@@ -299,7 +306,7 @@ The sol2 Lua API exposed to scripts:
 | `CreatedSeriesTime` | `at(i)`, `clear()`, `push_back(x,y)`, `size()` |
 | `CreatedSeriesXY` | `at(i)`, `clear()`, `push_back(x,y)`, `size()` |
 
-The current SDK `onTick()` in `DialogPluginTyped` fires periodically while the dialog is open, but:
+Dialog-level `onTick()` in `DialogPluginTyped` fires periodically while the dialog is open, but:
 - Has no access to the current time slider value
 - Cannot call `toolboxHost().appendRecord()` — the toolbox host is separate from the dialog plugin
 - Cannot be used to implement time-reactive series generation
@@ -388,13 +395,13 @@ The Lua editor persists several settings directly via `QSettings` outside of `sa
 | 4.2 | Transform registry (re-apply on reload) | MEDIUM | MEDIUM | HIGH | MEDIUM |
 | 4.3 | `QSettings` fine-grained persistence | LOW | LOW | MEDIUM | LOW |
 
-**Feature parity estimate without SDK changes:**
+**Original feature parity estimate from the first gap analysis:**
 
 | Toolbox | Parity | Blocking gaps |
 |---------|--------|---------------|
-| Quaternion | ~80% | No chart preview; no drag-drop auto-fill |
-| FFT | ~60% | No chart preview; no drag-drop; no zoom-aware range; no ScatterXY output |
-| Lua Editor | ~10% | No editable code widget; no reactive execution |
+| Quaternion | ~80% | Originally blocked by chart preview and drag-drop auto-fill; revalidate against current chart/drop APIs |
+| FFT | ~60% | Revalidate chart, drag-drop, and zoom support; ScatterXY output remains a datastore question |
+| Lua Editor | ~10% | Editable code widget now exists; reactive execution still needs product-level validation |
 
 ---
 
@@ -478,4 +485,6 @@ void appendArrowScatterIpc(TopicHandle topic,
 
 ---
 
-*Gaps 1.1, 1.4, 2.1, 3.1, 3.3, and 4.1 are blocking for their respective toolboxes and require new SDK infrastructure. The remaining gaps represent UX degradation that can be partially mitigated with workarounds within the current SDK.*
+*Historical note: several gaps called blocking in this document have since
+received SDK support. Before using this as a porting checklist, re-run the
+gap analysis against the current Dialog SDK and datastore APIs.*

pj_base/include/pj_base/data_source_protocol.h

@@ -2,7 +2,7 @@
  * @file data_source_protocol.h
  * @brief C ABI protocol for DataSource plugins (version 4).
  *
- * v4 summary of changes vs v3:
+ * v4 summary:
  *   - Arrow C Data Interface at the write boundary: bulk loaders use
  *     SourceWriteHost::append_arrow_stream instead of per-row appends.
  *     See pj_base/plugin_data_api.h. append_arrow_ipc is removed.
@@ -49,7 +49,7 @@ extern "C" {
  * Reads of any slot added after v4.0 must be gated with PJ_HAS_TAIL_SLOT.
  *
  * Computed as `offsetof(last v4.0 slot) + sizeof(its function pointer)`.
- * Last v4.0 slot is `get_plugin_extension` (promoted from v3.1 tail).
+ * Last v4.0 slot is `get_plugin_extension`.
  */
 #define PJ_DATA_SOURCE_MIN_VTABLE_SIZE \
   (offsetof(PJ_data_source_vtable_t, get_plugin_extension) + sizeof(const void* (*)(void*, PJ_string_view_t)))

pj_base/include/pj_base/message_parser_protocol.h

@@ -2,11 +2,12 @@
  * @file message_parser_protocol.h
  * @brief C ABI protocol for MessageParser plugins (version 4).
  *
- * v4 summary of changes vs v3:
+ * v4 summary:
  *   - Every vtable slot is PJ_NOEXCEPT and carries a thread-class tag.
  *   - Parser write host (pj.parser_write.v1) no longer has
- *     append_arrow_ipc — see plugin_data_api.h. Parsers stay per-record;
- *     the host coalesces into Arrow batches internally.
+ *     append_arrow_ipc — see plugin_data_api.h. Parsers normally write
+ *     per-record, with an optional append_arrow_stream tail slot for
+ *     parser-shaped formats that naturally decode batches.
  *
  * Pure-functional production (scalars by value, canonical objects by
  * value with BufferAnchor) is a C++ SDK contract: parsers inheriting from
@@ -46,7 +47,7 @@ extern "C" {
  * MUST NOT GROW when new tail slots are appended. See PJ_ABI_VERSION comment
  * in plugin_data_api.h for the rationale.
  *
- * Last v4.0 slot is `get_plugin_extension` (promoted from v3 tail).
+ * Last v4.0 slot is `get_plugin_extension`.
  */
 #define PJ_MESSAGE_PARSER_MIN_VTABLE_SIZE \
   (offsetof(PJ_message_parser_vtable_t, get_plugin_extension) + sizeof(const void* (*)(void*, PJ_string_view_t)))
@@ -82,8 +83,8 @@ typedef struct PJ_message_parser_vtable_t {
    *                and the marketplace; must be unique per plugin.
    *   "name"     — human-readable plugin name (string).
    *   "version"  — semver version string (string).
-   *   "encoding" — encoding this parser handles (string). The host uses
-   *                this to match binding requests to parsers.
+   *   "encoding" — encodings this parser handles (array of strings). The host
+   *                uses this to match binding requests to parsers.
    */
   const char* manifest_json;
 

pj_base/include/pj_base/plugin_data_api.h

@@ -45,7 +45,7 @@ extern "C" {
  * (e.g. DataSource + Dialog in one .so) work without any extra ceremony.
  * Do not redefine it manually.
  *
- * v4 plugins advertise version 4. Breaking v3→v4 changes:
+ * v4 plugins advertise version 4. Data-plane changes from the pre-v4 design:
  *   - Arrow C Data Interface replaces Arrow IPC bytes at the boundary
  *     (append_arrow_stream + read_series_arrow).
  *   - append_arrow_ipc removed from all write hosts.
@@ -388,9 +388,10 @@ typedef struct {
  * Parser write host: single-topic writes. The bound topic is set at
  * service-creation time; the parser plugin never names it.
  *
- * No append_arrow_stream: parsers are inherently per-message. The host
- * internally coalesces per-record appends into Arrow batches before
- * committing to storage — plugin authors never see the batch grain. */
+ * append_arrow_stream is an optional tail slot for parser-shaped formats
+ * that naturally decode a batch in one parse() call. Ownership matches
+ * PJ_source_write_host_vtable_t::append_arrow_stream. Plugins must gate
+ * this slot with PJ_HAS_TAIL_SLOT when calling through the C ABI directly. */
 typedef struct PJ_parser_write_host_vtable_t {
   uint32_t abi_version;
   uint32_t struct_size;
@@ -409,8 +410,22 @@ typedef struct PJ_parser_write_host_vtable_t {
   bool (*append_bound_record)(
       void* ctx, int64_t timestamp, const PJ_bound_field_value_t* fields, size_t field_count,
       PJ_error_t* out_error) PJ_NOEXCEPT;
+
+  /* [stream-thread] Optional batch path. Plugin hands ownership of an Arrow
+   * C Data Interface stream for the bound topic. The timestamp column rule
+   * and success/failure ownership contract are identical to
+   * PJ_source_write_host_vtable_t::append_arrow_stream. */
+  bool (*append_arrow_stream)(
+      void* ctx, struct ArrowArrayStream* stream, PJ_string_view_t timestamp_column, PJ_error_t* out_error) PJ_NOEXCEPT;
 } PJ_parser_write_host_vtable_t;
 
+/*
+ * Parser write-host v4.0 floor, before append_arrow_stream was added as an
+ * optional tail slot. Hosts/plugins that care about the batch path must use
+ * PJ_HAS_TAIL_SLOT(PJ_parser_write_host_vtable_t, vtable, append_arrow_stream).
+ */
+#define PJ_PARSER_WRITE_HOST_MIN_VTABLE_SIZE (offsetof(PJ_parser_write_host_vtable_t, append_arrow_stream))
+
 typedef struct {
   void* ctx;
   const PJ_parser_write_host_vtable_t* vtable;

pj_base/include/pj_base/sdk/arrow.hpp

@@ -133,13 +133,16 @@ using ArrowArrayHolder = detail::ArrowHolder<::ArrowArray>;
 ///
 /// Recommended usage: hand the holder by rvalue reference to the
 /// `appendArrowStream(ArrowStreamHolder&&, ...)` overload on
-/// `SourceWriteHostView` / `ToolboxHostView`, which disarms the holder on
-/// success:
+/// `SourceWriteHostView`, `ParserWriteHostView`, or `ToolboxHostView`,
+/// which disarms the holder on success:
 ///
 ///   ArrowStreamHolder stream(buildMyStream());
 ///   auto status = writeHost.appendArrowStream(topic, std::move(stream), "timestamp");
 ///   // on success, holder is inert; on failure, destructor releases the stream.
 ///
+/// Parser write hosts omit the `topic` argument because the host binds the
+/// parser to one topic before parsing begins.
+///
 /// The raw-pointer overload of `appendArrowStream` remains as an ABI escape
 /// hatch for callers that own the stream through some other mechanism.
 using ArrowStreamHolder = detail::ArrowHolder<::ArrowArrayStream>;