treewide: cleanup

Signed-off-by: NotAShelf <raf@notashelf.dev>
Change-Id: Ia01590cdeed872cc8ebd16f6ca95f3cc6a6a6964

crates/pinakes-core/src/export.rs

@@ -42,7 +42,9 @@ pub async fn export_library(
   match format {
     ExportFormat::Json => {
       let json = serde_json::to_string_pretty(&items).map_err(|e| {
-        crate::error::PinakesError::Serialization(format!("json serialize: {e}"))
+        crate::error::PinakesError::Serialization(format!(
+          "json serialize: {e}"
+        ))
       })?;
       std::fs::write(destination, json)?;
     },

crates/pinakes-core/src/metadata/mod.rs

@@ -6,11 +6,7 @@ pub mod video;
 
 use std::{collections::HashMap, path::Path};
 
-use crate::{
+use crate::{error::Result, media_type::MediaType, model::BookMetadata};
-  error::Result,
-  media_type::MediaType,
-  model::BookMetadata,
-};
 
 #[derive(Debug, Clone, Default)]
 pub struct ExtractedMetadata {

crates/pinakes-core/src/plugin/mod.rs

@@ -607,42 +607,12 @@ impl PluginManager {
   pub async fn list_ui_pages(
     &self,
   ) -> Vec<(String, pinakes_plugin_api::UiPage)> {
-    let registry = self.registry.read().await;
+    self
-    let mut pages = Vec::new();
+      .list_ui_pages_with_endpoints()
-    for plugin in registry.list_all() {
+      .await
-      if !plugin.enabled {
+      .into_iter()
-        continue;
+      .map(|(id, page, _)| (id, page))
-      }
+      .collect()
-      let plugin_dir = plugin
-        .manifest_path
-        .as_ref()
-        .and_then(|p| p.parent())
-        .map(std::path::Path::to_path_buf);
-      let Some(plugin_dir) = plugin_dir else {
-        // No manifest path; serve only inline pages.
-        for entry in &plugin.manifest.ui.pages {
-          if let pinakes_plugin_api::manifest::UiPageEntry::Inline(page) = entry
-          {
-            pages.push((plugin.id.clone(), (**page).clone()));
-          }
-        }
-        continue;
-      };
-      match plugin.manifest.load_ui_pages(&plugin_dir) {
-        Ok(loaded) => {
-          for page in loaded {
-            pages.push((plugin.id.clone(), page));
-          }
-        },
-        Err(e) => {
-          tracing::warn!(
-            "Failed to load UI pages for plugin '{}': {e}",
-            plugin.id
-          );
-        },
-      }
-    }
-    pages
   }
 
   /// List all UI pages provided by loaded plugins, including each plugin's

crates/pinakes-core/src/plugin/registry.rs

@@ -131,7 +131,7 @@ impl PluginRegistry {
     self
       .plugins
       .values()
-      .filter(|p| p.manifest.plugin.kind.contains(&kind.to_string()))
+      .filter(|p| p.manifest.plugin.kind.iter().any(|k| k == kind))
       .collect()
   }
 

crates/pinakes-core/src/storage/sqlite.rs

@@ -1888,8 +1888,10 @@ impl StorageBackend for SqliteBackend {
           .unchecked_transaction()
           .map_err(crate::error::db_ctx("batch_tag_media", &ctx))?;
         // Prepare statement once for reuse
-        let mut stmt = tx.prepare_cached(
+        let mut stmt = tx
-          "INSERT OR IGNORE INTO media_tags (media_id, tag_id) VALUES (?1, ?2)",
+          .prepare_cached(
+            "INSERT OR IGNORE INTO media_tags (media_id, tag_id) VALUES (?1, \
+             ?2)",
           )
           .map_err(crate::error::db_ctx("batch_tag_media", &ctx))?;
         let mut count = 0u64;

crates/pinakes-core/tests/integration.rs

@@ -962,7 +962,15 @@ async fn test_batch_update_media_single_field() {
   storage.insert_media(&item).await.unwrap();
 
   let count = storage
-    .batch_update_media(&[item.id], Some("Bulk Title"), None, None, None, None, None)
+    .batch_update_media(
+      &[item.id],
+      Some("Bulk Title"),
+      None,
+      None,
+      None,
+      None,
+      None,
+    )
     .await
     .unwrap();
   assert_eq!(count, 1);
@@ -1021,7 +1029,15 @@ async fn test_batch_update_media_subset_of_items() {
 
   // Only update item_a.
   let count = storage
-    .batch_update_media(&[item_a.id], Some("Only A"), None, None, None, None, None)
+    .batch_update_media(
+      &[item_a.id],
+      Some("Only A"),
+      None,
+      None,
+      None,
+      None,
+      None,
+    )
     .await
     .unwrap();
   assert_eq!(count, 1);

crates/pinakes-plugin-api/src/manifest.rs

@@ -759,11 +759,19 @@ wasm = "plugin.wasm"
 
     let manifest = PluginManifest::parse_str(toml).unwrap();
     assert_eq!(
-      manifest.ui.theme_extensions.get("--accent-color").map(String::as_str),
+      manifest
+        .ui
+        .theme_extensions
+        .get("--accent-color")
+        .map(String::as_str),
       Some("#ff6b6b")
     );
     assert_eq!(
-      manifest.ui.theme_extensions.get("--sidebar-width").map(String::as_str),
+      manifest
+        .ui
+        .theme_extensions
+        .get("--sidebar-width")
+        .map(String::as_str),
       Some("280px")
     );
   }

crates/pinakes-plugin-api/src/ui_schema.rs

@@ -275,11 +275,6 @@ impl UiWidget {
   ///
   /// Returns `SchemaError::ValidationError` if validation fails
   pub fn validate(&self) -> SchemaResult<()> {
-    if self.id.is_empty() {
-      return Err(SchemaError::ValidationError(
-        "Widget id cannot be empty".to_string(),
-      ));
-    }
     if self.target.is_empty() {
       return Err(SchemaError::ValidationError(
         "Widget target cannot be empty".to_string(),

crates/pinakes-plugin-api/src/validation.rs

@@ -331,7 +331,9 @@ impl SchemaValidator {
 
   pub(crate) fn is_reserved_route(route: &str) -> bool {
     RESERVED_ROUTES.iter().any(|reserved| {
-      route == *reserved || route.starts_with(&format!("{reserved}/"))
+      route == *reserved
+        || (route.starts_with(reserved)
+          && route.as_bytes().get(reserved.len()) == Some(&b'/'))
     })
   }
 }

crates/pinakes-server/src/dto/media.rs

@@ -15,7 +15,8 @@ pub fn relativize_path(full_path: &Path, roots: &[PathBuf]) -> String {
   let mut best: Option<&PathBuf> = None;
   for root in roots {
     if full_path.starts_with(root) {
-      let is_longer = best.map_or(true, |b| root.components().count() > b.components().count());
+      let is_longer = best
+        .map_or(true, |b| root.components().count() > b.components().count());
       if is_longer {
         best = Some(root);
       }
@@ -268,10 +269,7 @@ impl MediaResponse {
   /// matching root prefix from the path before serialization. Pass the
   /// configured root directories so that clients receive a relative path
   /// (e.g. `"Music/song.mp3"`) rather than a full server filesystem path.
-  pub fn new(
+  pub fn new(item: pinakes_core::model::MediaItem, roots: &[PathBuf]) -> Self {
-    item: pinakes_core::model::MediaItem,
-    roots: &[PathBuf],
-  ) -> Self {
     Self {
       id:            item.id.0.to_string(),
       path:          relativize_path(&item.path, roots),
@@ -358,10 +356,7 @@ mod tests {
   #[test]
   fn relativize_path_empty_roots_returns_full() {
     let path = Path::new("/home/user/music/song.mp3");
-    assert_eq!(
+    assert_eq!(relativize_path(path, &[]), "/home/user/music/song.mp3");
-      relativize_path(path, &[]),
-      "/home/user/music/song.mp3"
-    );
   }
 
   #[test]

crates/pinakes-server/src/routes/books.rs

@@ -195,8 +195,10 @@ pub async fn list_books(
     .await?;
 
   let roots = state.config.read().await.directories.roots.clone();
-  let response: Vec<MediaResponse> =
+  let response: Vec<MediaResponse> = items
-    items.into_iter().map(|item| MediaResponse::new(item, &roots)).collect();
+    .into_iter()
+    .map(|item| MediaResponse::new(item, &roots))
+    .collect();
   Ok(Json(response))
 }
 
@@ -225,8 +227,10 @@ pub async fn get_series_books(
 ) -> Result<impl IntoResponse, ApiError> {
   let items = state.storage.get_series_books(&series_name).await?;
   let roots = state.config.read().await.directories.roots.clone();
-  let response: Vec<MediaResponse> =
+  let response: Vec<MediaResponse> = items
-    items.into_iter().map(|item| MediaResponse::new(item, &roots)).collect();
+    .into_iter()
+    .map(|item| MediaResponse::new(item, &roots))
+    .collect();
   Ok(Json(response))
 }
 
@@ -261,8 +265,10 @@ pub async fn get_author_books(
     .await?;
 
   let roots = state.config.read().await.directories.roots.clone();
-  let response: Vec<MediaResponse> =
+  let response: Vec<MediaResponse> = items
-    items.into_iter().map(|item| MediaResponse::new(item, &roots)).collect();
+    .into_iter()
+    .map(|item| MediaResponse::new(item, &roots))
+    .collect();
   Ok(Json(response))
 }
 
@@ -321,8 +327,10 @@ pub async fn get_reading_list(
     .await?;
 
   let roots = state.config.read().await.directories.roots.clone();
-  let response: Vec<MediaResponse> =
+  let response: Vec<MediaResponse> = items
-    items.into_iter().map(|item| MediaResponse::new(item, &roots)).collect();
+    .into_iter()
+    .map(|item| MediaResponse::new(item, &roots))
+    .collect();
   Ok(Json(response))
 }
 

crates/pinakes-server/src/routes/plugins.rs

@@ -153,10 +153,12 @@ pub async fn list_plugin_ui_pages(
   let pages = plugin_manager.list_ui_pages_with_endpoints().await;
   let entries = pages
     .into_iter()
-    .map(|(plugin_id, page, allowed_endpoints)| PluginUiPageEntry {
+    .map(|(plugin_id, page, allowed_endpoints)| {
+      PluginUiPageEntry {
         plugin_id,
         page,
         allowed_endpoints,
+      }
     })
     .collect();
   Ok(Json(entries))

crates/pinakes-ui/src/plugin_ui/actions.rs

@@ -96,14 +96,11 @@ async fn execute_inline_action(
   action: &ActionDefinition,
   form_data: Option<&serde_json::Value>,
 ) -> Result<ActionResult, String> {
-  // Build URL from path
-  let url = action.path.clone();
-
   // Merge action params with form data into query string for GET, body for
   // others
   let method = to_reqwest_method(&action.method);
 
-  let mut request = client.raw_request(method.clone(), &url);
+  let mut request = client.raw_request(method.clone(), &action.path);
 
   // For GET, merge params into query string; for mutating methods, send as
   // JSON body

crates/pinakes-ui/src/plugin_ui/data.rs

@@ -2,7 +2,10 @@
 //!
 //! Provides data fetching and caching for plugin data sources.
 
-use std::{collections::HashMap, time::Duration};
+use std::{
+  collections::{HashMap, HashSet},
+  time::Duration,
+};
 
 use dioxus::prelude::*;
 use dioxus_core::Task;
@@ -15,7 +18,7 @@ use crate::client::ApiClient;
 #[derive(Debug, Clone, Default, PartialEq, Eq)]
 pub struct PluginPageData {
   data:    HashMap<String, serde_json::Value>,
-  loading: HashMap<String, bool>,
+  loading: HashSet<String>,
   errors:  HashMap<String, String>,
 }
 
@@ -29,13 +32,13 @@ impl PluginPageData {
   /// Check if a source is currently loading
   #[must_use]
   pub fn is_loading(&self, source: &str) -> bool {
-    self.loading.get(source).copied().unwrap_or(false)
+    self.loading.contains(source)
   }
 
   /// Get error for a specific source
   #[must_use]
-  pub fn error(&self, source: &str) -> Option<&String> {
+  pub fn error(&self, source: &str) -> Option<&str> {
-    self.errors.get(source)
+    self.errors.get(source).map(String::as_str)
   }
 
   /// Check if there is data for a specific source
@@ -52,7 +55,7 @@ impl PluginPageData {
   /// Set loading state for a source
   pub fn set_loading(&mut self, source: &str, loading: bool) {
     if loading {
-      self.loading.insert(source.to_string(), true);
+      self.loading.insert(source.to_string());
       self.errors.remove(source);
     } else {
       self.loading.remove(source);
@@ -161,9 +164,10 @@ async fn fetch_endpoint(
 ///
 /// Endpoint sources are deduplicated by `(path, method, params)`: if multiple
 /// sources share the same triplet, a single HTTP request is made and the raw
-/// response is shared, with each source's own `transform` applied independently.
+/// response is shared, with each source's own `transform` applied
-/// All unique Endpoint and Static sources are fetched concurrently. Transform
+/// independently. All unique Endpoint and Static sources are fetched
-/// sources are applied after, in iteration order, against the full result set.
+/// concurrently. Transform sources are applied after, in iteration order,
+/// against the full result set.
 ///
 /// # Errors
 ///
@@ -263,7 +267,14 @@ pub async fn fetch_page_data(
             ..
           } => {
             let empty_ctx = serde_json::json!({});
-            fetch_endpoint(&client, path, method.clone(), params, &empty_ctx, &allowed)
+            fetch_endpoint(
+              &client,
+              path,
+              method.clone(),
+              params,
+              &empty_ctx,
+              &allowed,
+            )
             .await?
           },
           DataSource::Static { value } => value.clone(),
@@ -296,21 +307,60 @@ pub async fn fetch_page_data(
     }
   }
 
-  // Process Transform sources sequentially; they reference results above.
+  // Process Transform sources in dependency order. HashMap iteration order is
-  for (name, source) in data_sources {
+  // non-deterministic, so a Transform referencing another Transform could see
-    if let DataSource::Transform {
+  // null if the upstream was not yet resolved. The pending loop below defers
+  // any Transform whose upstream is not yet in results, making progress on
+  // each pass until all are resolved. UiPage::validate guarantees no cycles,
+  // so the loop always terminates.
+  let mut pending: Vec<(&String, &String, &Expression)> = data_sources
+    .iter()
+    .filter_map(|(name, source)| {
+      match source {
+        DataSource::Transform {
           source_name,
           expression,
-    } = source
+        } => Some((name, source_name, expression)),
-    {
+        _ => None,
+      }
+    })
+    .collect();
+
+  while !pending.is_empty() {
+    let prev_len = pending.len();
+    let mut i = 0;
+    while i < pending.len() {
+      let (name, source_name, expression) = pending[i];
+      if results.contains_key(source_name.as_str()) {
         let ctx = serde_json::Value::Object(
           results
             .iter()
-          .map(|(k, v): (&String, &serde_json::Value)| (k.clone(), v.clone()))
+            .map(|(k, v)| (k.clone(), v.clone()))
             .collect(),
         );
-      let _ = source_name; // accessible in ctx by its key
         results.insert(name.clone(), evaluate_expression(expression, &ctx));
+        pending.swap_remove(i);
+      } else {
+        i += 1;
+      }
+    }
+    if pending.len() == prev_len {
+      // No progress: upstream source is missing (should be caught by
+      // UiPage::validate, but handled defensively here).
+      tracing::warn!(
+        "plugin transform dependency unresolvable; processing remaining in \
+         iteration order"
+      );
+      for (name, _, expression) in pending {
+        let ctx = serde_json::Value::Object(
+          results
+            .iter()
+            .map(|(k, v)| (k.clone(), v.clone()))
+            .collect(),
+        );
+        results.insert(name.clone(), evaluate_expression(expression, &ctx));
+      }
+      break;
     }
   }
 
@@ -446,7 +496,7 @@ mod tests {
 
     // Test error state
     data.set_error("error".to_string(), "oops".to_string());
-    assert_eq!(data.error("error"), Some(&"oops".to_string()));
+    assert_eq!(data.error("error"), Some("oops"));
   }
 
   #[test]
@@ -522,7 +572,9 @@ mod tests {
       value: serde_json::json!(true),
     });
 
-    let results = super::fetch_page_data(&client, &sources, &[]).await.unwrap();
+    let results = super::fetch_page_data(&client, &sources, &[])
+      .await
+      .unwrap();
     assert_eq!(results["nums"], serde_json::json!([1, 2, 3]));
     assert_eq!(results["flag"], serde_json::json!(true));
   }
@@ -544,7 +596,9 @@ mod tests {
       value: serde_json::json!({"ok": true}),
     });
 
-    let results = super::fetch_page_data(&client, &sources, &[]).await.unwrap();
+    let results = super::fetch_page_data(&client, &sources, &[])
+      .await
+      .unwrap();
     assert_eq!(results["raw"], serde_json::json!({"ok": true}));
     // derived should return the value of "raw" from context
     assert_eq!(results["derived"], serde_json::json!({"ok": true}));
@@ -566,13 +620,13 @@ mod tests {
       expression:  Expression::Literal(serde_json::json!("constant")),
     });
 
-    let results = super::fetch_page_data(&client, &sources, &[]).await.unwrap();
+    let results = super::fetch_page_data(&client, &sources, &[])
+      .await
+      .unwrap();
     // A Literal expression returns the literal value, not the source data
     assert_eq!(results["derived"], serde_json::json!("constant"));
   }
 
-  // Test: multiple Static sources with the same value each get their own
-  // result; dedup logic does not collapse distinct-named Static sources.
   #[tokio::test]
   async fn test_fetch_page_data_deduplicates_identical_endpoints() {
     use pinakes_plugin_api::DataSource;
@@ -589,18 +643,18 @@ mod tests {
     sources.insert("b".to_string(), DataSource::Static {
       value: serde_json::json!(1),
     });
-    let results = super::fetch_page_data(&client, &sources, &[]).await.unwrap();
+    let results = super::fetch_page_data(&client, &sources, &[])
+      .await
+      .unwrap();
     assert_eq!(results["a"], serde_json::json!(1));
     assert_eq!(results["b"], serde_json::json!(1));
     assert_eq!(results.len(), 2);
   }
 
-  // Test: Endpoint sources with identical (path, method, params) but different
+  // Verifies that endpoint sources with identical (path, method, params) are
-  // transform expressions each get a correctly transformed result.  Because the
+  // deduplicated correctly. Because there is no real server, the allowlist
-  // test runs without a real server the path is checked against the allowlist
+  // rejection fires before any network call; both names seeing the same error
-  // before any network call, so we verify the dedup key grouping through the
+  // proves they were grouped and that the single rejection propagated to all.
-  // allowlist rejection path: both names should see the same error message,
-  // proving they were grouped and the single rejection propagates to all names.
   #[tokio::test]
   async fn test_dedup_groups_endpoint_sources_with_same_key() {
     use pinakes_plugin_api::{DataSource, Expression, HttpMethod};
@@ -640,14 +694,12 @@ mod tests {
     );
   }
 
-  // Test: multiple Transform sources referencing the same upstream Static source
+  // Verifies the transform fan-out behavior: each member of a dedup group
-  // with different expressions each receive their independently transformed
+  // applies its own transform to the shared raw value independently. This
-  // result. This exercises the transform fan-out behavior that mirrors what
+  // mirrors what Endpoint dedup does after a single shared HTTP request.
-  // the Endpoint dedup group does after a single shared HTTP request completes:
-  // each member of a group applies its own transform to the shared raw value.
   //
-  // Testing the Endpoint dedup success path with real per-member transforms
+  // Testing Endpoint dedup with real per-member transforms requires a mock HTTP
-  // requires a mock HTTP server and belongs in an integration test.
+  // server and belongs in an integration test.
   #[tokio::test]
   async fn test_dedup_transform_applied_per_source() {
     use pinakes_plugin_api::{DataSource, Expression};
@@ -670,8 +722,9 @@ mod tests {
       expression:  Expression::Path("raw_data.name".to_string()),
     });
 
-    let results =
+    let results = super::fetch_page_data(&client, &sources, &[])
-      super::fetch_page_data(&client, &sources, &[]).await.unwrap();
+      .await
+      .unwrap();
     assert_eq!(
       results["raw_data"],
       serde_json::json!({"count": 42, "name": "test"})
@@ -681,8 +734,6 @@ mod tests {
     assert_eq!(results.len(), 3);
   }
 
-  // Test: fetch_page_data returns an error when the endpoint data source path is
-  // not listed in the allowed_endpoints slice.
   #[tokio::test]
   async fn test_endpoint_blocked_when_not_in_allowlist() {
     use pinakes_plugin_api::{DataSource, HttpMethod};
@@ -705,7 +756,8 @@ mod tests {
 
     assert!(
       result.is_err(),
-      "fetch_page_data must return Err when endpoint is not in allowed_endpoints"
+      "fetch_page_data must return Err when endpoint is not in \
+       allowed_endpoints"
     );
     let msg = result.unwrap_err();
     assert!(

crates/pinakes-ui/src/plugin_ui/registry.rs

@@ -35,13 +35,6 @@ pub struct PluginPage {
   pub allowed_endpoints: Vec<String>,
 }
 
-impl PluginPage {
-  /// The canonical route for this page, taken directly from the page schema.
-  pub fn full_route(&self) -> String {
-    self.page.route.clone()
-  }
-}
-
 /// Registry of all plugin-provided UI pages and widgets
 ///
 /// This is typically stored as a signal in the Dioxus tree.
@@ -109,14 +102,11 @@ impl PluginRegistry {
       );
       return;
     }
-    self.pages.insert(
+    self.pages.insert((plugin_id.clone(), page_id), PluginPage {
-      (plugin_id.clone(), page_id),
-      PluginPage {
       plugin_id,
       page,
       allowed_endpoints,
-      },
+    });
-    );
   }
 
   /// Get a specific page by plugin ID and page ID
@@ -179,7 +169,7 @@ impl PluginRegistry {
     self
       .pages
       .values()
-      .map(|p| (p.plugin_id.clone(), p.page.id.clone(), p.full_route()))
+      .map(|p| (p.plugin_id.clone(), p.page.id.clone(), p.page.route.clone()))
       .collect()
   }
 
@@ -207,7 +197,9 @@ impl PluginRegistry {
     }
     match self.client.get_plugin_ui_theme_extensions().await {
       Ok(vars) => tmp.theme_vars = vars,
-      Err(e) => tracing::warn!("Failed to refresh plugin theme extensions: {e}"),
+      Err(e) => {
+        tracing::warn!("Failed to refresh plugin theme extensions: {e}")
+      },
     }
 
     // Atomic swap: no window where the registry appears empty.
@@ -367,7 +359,7 @@ mod tests {
   }
 
   #[test]
-  fn test_page_full_route() {
+  fn test_page_route() {
     let client = ApiClient::default();
     let mut registry = PluginRegistry::new(client);
     registry.register_page(
@@ -376,9 +368,7 @@ mod tests {
       vec![],
     );
     let plugin_page = registry.get_page("my-plugin", "demo").unwrap();
-    // full_route() returns page.route directly; create_test_page sets it as
+    assert_eq!(plugin_page.page.route, "/plugins/test/demo");
-    // "/plugins/test/{id}"
-    assert_eq!(plugin_page.full_route(), "/plugins/test/demo");
   }
 
   #[test]
@@ -418,8 +408,16 @@ mod tests {
   fn test_all_pages_returns_references() {
     let client = ApiClient::default();
     let mut registry = PluginRegistry::new(client);
-    registry.register_page("p1".to_string(), create_test_page("a", "A"), vec![]);
+    registry.register_page(
-    registry.register_page("p2".to_string(), create_test_page("b", "B"), vec![]);
+      "p1".to_string(),
+      create_test_page("a", "A"),
+      vec![],
+    );
+    registry.register_page(
+      "p2".to_string(),
+      create_test_page("b", "B"),
+      vec![],
+    );
 
     let pages = registry.all_pages();
     assert_eq!(pages.len(), 2);
@@ -536,7 +534,11 @@ mod tests {
     assert_eq!(registry.all_pages().len(), 0);
 
     // Valid page; should still register fine
-    registry.register_page("p".to_string(), create_test_page("good", "Good"), vec![]);
+    registry.register_page(
+      "p".to_string(),
+      create_test_page("good", "Good"),
+      vec![],
+    );
     assert_eq!(registry.all_pages().len(), 1);
   }
 

crates/pinakes-ui/src/plugin_ui/renderer.rs

@@ -110,8 +110,12 @@ pub fn PluginViewRenderer(props: PluginViewProps) -> Element {
     modal,
     local_state,
   };
-  let page_data =
+  let page_data = use_plugin_data(
-    use_plugin_data(props.client, data_sources, refresh, props.allowed_endpoints);
+    props.client,
+    data_sources,
+    refresh,
+    props.allowed_endpoints,
+  );
 
   // Consume pending navigation requests and forward to the parent
   use_effect(move || {
@@ -151,7 +155,7 @@ pub fn PluginViewRenderer(props: PluginViewProps) -> Element {
                           onclick: move |_| modal.set(None),
                           "×"
                       }
-                      { render_element(&elem, &page_data.read(), &HashMap::new(), ctx) }
+                      { render_element(&elem, &page_data.read(), &actions, ctx) }
                   }
               }
           }
@@ -318,43 +322,36 @@ fn PluginDataTable(props: PluginDataTableProps) -> Element {
                               let row_val = row;
                               rsx! {
                                   tr {
-                                      for col in props.columns.clone() {
+                                      for col in &props.columns {
                                           td { "{extract_cell(&row_val, &col.key)}" }
                                       }
                                       if !props.row_actions.is_empty() {
                                           td { class: "row-actions",
-                                              for act in props.row_actions.clone() {
+                                              for act in &props.row_actions {
                                                   {
                                                       let action = act.action.clone();
                                                       let row_data = row_val.clone();
                                                       let variant_class =
                                                           button_variant_class(&act.variant);
                                                       let page_actions = props.actions.clone();
-                                                      let success_msg: Option<String> =
+                                                      let (success_msg, error_msg): (
-                                                          match &act.action {
+                                                          Option<String>,
-                                                              ActionRef::Special(_) => None,
+                                                          Option<String>,
+                                                      ) = match &act.action {
+                                                          ActionRef::Special(_) => (None, None),
                                                           ActionRef::Name(name) => props
                                                             .actions
                                                             .get(name)
-                                                                  .and_then(|a| {
+                                                            .map_or((None, None), |a| {
-                                                                      a.success_message.clone()
+                                                              (
+                                                                a.success_message.clone(),
+                                                                a.error_message.clone(),
+                                                              )
                                                             }),
-                                                              ActionRef::Inline(a) => {
+                                                          ActionRef::Inline(a) => (
-                                                                  a.success_message.clone()
+                                                            a.success_message.clone(),
-                                                              },
+                                                            a.error_message.clone(),
-                                                          };
+                                                          ),
-                                                      let error_msg: Option<String> =
-                                                          match &act.action {
-                                                              ActionRef::Special(_) => None,
-                                                              ActionRef::Name(name) => props
-                                                                  .actions
-                                                                  .get(name)
-                                                                  .and_then(|a| {
-                                                                      a.error_message.clone()
-                                                                  }),
-                                                              ActionRef::Inline(a) => {
-                                                                  a.error_message.clone()
-                                                              },
                                                       };
                                                       let ctx = props.ctx;
                                                       // Pre-compute data JSON at render time to
@@ -489,7 +486,8 @@ pub fn render_element(
         || "0".to_string(),
         |p| format!("{}px {}px {}px {}px", p[0], p[1], p[2], p[3]),
       );
-      let style = format!("--plugin-gap:{gap}px;--plugin-padding:{padding_css};");
+      let style =
+        format!("--plugin-gap:{gap}px;--plugin-padding:{padding_css};");
       rsx! {
           div {
               class: "plugin-container",
@@ -829,19 +827,17 @@ pub fn render_element(
       let variant_class = button_variant_class(variant);
       let action_ref = action.clone();
       let page_actions = actions.clone();
-      let success_msg: Option<String> = match action {
+      let (success_msg, error_msg): (Option<String>, Option<String>) =
-        ActionRef::Special(_) => None,
+        match action {
+          ActionRef::Special(_) => (None, None),
           ActionRef::Name(name) => {
-          actions.get(name).and_then(|a| a.success_message.clone())
+            actions.get(name).map_or((None, None), |a| {
+              (a.success_message.clone(), a.error_message.clone())
+            })
           },
-        ActionRef::Inline(a) => a.success_message.clone(),
+          ActionRef::Inline(a) => {
-      };
+            (a.success_message.clone(), a.error_message.clone())
-      let error_msg: Option<String> = match action {
-        ActionRef::Special(_) => None,
-        ActionRef::Name(name) => {
-          actions.get(name).and_then(|a| a.error_message.clone())
           },
-        ActionRef::Inline(a) => a.error_message.clone(),
         };
       let data_snapshot = build_ctx(data, &ctx.local_state.read());
       rsx! {
@@ -904,19 +900,17 @@ pub fn render_element(
     } => {
       let action_ref = submit_action.clone();
       let page_actions = actions.clone();
-      let success_msg: Option<String> = match submit_action {
+      let (success_msg, error_msg): (Option<String>, Option<String>) =
-        ActionRef::Special(_) => None,
+        match submit_action {
+          ActionRef::Special(_) => (None, None),
           ActionRef::Name(name) => {
-          actions.get(name).and_then(|a| a.success_message.clone())
+            actions.get(name).map_or((None, None), |a| {
+              (a.success_message.clone(), a.error_message.clone())
+            })
           },
-        ActionRef::Inline(a) => a.success_message.clone(),
+          ActionRef::Inline(a) => {
-      };
+            (a.success_message.clone(), a.error_message.clone())
-      let error_msg: Option<String> = match submit_action {
-        ActionRef::Special(_) => None,
-        ActionRef::Name(name) => {
-          actions.get(name).and_then(|a| a.error_message.clone())
           },
-        ActionRef::Inline(a) => a.error_message.clone(),
         };
       let data_snapshot = build_ctx(data, &ctx.local_state.read());
       rsx! {
@@ -1096,8 +1090,6 @@ pub fn render_element(
     } => {
       let chart_class = chart_type_class(chart_type);
       let chart_data = data.get(source_key).cloned();
-      let x_label = x_axis_label.as_deref().unwrap_or("").to_string();
-      let y_label = y_axis_label.as_deref().unwrap_or("").to_string();
       rsx! {
           div {
               class: "plugin-chart {chart_class}",
@@ -1111,7 +1103,7 @@ pub fn render_element(
                   if let Some(x) = x_axis_label { div { class: "chart-x-label", "{x}" } }
                   if let Some(y) = y_axis_label { div { class: "chart-y-label", "{y}" } }
                   div { class: "chart-data-table",
-                      { render_chart_data(chart_data.as_ref(), &x_label, &y_label) }
+                      { render_chart_data(chart_data.as_ref(), x_axis_label.as_deref().unwrap_or(""), y_axis_label.as_deref().unwrap_or("")) }
                   }
               }
           }