diff --git a/AGENTS.md b/AGENTS.md index ef71bbe..bdbfbc2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -41,6 +41,9 @@ These instructions apply to the entire repository. ## Rust workspace +- Stable resources and components use typed IDs. Collection indices are + one-shot lookup positions only; they must not cross action, job, frame, or + persistence boundaries. - Respect crate boundaries: parsing in `plotx-io`, scientific algorithms in `plotx-analysis`, spectral transforms in `plotx-processing`, presentation models in `plotx-figure`, rendering in `plotx-render`, application state in diff --git a/Cargo.lock b/Cargo.lock index 9f7f7c2..6a14da4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4166,6 +4166,7 @@ dependencies = [ "plotx-analysis", "plotx-io", "rustfft", + "serde", "thiserror 2.0.18", ] diff --git a/crates/app/src/shot.rs b/crates/app/src/shot.rs index a03ea4b..4bfe2e0 100644 --- a/crates/app/src/shot.rs +++ b/crates/app/src/shot.rs @@ -333,8 +333,8 @@ fn setup(app: &mut PlotxApp) { && let Some(object) = app.doc.canvases[ci].active_plot_object_id() { app.session.ui.analysis_selection = Some(AnalysisSelection { - dataset: 0, - canvas: ci, + dataset: app.doc.datasets[0].resource_id(), + canvas: app.doc.canvases[ci].resource_id, object, x_range: AxisRange::new(FIT_LO, FIT_HI), y_range: None, diff --git a/crates/app/src/ui/batch_workflow.rs b/crates/app/src/ui/batch_workflow.rs index 644c884..f1144ae 100644 --- a/crates/app/src/ui/batch_workflow.rs +++ b/crates/app/src/ui/batch_workflow.rs @@ -402,7 +402,7 @@ impl AutomationUi { .and_then(|index| app.doc.canvases.get(index)) { let target = plotx_core::automation::ResourceRef { - id: canvas.resource_id.clone(), + id: canvas.resource_id.to_string(), kind: plotx_core::automation::ResourceKindId::new( plotx_core::automation::KIND_CANVAS, ), @@ -424,7 +424,7 @@ fn highlight(app: &mut PlotxApp, id: &str) { .doc .datasets .iter() - .position(|dataset| dataset.resource_id() == id) + .position(|dataset| dataset.resource_id().to_string() == id) { app.set_active_dataset(Some(index)); } @@ -432,7 +432,7 @@ fn highlight(app: &mut PlotxApp, id: &str) { .doc .canvases .iter() - .position(|canvas| canvas.resource_id == id) + .position(|canvas| canvas.resource_id.to_string() == id) { app.session.active_canvas = Some(index); app.sync_selection_to_active_canvas(); diff --git a/crates/app/src/ui/canvas/board.rs b/crates/app/src/ui/canvas/board.rs index f3f9a82..2260a70 100644 --- a/crates/app/src/ui/canvas/board.rs +++ b/crates/app/src/ui/canvas/board.rs @@ -485,8 +485,10 @@ fn activate_frame(app: &mut PlotxApp, frame: FrameRef) { FrameRef::Page(ci) => { activate_page(app, ci); if let Some(canvas) = app.doc.canvases.get(ci) { - let datasets = canvas.dataset_indices(); - let lead = canvas.active_dataset(); + let lead = canvas + .active_dataset() + .and_then(|id| app.doc.dataset_index(id)); + let datasets = app.doc.page_dataset_indices(ci); app.focus_datasets(&datasets, lead); } } diff --git a/crates/app/src/ui/canvas/geometry.rs b/crates/app/src/ui/canvas/geometry.rs index e096638..173a77a 100644 --- a/crates/app/src/ui/canvas/geometry.rs +++ b/crates/app/src/ui/canvas/geometry.rs @@ -394,7 +394,8 @@ mod tests { use super::*; use plotx_core::state::TextBox; - fn text_object(id: ObjectId, frame: ObjectFrame) -> CanvasObject { + fn text_object(id: u64, frame: ObjectFrame) -> CanvasObject { + let id = ObjectId::new(id); CanvasObject { id, name: format!("o{id}"), @@ -461,7 +462,7 @@ mod tests { zoom: 2.0, }; let page = bt.page_screen_rect(&canvas); - let r = bt.object_screen_rect(&canvas, 5).unwrap(); + let r = bt.object_screen_rect(&canvas, ObjectId::new(5)).unwrap(); assert!((r.left - (page.left() + 12.0 * 2.0)).abs() < 1e-3); assert!((r.top - (page.top() + 8.0 * 2.0)).abs() < 1e-3); assert!((r.width - 40.0 * 2.0).abs() < 1e-3); @@ -615,6 +616,6 @@ mod tests { .objects .push(text_object(2, ObjectFrame::new(20.0, 20.0, 50.0, 50.0))); let hit = hit_object(&canvas, Pos2::new(35.0, 35.0), 1.0).unwrap(); - assert_eq!(hit.object, 2); + assert_eq!(hit.object, ObjectId::new(2)); } } diff --git a/crates/app/src/ui/canvas/interactions.rs b/crates/app/src/ui/canvas/interactions.rs index 0c8edb2..9bbbaae 100644 --- a/crates/app/src/ui/canvas/interactions.rs +++ b/crates/app/src/ui/canvas/interactions.rs @@ -110,8 +110,8 @@ pub(crate) fn finish_selection_drag( }); app.session.ui.analysis_selection = Some(AnalysisSelection { - dataset, - canvas: ci, + dataset: app.doc.datasets[dataset].resource_id(), + canvas: app.doc.canvases[ci].resource_id, object: object_id, x_range: x, y_range: y, @@ -481,8 +481,12 @@ fn select_object_datasets(app: &mut PlotxApp, ci: usize, id: ObjectId) { let Some(object) = app.doc.canvases[ci].object(id) else { return; }; - let active = object.dataset(); - let datasets = object.dataset_indices(); + let active = object.dataset().and_then(|id| app.doc.dataset_index(id)); + let datasets = object + .dataset_ids() + .into_iter() + .filter_map(|id| app.doc.dataset_index(id)) + .collect::>(); if !datasets.is_empty() { app.focus_datasets(&datasets, active); } else { diff --git a/crates/app/src/ui/canvas/mod.rs b/crates/app/src/ui/canvas/mod.rs index 954a7d0..4830bb6 100644 --- a/crates/app/src/ui/canvas/mod.rs +++ b/crates/app/src/ui/canvas/mod.rs @@ -248,6 +248,7 @@ pub fn render_central(app: &mut PlotxApp, ui: &mut Ui) { let Some(di) = app.doc.canvases[ci] .object(object_id) .and_then(|object| object.dataset()) + .and_then(|id| app.doc.dataset_index(id)) else { return; }; @@ -557,7 +558,7 @@ fn resize_cursor(handle: ResizeHandle) -> egui::CursorIcon { mod tests { use super::*; use plotx_core::state::{ - CanvasObject, CanvasObjectKind, CanvasViewport, PanelMeta, PlotObject, TextBox, + CanvasObject, CanvasObjectKind, CanvasViewport, DatasetId, PanelMeta, PlotObject, TextBox, }; use plotx_figure::{Axis, Figure}; @@ -565,7 +566,7 @@ mod tests { fn hit_object_selects_text_box() { let mut canvas = CanvasDocument::new("page".to_owned(), [200.0, 200.0]); canvas.objects.push(CanvasObject { - id: 7, + id: ObjectId::new(7), name: "Text".to_owned(), frame: ObjectFrame::new(20.0, 20.0, 100.0, 30.0), locked: false, @@ -576,21 +577,21 @@ mod tests { let hit = hit_object(&canvas, Pos2::new(50.0, 30.0), 1.0); - assert_eq!(hit.map(|hit| hit.object), Some(7)); + assert_eq!(hit.map(|hit| hit.object), Some(ObjectId::new(7))); } #[test] fn hit_object_finds_object_outside_page_bounds() { let mut canvas = CanvasDocument::new("page".to_owned(), [100.0, 100.0]); canvas.objects.push(CanvasObject { - id: 1, + id: ObjectId::new(1), name: "plot".to_owned(), frame: ObjectFrame::new(-30.0, 20.0, 50.0, 40.0), locked: false, visible: true, group: None, kind: CanvasObjectKind::Plot(Box::new(PlotObject { - binding: plotx_core::state::DataBinding::single(0), + binding: plotx_core::state::DataBinding::single(DatasetId::new()), chart: plotx_core::state::ChartSpec::default(), stack: plotx_core::state::StackSpec::default(), projections: plotx_core::state::AxisProjections::default(), @@ -607,7 +608,7 @@ mod tests { let hit = hit_object(&canvas, Pos2::new(-10.0, 30.0), 1.0); - assert_eq!(hit.map(|hit| hit.object), Some(1)); + assert_eq!(hit.map(|hit| hit.object), Some(ObjectId::new(1))); } #[test] @@ -615,14 +616,14 @@ mod tests { let mut app = PlotxApp::new(); let mut canvas = CanvasDocument::new("page".to_owned(), [200.0, 200.0]); canvas.objects.push(CanvasObject { - id: 3, + id: ObjectId::new(3), name: "plot".to_owned(), frame: ObjectFrame::new(10.0, 10.0, 80.0, 60.0), locked: false, visible: true, group: None, kind: CanvasObjectKind::Plot(Box::new(PlotObject { - binding: plotx_core::state::DataBinding::single(0), + binding: plotx_core::state::DataBinding::single(DatasetId::new()), chart: plotx_core::state::ChartSpec::default(), stack: plotx_core::state::StackSpec::default(), projections: plotx_core::state::AxisProjections::default(), @@ -638,13 +639,13 @@ mod tests { }); app.doc.canvases.push(canvas); app.session.active_canvas = Some(0); - app.doc.canvases[0].selected_object = Some(3); + app.doc.canvases[0].selected_object = Some(ObjectId::new(3)); app.session.tool = Tool::Select; assert_eq!(data_edit_target(&app, 0), None); app.session.tool = Tool::BrowseZoom; - assert_eq!(data_edit_target(&app, 0), Some(3)); + assert_eq!(data_edit_target(&app, 0), Some(ObjectId::new(3))); } #[test] diff --git a/crates/app/src/ui/canvas/painting.rs b/crates/app/src/ui/canvas/painting.rs index eb97ea7..f0fac9f 100644 --- a/crates/app/src/ui/canvas/painting.rs +++ b/crates/app/src/ui/canvas/painting.rs @@ -84,7 +84,7 @@ pub(crate) fn paint_analysis_selection( let Some(selection) = &app.session.ui.analysis_selection else { return; }; - if selection.canvas != ci || selection.object != object_id { + if selection.canvas != app.doc.canvases[ci].resource_id || selection.object != object_id { return; } let Some(object) = app.doc.canvases[ci].object(object_id) else { diff --git a/crates/app/src/ui/canvas/phase.rs b/crates/app/src/ui/canvas/phase.rs index 40f779f..f1d205f 100644 --- a/crates/app/src/ui/canvas/phase.rs +++ b/crates/app/src/ui/canvas/phase.rs @@ -51,6 +51,7 @@ pub(crate) fn handle_phase_before_paint( let Some(di) = app.doc.canvases[ci] .object(object_id) .and_then(|object| object.dataset()) + .and_then(|id| app.doc.dataset_index(id)) else { return; }; diff --git a/crates/app/src/ui/canvas/tiling.rs b/crates/app/src/ui/canvas/tiling.rs index 5535192..fb5afdf 100644 --- a/crates/app/src/ui/canvas/tiling.rs +++ b/crates/app/src/ui/canvas/tiling.rs @@ -268,10 +268,10 @@ pub(crate) fn paint_tile_preview( mod tests { use super::*; - fn drag(canvas: usize, object: ObjectId) -> ObjectDrag { + fn drag(canvas: usize, object: u64) -> ObjectDrag { ObjectDrag { canvas, - object, + object: ObjectId::new(object), kind: ObjectDragKind::Move, before: ObjectFrame::new(0.0, 0.0, 10.0, 10.0), start_pointer: [0.0; 2], @@ -285,12 +285,14 @@ mod tests { fn tile_cache_identity_tracks_source_region_target_and_existing_order() { let layout = plotx_core::layout::PageLayout::default(); let page = [400.0, 300.0]; + let ids = [ObjectId::new(20), ObjectId::new(21)]; + let reversed_ids = [ObjectId::new(21), ObjectId::new(20)]; let base = tile_cache_key( &drag(0, 10), 2, page, layout, - &[20, 21], + &ids, plotx_core::layout::TilingDropRegion::Left, None, ); @@ -301,7 +303,7 @@ mod tests { 2, page, layout, - &[20, 21], + &ids, plotx_core::layout::TilingDropRegion::Left, None, ) @@ -313,7 +315,7 @@ mod tests { 2, page, layout, - &[20, 21], + &ids, plotx_core::layout::TilingDropRegion::Left, None, ) @@ -325,7 +327,7 @@ mod tests { 3, page, layout, - &[20, 21], + &ids, plotx_core::layout::TilingDropRegion::Left, None, ) @@ -337,7 +339,7 @@ mod tests { 2, [401.0, 300.0], layout, - &[20, 21], + &ids, plotx_core::layout::TilingDropRegion::Left, None, ) @@ -349,7 +351,7 @@ mod tests { 2, page, plotx_core::layout::PageLayout { cols: 2, ..layout }, - &[20, 21], + &ids, plotx_core::layout::TilingDropRegion::Left, None, ) @@ -361,7 +363,7 @@ mod tests { 2, page, layout, - &[20, 21], + &ids, plotx_core::layout::TilingDropRegion::Right, None, ) @@ -373,7 +375,7 @@ mod tests { 2, page, layout, - &[21, 20], + &reversed_ids, plotx_core::layout::TilingDropRegion::Left, None, ) @@ -383,7 +385,7 @@ mod tests { 2, page, layout, - &[20, 21], + &ids, plotx_core::layout::TilingDropRegion::Retile, Some(0), ); @@ -392,7 +394,7 @@ mod tests { 2, page, layout, - &[20, 21], + &ids, plotx_core::layout::TilingDropRegion::Retile, Some(3), ); diff --git a/crates/app/src/ui/canvas_size.rs b/crates/app/src/ui/canvas_size.rs index 18c5c4e..0b9a9b9 100644 --- a/crates/app/src/ui/canvas_size.rs +++ b/crates/app/src/ui/canvas_size.rs @@ -360,7 +360,7 @@ pub(crate) fn page_size_chrome( label = format!("{label} · auto"); } let overflows = content_overflows(canvas); - let resource_id = canvas.resource_id.clone(); + let resource_id = canvas.resource_id.to_string(); let suggestion = wider_preset_suggestion(canvas).filter(|s| !suggestion_dismissed(&ctx, &resource_id, s)); diff --git a/crates/app/src/ui/commands.rs b/crates/app/src/ui/commands.rs index fd64c6c..67835ca 100644 --- a/crates/app/src/ui/commands.rs +++ b/crates/app/src/ui/commands.rs @@ -300,9 +300,9 @@ pub(super) fn chart_plot_target(app: &PlotxApp, dataset: usize) -> Option<(usize continue; }; let hit = canvas.objects.iter().find(|object| { - object - .plot() - .is_some_and(|plot| plot.binding.primary_dataset() == dataset) + object.plot().is_some_and(|plot| { + plot.binding.primary_dataset() == Some(app.doc.datasets[dataset].resource_id()) + }) }); if let Some(object) = hit { return Some((ci, object.id)); diff --git a/crates/app/src/ui/commands_tests.rs b/crates/app/src/ui/commands_tests.rs index 5e7f3b1..3c08cac 100644 --- a/crates/app/src/ui/commands_tests.rs +++ b/crates/app/src/ui/commands_tests.rs @@ -320,8 +320,8 @@ fn transient_state_never_changes_ribbon_group_visibility() { app.session.tool = Tool::Integrate; app.session.ui.peak_column = Some(plotx_core::data::ColumnId::new()); app.session.ui.analysis_selection = Some(AnalysisSelection { - dataset: 0, - canvas, + dataset: app.doc.datasets[0].resource_id(), + canvas: app.doc.canvases[canvas].resource_id, object, x_range: range, y_range: None, diff --git a/crates/app/src/ui/data_sheet/transform.rs b/crates/app/src/ui/data_sheet/transform.rs index bc99417..2804c7c 100644 --- a/crates/app/src/ui/data_sheet/transform.rs +++ b/crates/app/src/ui/data_sheet/transform.rs @@ -1,5 +1,5 @@ use plotx_core::data::{RelPlanV1, SnapshotRead, TableSchema}; -use plotx_core::state::TableEditDelta; +use plotx_core::state::{DatasetId, TableEditDelta}; pub(super) struct TableTransformRequest { pub input_datasets: Vec, @@ -18,7 +18,7 @@ pub(super) struct TableSheetContext<'a> { pub dataset: usize, pub commit: &'a mut Option, pub transform: &'a mut Option, - pub refresh: &'a mut Option<(usize, Vec)>, + pub refresh: &'a mut Option<(usize, Vec)>, pub catalog: &'a [TableCatalogEntry], pub transform_running: bool, } diff --git a/crates/app/src/ui/data_sheet/typed.rs b/crates/app/src/ui/data_sheet/typed.rs index 204b9a2..e678bf4 100644 --- a/crates/app/src/ui/data_sheet/typed.rs +++ b/crates/app/src/ui/data_sheet/typed.rs @@ -3,7 +3,7 @@ //! (RowId, revisions, patches) never appears in the UI. use egui::Ui; -use plotx_core::state::{TableDataset, TableEditDelta}; +use plotx_core::state::{DatasetId, TableDataset, TableEditDelta}; use super::grid::{self, SheetState}; use super::transform::TableSheetContext; @@ -82,7 +82,7 @@ fn toolbar( dataset: usize, running: bool, request: &mut Option, - refresh: &mut Option<(usize, Vec)>, + refresh: &mut Option<(usize, Vec)>, catalog: &[super::transform::TableCatalogEntry], ) { ui.horizontal(|ui| { @@ -99,6 +99,9 @@ fn toolbar( .on_hover_text("Re-run this table's source recipe and keep your cell edits") .clicked() { + // Carry the source DatasetIds through untouched; start_table_refresh + // resolves them and reports any missing source, instead of silently + // dropping unresolved ones here. *refresh = Some((dataset, refresh_sources.unwrap())); } ui.menu_button("Combine", |ui| { diff --git a/crates/app/src/ui/object_inspector.rs b/crates/app/src/ui/object_inspector.rs index 7d35815..87f0b18 100644 --- a/crates/app/src/ui/object_inspector.rs +++ b/crates/app/src/ui/object_inspector.rs @@ -174,8 +174,8 @@ fn data_section(app: &mut PlotxApp, ci: usize, object: ObjectId, ui: &mut Ui) { swatch(ui, color); let name = app .doc - .datasets - .get(sb.dataset) + .dataset_index(sb.dataset) + .and_then(|index| app.doc.datasets.get(index)) .map(Dataset::display_name) .unwrap_or_default(); let label = if i == 0 { @@ -242,10 +242,9 @@ fn data_section(app: &mut PlotxApp, ci: usize, object: ObjectId, ui: &mut Ui) { } let candidates = app.stack_candidates(&binding); - if app - .doc - .datasets - .get(binding.primary_dataset()) + if binding + .primary_dataset() + .and_then(|id| app.doc.dataset_by_id(id)) .map(Dataset::domain) .is_some_and(|d| d.stack_kind().is_some()) { @@ -259,7 +258,8 @@ fn data_section(app: &mut PlotxApp, ci: usize, object: ObjectId, ui: &mut Ui) { let label = app.doc.datasets[*di].display_name(); if ui.selectable_label(false, label).clicked() { let mut b = binding.clone(); - b.series.push(SeriesBinding::new(*di)); + b.series + .push(SeriesBinding::new(app.doc.datasets[*di].resource_id())); next_binding = Some(b); } } @@ -270,10 +270,9 @@ fn data_section(app: &mut PlotxApp, ci: usize, object: ObjectId, ui: &mut Ui) { } if is_stack { - let kind = app - .doc - .datasets - .get(binding.primary_dataset()) + let kind = binding + .primary_dataset() + .and_then(|id| app.doc.dataset_by_id(id)) .and_then(|d| d.domain().stack_kind()); if let Some(kind) = kind { stack_controls(kind, &stack, &mut next_stack, ui); diff --git a/crates/app/src/ui/object_inspector/chart_gallery.rs b/crates/app/src/ui/object_inspector/chart_gallery.rs index 8589842..284a6c2 100644 --- a/crates/app/src/ui/object_inspector/chart_gallery.rs +++ b/crates/app/src/ui/object_inspector/chart_gallery.rs @@ -12,10 +12,14 @@ pub(super) fn chart_gallery(app: &mut PlotxApp, ci: usize, object: ObjectId, ui: return; }; let current = plot.chart.clone(); - let primary = plot.binding.primary_dataset(); - let Some(domain) = app.doc.datasets.get(primary).map(Dataset::domain) else { + let Some(primary) = plot + .binding + .primary_dataset() + .and_then(|id| app.doc.dataset_index(id)) + else { return; }; + let domain = app.doc.datasets[primary].domain(); let types = chart_types_for(domain); let current_id = if chart_type(¤t.type_id).is_some_and(|c| c.domains.contains(&domain)) { current.type_id.clone() diff --git a/crates/app/src/ui/primary_sidebar.rs b/crates/app/src/ui/primary_sidebar.rs index a76b667..a7e4282 100644 --- a/crates/app/src/ui/primary_sidebar.rs +++ b/crates/app/src/ui/primary_sidebar.rs @@ -169,8 +169,10 @@ fn canvas_list(app: &mut PlotxApp, ui: &mut Ui) { plotx_core::state::toggle_frame_selection_synced(app, FrameRef::Page(ci)); } else { app.session.active_canvas = Some(ci); - let datasets = app.doc.canvases[ci].dataset_indices(); - let lead = app.doc.canvases[ci].active_dataset(); + let lead = app.doc.canvases[ci] + .active_dataset() + .and_then(|id| app.doc.dataset_index(id)); + let datasets = app.doc.page_dataset_indices(ci); app.focus_datasets(&datasets, lead); app.sync_selection_to_active_canvas(); app.reset_interaction(); @@ -182,7 +184,9 @@ fn canvas_list(app: &mut PlotxApp, ui: &mut Ui) { } if let Some(ci) = start_rename { app.session.active_canvas = Some(ci); - let active = app.doc.canvases[ci].active_dataset(); + let active = app.doc.canvases[ci] + .active_dataset() + .and_then(|id| app.doc.dataset_index(id)); app.set_active_dataset(active); app.sync_selection_to_active_canvas(); app.reset_interaction(); @@ -295,7 +299,8 @@ fn object_list(app: &mut PlotxApp, ci: usize, ui: &mut Ui) { app.select_object(ci, object_id); let active = app.doc.canvases[ci] .object(object_id) - .and_then(|object| object.dataset()); + .and_then(|object| object.dataset()) + .and_then(|id| app.doc.dataset_index(id)); app.set_active_dataset(active); app.reset_interaction(); app.session.ui.panel_note_inline_edit = None; @@ -749,7 +754,7 @@ fn provenance_source_frame(app: &PlotxApp, di: usize) -> Option { .doc .datasets .iter() - .position(|dataset| dataset.resource_id() == source_resource)?; + .position(|dataset| dataset.resource_id().to_string() == source_resource)?; plotx_core::state::page_frame_showing_dataset(app, src).or_else(|| { app.doc .datasets diff --git a/crates/app/src/ui/primary_sidebar/data_browser.rs b/crates/app/src/ui/primary_sidebar/data_browser.rs index 697a63c..b196771 100644 --- a/crates/app/src/ui/primary_sidebar/data_browser.rs +++ b/crates/app/src/ui/primary_sidebar/data_browser.rs @@ -1,6 +1,6 @@ use plotx_core::data::ColumnId; -use plotx_core::state::{Dataset, PlotxApp}; -use std::collections::HashSet; +use plotx_core::state::{Dataset, DatasetId, PlotxApp}; +use std::collections::{HashMap, HashSet}; #[derive(Clone, Debug, PartialEq)] pub(super) struct DataTree { @@ -59,33 +59,43 @@ impl AnalysisKind { impl DataTree { pub fn build(app: &PlotxApp) -> Self { let count = app.doc.datasets.len(); + // This tree is rebuilt every visible frame; resolve source ids through a + // one-shot map instead of a linear `dataset_index` scan per lineage edge. + let index_of: HashMap = app + .doc + .datasets + .iter() + .enumerate() + .map(|(i, dataset)| (dataset.resource_id(), i)) + .collect(); let mut children = vec![Vec::new(); count]; for (derived, dataset) in app.doc.datasets.iter().enumerate() { if let Some(lineage) = dataset.lineage() { for &source in &lineage.sources { - if source < count && source != derived { + if let Some(source) = index_of.get(&source).copied() + && source != derived + { children[source].push(derived); } } } } - let mut roots: Vec = app - .doc - .datasets - .iter() - .enumerate() - .filter(|(di, dataset)| { - dataset.lineage().is_none_or(|lineage| { - lineage.sources.is_empty() - || lineage - .sources - .iter() - .all(|&source| source >= count || source == *di) + let mut roots: Vec = + app.doc + .datasets + .iter() + .enumerate() + .filter(|(di, dataset)| { + dataset.lineage().is_none_or(|lineage| { + lineage.sources.is_empty() + || lineage.sources.iter().all(|&source| { + index_of.get(&source).copied().is_none_or(|i| i == *di) + }) + }) }) - }) - .map(|(di, _)| di) - .collect(); + .map(|(di, _)| di) + .collect(); // Corrupt in-memory graphs can consist only of a cycle. Keep every // dataset reachable even then; recursion below cuts repeated path nodes. @@ -299,7 +309,7 @@ fn reveal_sources(app: &mut PlotxApp, di: usize, visiting: &mut HashSet) .map(|lineage| lineage.sources.clone()) .unwrap_or_default(); for source in sources { - if source < app.doc.datasets.len() { + if let Some(source) = app.doc.dataset_index(source) { app.session .ui .data_browser_collapsed_datasets @@ -319,7 +329,8 @@ pub(super) fn sources_tooltip(app: &PlotxApp, di: usize) -> Option { let names: Vec<_> = lineage .sources .iter() - .filter_map(|&source| app.doc.datasets.get(source)) + .filter_map(|&source| app.doc.dataset_index(source)) + .filter_map(|source| app.doc.datasets.get(source)) .map(Dataset::display_name) .collect(); Some(format!( @@ -333,7 +344,7 @@ pub(super) fn sources_tooltip(app: &PlotxApp, di: usize) -> Option { mod tests { use super::*; use plotx_core::state::{ - CurveFitReference, DatasetLineage, DerivationKind, FloatSeries, LineShapeKind, + CurveFitReference, DatasetId, DatasetLineage, DerivationKind, FloatSeries, LineShapeKind, MultipletPatternKind, NmrDataset, PeakMark, PeakOrigin, StoredLineFit, StoredMultiplet, materialized_float_series_table, }; @@ -355,7 +366,7 @@ mod tests { Dataset::Nmr(Box::new(dataset)) } - fn derived(name: &str, kind: DerivationKind, sources: &[usize]) -> Dataset { + fn derived(name: &str, kind: DerivationKind, sources: &[DatasetId]) -> Dataset { let mut table = materialized_float_series_table( ("x".into(), "".into(), vec![Some(0.0)]), Vec::new(), @@ -370,12 +381,18 @@ mod tests { #[test] fn builds_deep_multi_source_references_in_stable_order() { let mut app = PlotxApp::new(); - app.doc.datasets = vec![ - root("A"), - root("B"), - derived("AB", DerivationKind::SpectrumArithmetic, &[0, 1]), - derived("deep", DerivationKind::LineFitTable, &[2]), + app.doc.datasets = vec![root("A"), root("B")]; + let roots = [ + app.doc.datasets[0].resource_id(), + app.doc.datasets[1].resource_id(), ]; + app.doc + .datasets + .push(derived("AB", DerivationKind::SpectrumArithmetic, &roots)); + let ab = app.doc.datasets[2].resource_id(); + app.doc + .datasets + .push(derived("deep", DerivationKind::LineFitTable, &[ab])); let tree = DataTree::build(&app); assert_eq!( tree.roots.iter().map(|n| n.dataset).collect::>(), @@ -390,10 +407,11 @@ mod tests { #[test] fn filtering_keeps_ancestor_path() { let mut app = PlotxApp::new(); - app.doc.datasets = vec![ - root("source"), - derived("result", DerivationKind::LineFitTable, &[0]), - ]; + app.doc.datasets = vec![root("source")]; + let source = app.doc.datasets[0].resource_id(); + app.doc + .datasets + .push(derived("result", DerivationKind::LineFitTable, &[source])); let filtered = DataTree::build(&app).filtered(&app, "peak fit table"); assert_eq!(filtered.roots.len(), 1); assert_eq!(filtered.roots[0].dataset, 0); @@ -403,10 +421,16 @@ mod tests { #[test] fn cycles_are_cut_and_remain_accessible() { let mut app = PlotxApp::new(); - app.doc.datasets = vec![ - derived("A", DerivationKind::Slice, &[1]), - derived("B", DerivationKind::Projection, &[0]), + app.doc.datasets = vec![root("A"), root("B")]; + let ids = [ + app.doc.datasets[0].resource_id(), + app.doc.datasets[1].resource_id(), ]; + app.doc.datasets[0].set_lineage(Some(DatasetLineage::new(DerivationKind::Slice, [ids[1]]))); + app.doc.datasets[1].set_lineage(Some(DatasetLineage::new( + DerivationKind::Projection, + [ids[0]], + ))); let tree = DataTree::build(&app); assert!(!tree.roots.is_empty()); assert!(tree.roots[0].derived[0].derived[0].cycle_cut); @@ -498,11 +522,17 @@ mod tests { #[test] fn external_focus_reveals_ancestor_branches() { let mut app = PlotxApp::new(); - app.doc.datasets = vec![ - root("source"), - derived("child", DerivationKind::Slice, &[0]), - derived("grandchild", DerivationKind::LineFitTable, &[1]), - ]; + app.doc.datasets = vec![root("source")]; + let source = app.doc.datasets[0].resource_id(); + app.doc + .datasets + .push(derived("child", DerivationKind::Slice, &[source])); + let child = app.doc.datasets[1].resource_id(); + app.doc.datasets.push(derived( + "grandchild", + DerivationKind::LineFitTable, + &[child], + )); app.session .ui .data_browser_collapsed_datasets diff --git a/crates/app/src/ui/tools/electrophysiology.rs b/crates/app/src/ui/tools/electrophysiology.rs index 8c8c2cb..1e84cf0 100644 --- a/crates/app/src/ui/tools/electrophysiology.rs +++ b/crates/app/src/ui/tools/electrophysiology.rs @@ -206,7 +206,9 @@ pub(super) fn electrophysiology_group(app: &mut PlotxApp, di: usize, ui: &mut Ui match result { Ok((table, name, kind)) => { let table_index = app.insert_typed_table_dataset(table, name.to_owned()); - app.doc.datasets[table_index].set_lineage(Some(DatasetLineage::new(kind, [di]))); + let source_id = app.doc.datasets[di].resource_id(); + app.doc.datasets[table_index] + .set_lineage(Some(DatasetLineage::new(kind, [source_id]))); } Err(error) => app.session.status = error.to_string(), } diff --git a/crates/app/src/ui/tools/mod.rs b/crates/app/src/ui/tools/mod.rs index ab0b898..c0233cb 100644 --- a/crates/app/src/ui/tools/mod.rs +++ b/crates/app/src/ui/tools/mod.rs @@ -116,7 +116,7 @@ fn analysis_group(app: &mut PlotxApp, di: usize, ui: &mut Ui) -> bool { .ui .analysis_selection .as_ref() - .map(|selection| selection.dataset == di) + .map(|selection| selection.dataset == app.doc.datasets[di].resource_id()) .unwrap_or(false); ui.horizontal(|ui| { let selected = app.session.tool == Tool::SelectRegion; diff --git a/crates/app/src/ui/tools/region_analysis.rs b/crates/app/src/ui/tools/region_analysis.rs index 7f9bacb..785ff34 100644 --- a/crates/app/src/ui/tools/region_analysis.rs +++ b/crates/app/src/ui/tools/region_analysis.rs @@ -351,7 +351,7 @@ pub(crate) fn open_region_table(app: &mut PlotxApp, di: usize) { .doc .canvases .iter() - .position(|canvas| canvas.active_dataset() == Some(tj)) + .position(|canvas| canvas.active_dataset() == Some(app.doc.datasets[tj].resource_id())) { app.session.active_canvas = Some(ci); app.sync_selection_to_active_canvas(); diff --git a/crates/app/src/ui/tools/slice.rs b/crates/app/src/ui/tools/slice.rs index 9ca89c3..26b74ee 100644 --- a/crates/app/src/ui/tools/slice.rs +++ b/crates/app/src/ui/tools/slice.rs @@ -1,7 +1,7 @@ use egui::{Button, ComboBox, DragValue, Ui}; use plotx_core::actions::Action; use plotx_core::state::{ - AxisProjection, Dataset, ObjectId, PlotxApp, ProjectionSource, SliceCursor, Tool, + AxisProjection, Dataset, DatasetId, ObjectId, PlotxApp, ProjectionSource, SliceCursor, Tool, }; use plotx_processing::{Processed2D, ProjectionMode, SliceKind}; @@ -127,13 +127,13 @@ fn projection_group(app: &mut PlotxApp, di: usize, ui: &mut Ui) { .map(|p| p.projections.clone()) .unwrap_or_default(); - let attachable: Vec<(usize, String)> = app + let attachable: Vec<(DatasetId, String)> = app .doc .datasets .iter() .enumerate() .filter(|(_, d)| d.as_nmr().is_some()) - .map(|(i, d)| (i, d.display_name())) + .map(|(_, d)| (d.resource_id(), d.display_name())) .collect(); let (f2_max, f1_max) = match &app.doc.datasets[di].as_nmr2d().unwrap().processed { @@ -182,7 +182,9 @@ fn active_plot_for(app: &PlotxApp, di: usize) -> Option<(usize, ObjectId)> { let id = canvas .objects .iter() - .find(|o| o.plot().map(|p| p.primary_dataset()) == Some(di)) + .find(|o| { + o.plot().and_then(|p| p.primary_dataset()) == Some(app.doc.datasets[di].resource_id()) + }) .map(|o| o.id)?; Some((ci, id)) } @@ -193,7 +195,7 @@ fn axis_projection_row( label: &str, salt: &str, axis: &mut AxisProjection, - attachable: &[(usize, String)], + attachable: &[(DatasetId, String)], slice_seed: Option, slice_max: usize, ) { @@ -257,7 +259,7 @@ fn axis_projection_row( } } -fn source_label(source: &ProjectionSource, attachable: &[(usize, String)]) -> String { +fn source_label(source: &ProjectionSource, attachable: &[(DatasetId, String)]) -> String { match source { ProjectionSource::None => "None".to_owned(), ProjectionSource::Sum => "Sum projection".to_owned(), @@ -277,7 +279,7 @@ fn slice_group_stack(app: &mut PlotxApp, di: usize, increments: usize, ui: &mut .active_canvas .and_then(|ci| app.doc.canvases.get(ci)) .and_then(|c| c.selected_plot_object_id()) - .unwrap_or(0); + .unwrap_or(ObjectId::new(0)); let mut index = app .session .ui diff --git a/crates/core/src/actions/app_impl/mod.rs b/crates/core/src/actions/app_impl/mod.rs index 8482c93..e426504 100644 --- a/crates/core/src/actions/app_impl/mod.rs +++ b/crates/core/src/actions/app_impl/mod.rs @@ -354,7 +354,9 @@ impl PlotxApp { self.doc.canvases.remove(*index); self.session.active_canvas = *active_after; if let Some(ci) = self.session.active_canvas { - let active = self.doc.canvases[ci].active_dataset(); + let active = self.doc.canvases[ci] + .active_dataset() + .and_then(|id| self.doc.dataset_index(id)); self.set_active_dataset(active); } self.reset_interaction(); @@ -407,7 +409,7 @@ impl PlotxApp { let id = inserted_object_id.unwrap_or(canvas.next_object_id); let object = self.build_plot_object(*dataset_index, frame, id, object_name); let canvas = self.doc.canvases.get_mut(*ci).unwrap(); - canvas.next_object_id = canvas.next_object_id.max(id + 1); + canvas.next_object_id = canvas.next_object_id.max(id.checked_advance(1)); canvas.objects.push(object); self.session.active_canvas = Some(*ci); } else { @@ -499,7 +501,9 @@ impl PlotxApp { } self.doc.canvases.insert(index, canvas); self.session.active_canvas = Some(index); - let active = self.doc.canvases[index].active_dataset(); + let active = self.doc.canvases[index] + .active_dataset() + .and_then(|id| self.doc.dataset_index(id)); self.set_active_dataset(active); self.session.view = PrimaryView::Canvas; self.set_selection(Selection::None); @@ -514,7 +518,8 @@ impl PlotxApp { let active = self .session .active_canvas - .and_then(|ci| self.doc.canvases[ci].active_dataset()); + .and_then(|ci| self.doc.canvases[ci].active_dataset()) + .and_then(|id| self.doc.dataset_index(id)); self.set_active_dataset(active); self.set_selection(Selection::None); } @@ -575,7 +580,7 @@ impl PlotxApp { fn insert_object_value(&mut self, canvas: usize, object: CanvasObject) { let id = object.id; if let Some(c) = self.doc.canvases.get_mut(canvas) { - c.next_object_id = c.next_object_id.max(id + 1); + c.next_object_id = c.next_object_id.max(id.checked_advance(1)); c.objects.push(object); } self.select_object(canvas, id); diff --git a/crates/core/src/actions/app_impl/revert.rs b/crates/core/src/actions/app_impl/revert.rs index 890253e..6ddb343 100644 --- a/crates/core/src/actions/app_impl/revert.rs +++ b/crates/core/src/actions/app_impl/revert.rs @@ -240,7 +240,7 @@ impl PlotxApp { if let Some(c) = self.doc.canvases.get_mut(*canvas) { let at = (*index).min(c.objects.len()); c.objects.insert(at, object.as_ref().clone()); - c.next_object_id = c.next_object_id.max(object.id + 1); + c.next_object_id = c.next_object_id.max(object.id.checked_advance(1)); } self.set_selection(selection_before.clone()); } @@ -265,7 +265,9 @@ impl PlotxApp { self.doc.canvases.insert(*index, canvas.clone()); self.session.active_canvas = *active_before; if let Some(ci) = self.session.active_canvas { - let active = self.doc.canvases[ci].active_dataset(); + let active = self.doc.canvases[ci] + .active_dataset() + .and_then(|id| self.doc.dataset_index(id)); self.set_active_dataset(active); } } diff --git a/crates/core/src/actions/build.rs b/crates/core/src/actions/build.rs index f6cbeeb..11fb50d 100644 --- a/crates/core/src/actions/build.rs +++ b/crates/core/src/actions/build.rs @@ -445,7 +445,7 @@ impl Action { Self::InsertDatasetWithCanvas { dataset_index: app.doc.datasets.len(), canvas_index: app.doc.canvases.len(), - canvas_resource_id: uuid::Uuid::new_v4().to_string(), + canvas_resource_id: crate::state::CanvasId::new(), dataset: Box::new(dataset), canvas_name, size_mm, @@ -462,7 +462,7 @@ impl Action { Self::InsertDatasetWithCanvas { dataset_index: app.doc.datasets.len(), canvas_index: app.doc.canvases.len(), - canvas_resource_id: uuid::Uuid::new_v4().to_string(), + canvas_resource_id: crate::state::CanvasId::new(), dataset: Box::new(dataset), canvas_name: String::new(), size_mm: crate::state::DEFAULT_CANVAS_SIZE_MM, diff --git a/crates/core/src/actions/mod.rs b/crates/core/src/actions/mod.rs index fe6eb2f..378f3e9 100644 --- a/crates/core/src/actions/mod.rs +++ b/crates/core/src/actions/mod.rs @@ -365,7 +365,7 @@ pub enum Action { InsertDatasetWithCanvas { dataset_index: usize, canvas_index: usize, - canvas_resource_id: String, + canvas_resource_id: crate::state::CanvasId, dataset: Box, canvas_name: String, size_mm: [f32; 2], diff --git a/crates/core/src/actions/tests/arithmetic.rs b/crates/core/src/actions/tests/arithmetic.rs index 447b206..9c16c22 100644 --- a/crates/core/src/actions/tests/arithmetic.rs +++ b/crates/core/src/actions/tests/arithmetic.rs @@ -41,7 +41,10 @@ fn subtract_with_coefficient_creates_named_dataset_on_new_canvas() { app.doc.datasets[2].lineage(), Some(&DatasetLineage::new( DerivationKind::SpectrumArithmetic, - [0, 1] + [ + app.doc.datasets[0].resource_id(), + app.doc.datasets[1].resource_id(), + ] )) ); } @@ -167,7 +170,7 @@ fn unary_scale_and_offset_create_independent_dataset() { app.doc.datasets[1].lineage(), Some(&DatasetLineage::new( DerivationKind::SpectrumArithmetic, - [0] + [app.doc.datasets[0].resource_id()] )) ); diff --git a/crates/core/src/actions/tests/authoring.rs b/crates/core/src/actions/tests/authoring.rs index 1273c98..a30778a 100644 --- a/crates/core/src/actions/tests/authoring.rs +++ b/crates/core/src/actions/tests/authoring.rs @@ -228,11 +228,23 @@ fn undo_finishes_and_reverts_a_live_axis_override_edit() { #[test] fn reorder_z_front_and_back_preserve_relative_order() { - let order = [1u64, 2, 3, 4]; - assert_eq!(reorder_z(&order, &[1, 3], ZOrder::Front), vec![2, 4, 1, 3]); - assert_eq!(reorder_z(&order, &[2, 4], ZOrder::Back), vec![2, 4, 1, 3]); - assert_eq!(reorder_z(&order, &[1], ZOrder::Forward), vec![2, 1, 3, 4]); - assert_eq!(reorder_z(&order, &[4], ZOrder::Backward), vec![1, 2, 4, 3]); + let ids = [1, 2, 3, 4].map(ObjectId::new); + assert_eq!( + reorder_z(&ids, &[ObjectId::new(1), ObjectId::new(3)], ZOrder::Front), + [2, 4, 1, 3].map(ObjectId::new) + ); + assert_eq!( + reorder_z(&ids, &[ObjectId::new(2), ObjectId::new(4)], ZOrder::Back), + [2, 4, 1, 3].map(ObjectId::new) + ); + assert_eq!( + reorder_z(&ids, &[ObjectId::new(1)], ZOrder::Forward), + [2, 1, 3, 4].map(ObjectId::new) + ); + assert_eq!( + reorder_z(&ids, &[ObjectId::new(4)], ZOrder::Backward), + [1, 2, 4, 3].map(ObjectId::new) + ); } #[test] @@ -249,14 +261,14 @@ fn bring_to_front_moves_id_to_front_end_and_undoes() { app.doc.canvases[0].objects.push(object); } let ids: Vec<_> = app.doc.canvases[0].objects.iter().map(|o| o.id).collect(); - assert_eq!(ids, vec![1, 2, 3]); + assert_eq!(ids, [1, 2, 3].map(ObjectId::new)); - app.apply_z_order(0, &[1], ZOrder::Front); + app.apply_z_order(0, &[ObjectId::new(1)], ZOrder::Front); let after: Vec<_> = app.doc.canvases[0].objects.iter().map(|o| o.id).collect(); - assert_eq!(after, vec![2, 3, 1]); - assert_eq!(*after.last().unwrap(), 1); + assert_eq!(after, [2, 3, 1].map(ObjectId::new)); + assert_eq!(*after.last().unwrap(), ObjectId::new(1)); app.undo(); let reverted: Vec<_> = app.doc.canvases[0].objects.iter().map(|o| o.id).collect(); - assert_eq!(reverted, vec![1, 2, 3]); + assert_eq!(reverted, [1, 2, 3].map(ObjectId::new)); } diff --git a/crates/core/src/actions/tests/integral_curve.rs b/crates/core/src/actions/tests/integral_curve.rs index 5b5e4d8..4a58365 100644 --- a/crates/core/src/actions/tests/integral_curve.rs +++ b/crates/core/src/actions/tests/integral_curve.rs @@ -149,7 +149,10 @@ fn overlay_only_dataset_does_not_contribute_integrals() { secondary.as_nmr_mut().unwrap().integrals = vec![sample_integral(4, 2.0, None)]; app.doc.datasets.push(secondary); let binding = DataBinding { - series: vec![SeriesBinding::new(0), SeriesBinding::new(1)], + series: vec![ + SeriesBinding::new(app.doc.datasets[0].resource_id()), + SeriesBinding::new(app.doc.datasets[1].resource_id()), + ], }; let fig = app.build_stacked_figure(&binding, &StackSpec::default(), [120.0, 80.0]); assert!(fig.integral_curves.is_empty()); diff --git a/crates/core/src/actions/tests/linefit.rs b/crates/core/src/actions/tests/linefit.rs index b65ca2c..970ce29 100644 --- a/crates/core/src/actions/tests/linefit.rs +++ b/crates/core/src/actions/tests/linefit.rs @@ -122,7 +122,10 @@ fn run_line_fit_stores_inline_result_and_materializes_on_request() { assert!(table.provenance.is_none()); assert_eq!( app.doc.datasets[1].lineage(), - Some(&DatasetLineage::new(DerivationKind::LineFitTable, [0])) + Some(&DatasetLineage::new( + DerivationKind::LineFitTable, + [app.doc.datasets[0].resource_id()] + )) ); let series_names: Vec<&str> = app.doc.canvases[0].objects[0] @@ -361,7 +364,7 @@ fn single_table_figure_gains_line_fit_overlays() { assert_eq!(app.doc.datasets[1].line_fits().len(), 1); let fig = app.build_binding_figure( - &DataBinding::single(1), + &DataBinding::single(app.doc.datasets[1].resource_id()), &ChartSpec::default_for(DataDomain::Table), &StackSpec::default(), [120.0, 80.0], @@ -404,7 +407,7 @@ fn non_line_table_charts_skip_line_fit_overlays() { "table_surface", ] { let fig = app.build_binding_figure( - &DataBinding::single(1), + &DataBinding::single(app.doc.datasets[1].resource_id()), &ChartSpec { type_id: id.to_owned(), ..ChartSpec::default() @@ -426,7 +429,10 @@ fn stacked_figures_exclude_line_fit_overlays() { app.execute_action(Action::set_line_fits(0, Vec::new(), vec![stored_sample(0)])); let binding = DataBinding { - series: vec![SeriesBinding::new(0), SeriesBinding::new(1)], + series: vec![ + SeriesBinding::new(app.doc.datasets[0].resource_id()), + SeriesBinding::new(app.doc.datasets[1].resource_id()), + ], }; let stack = StackSpec { mode: StackMode::Offset, @@ -443,7 +449,7 @@ fn single_plot_color_override_leaves_overlay_colors_alone() { app.execute_action(Action::set_line_fits(0, Vec::new(), vec![stored_sample(0)])); let override_color = Color::rgb(0x11, 0x22, 0x33); - let mut binding = DataBinding::single(0); + let mut binding = DataBinding::single(app.doc.datasets[0].resource_id()); binding.series[0].color = Some(override_color); let fig = app.build_binding_figure( &binding, diff --git a/crates/core/src/actions/tests/mod.rs b/crates/core/src/actions/tests/mod.rs index 155445e..6d0c8ef 100644 --- a/crates/core/src/actions/tests/mod.rs +++ b/crates/core/src/actions/tests/mod.rs @@ -15,6 +15,7 @@ mod linefit; mod more; mod multiplet; mod scheme_apply; +mod stable_identity; mod stack; mod tiling; use num_complex::Complex64; @@ -138,7 +139,7 @@ fn insert_dataset_existing_canvas_does_not_select_inserted_object() { app.execute_action(Action::InsertDatasetWithCanvas { dataset_index, canvas_index: app.doc.canvases.len(), - canvas_resource_id: uuid::Uuid::new_v4().to_string(), + canvas_resource_id: crate::state::CanvasId::new(), dataset: Box::new(dataset), canvas_name: "unused".to_owned(), size_mm: DEFAULT_CANVAS_SIZE_MM, @@ -155,6 +156,11 @@ fn insert_dataset_existing_canvas_does_not_select_inserted_object() { app.undo(); assert_eq!(app.doc.canvases[0].objects.len(), 1); assert_eq!(app.doc.canvases[0].selected_object, None); + assert!(app.doc.canvases[0].next_object_id > inserted_id); + + app.redo(); + assert!(app.doc.canvases[0].object(inserted_id).is_some()); + assert!(app.doc.canvases[0].next_object_id > inserted_id); } #[test] @@ -546,6 +552,8 @@ fn delete_canvas_undo_restores_order_and_active_canvas() { let mut app = sample_app(); push_canvas(&mut app, 0, "second canvas", [90.0, 60.0]); app.session.active_canvas = Some(0); + let canvas_id = app.doc.canvases[0].resource_id; + let object_id = app.doc.canvases[0].objects[0].id; app.execute_action(Action::delete_canvas(&app, 0).unwrap()); assert_eq!(app.doc.canvases[0].name, "second canvas"); @@ -553,6 +561,8 @@ fn delete_canvas_undo_restores_order_and_active_canvas() { app.undo(); assert_eq!(app.doc.canvases[0].name, "sample canvas"); + assert_eq!(app.doc.canvases[0].resource_id, canvas_id); + assert!(app.doc.canvases[0].object(object_id).is_some()); assert_eq!(app.doc.canvases[1].name, "second canvas"); assert_eq!(app.session.active_canvas, Some(0)); } diff --git a/crates/core/src/actions/tests/more.rs b/crates/core/src/actions/tests/more.rs index 829ec96..13f1e07 100644 --- a/crates/core/src/actions/tests/more.rs +++ b/crates/core/src/actions/tests/more.rs @@ -11,15 +11,15 @@ fn stacked_binding_builds_distinctly_coloured_series_with_legend() { let object = app.doc.canvases[0].objects[0].id; let binding = crate::state::DataBinding { series: vec![ - crate::state::SeriesBinding::new(0), - crate::state::SeriesBinding::new(1), + crate::state::SeriesBinding::new(app.doc.datasets[0].resource_id()), + crate::state::SeriesBinding::new(app.doc.datasets[1].resource_id()), ], }; app.execute_action(Action::set_data_binding( 0, object, - crate::state::DataBinding::single(0), + crate::state::DataBinding::single(app.doc.datasets[0].resource_id()), binding, )); @@ -41,7 +41,7 @@ fn single_table_color_override_recolors_points_and_error_bars() { use crate::state::{ChartSpec, DataBinding, DataDomain, SeriesBinding, StackSpec}; let (app, _) = table_app_with_sigma(vec![0.1, 0.1, 0.1]); let color = plotx_figure::Color::rgb(0xaa, 0x22, 0x44); - let mut series = SeriesBinding::new(0); + let mut series = SeriesBinding::new(app.doc.datasets[0].resource_id()); series.color = Some(color); let figure = app.build_binding_figure( &DataBinding { @@ -65,7 +65,7 @@ fn single_table_color_override_recolors_bar_polygons() { use crate::state::{ChartSpec, DataBinding, SeriesBinding, StackSpec}; let (app, _) = table_app_with_sigma(vec![0.1, 0.1, 0.1]); let color = plotx_figure::Color::rgb(0xaa, 0x22, 0x44); - let mut series = SeriesBinding::new(0); + let mut series = SeriesBinding::new(app.doc.datasets[0].resource_id()); series.color = Some(color); let figure = app.build_binding_figure( &DataBinding { @@ -139,7 +139,7 @@ fn stack_candidates_reject_incompatible_datasets() { .push(Dataset::Nmr2D(Box::new(crate::state::Nmr2DDataset::load( synthetic_2d(), )))); - let binding = crate::state::DataBinding::single(0); + let binding = crate::state::DataBinding::single(app.doc.datasets[0].resource_id()); let candidates = app.stack_candidates(&binding); assert!(candidates.contains(&1), "the other 1D spectrum is eligible"); @@ -150,7 +150,7 @@ fn stack_candidates_reject_incompatible_datasets() { // The 2D primary is a Field-stackable domain, but with no other 2D dataset // loaded there is nothing to overlay onto it. - let two_d = crate::state::DataBinding::single(2); + let two_d = crate::state::DataBinding::single(app.doc.datasets[2].resource_id()); assert!(app.stack_candidates(&two_d).is_empty()); } @@ -174,7 +174,7 @@ fn axis_projections_attach_and_project_survive_undo() { let before = AxisProjections::default(); let after = AxisProjections { top: AxisProjection { - source: ProjectionSource::Attached(0), + source: ProjectionSource::Attached(app.doc.datasets[0].resource_id()), visible: true, }, left: AxisProjection { diff --git a/crates/core/src/actions/tests/multiplet.rs b/crates/core/src/actions/tests/multiplet.rs index f053862..07256fa 100644 --- a/crates/core/src/actions/tests/multiplet.rs +++ b/crates/core/src/actions/tests/multiplet.rs @@ -96,7 +96,10 @@ fn apply_multiplet_analysis_stores_creates_table_and_undoes_as_one_step() { ); assert_eq!( app.doc.datasets[1].lineage(), - Some(&DatasetLineage::new(DerivationKind::MultipletTable, [0])) + Some(&DatasetLineage::new( + DerivationKind::MultipletTable, + [app.doc.datasets[0].resource_id()] + )) ); app.undo(); diff --git a/crates/core/src/actions/tests/stable_identity.rs b/crates/core/src/actions/tests/stable_identity.rs new file mode 100644 index 0000000..f833a00 --- /dev/null +++ b/crates/core/src/actions/tests/stable_identity.rs @@ -0,0 +1,100 @@ +use super::{sample_app, synthetic_1d}; +use crate::actions::Action; +use crate::state::{ + AxisProjection, DEFAULT_CANVAS_SIZE_MM, Dataset, DatasetId, DatasetLineage, DerivationKind, + NmrDataset, ObjectFrame, ProjectionSource, SeriesBinding, +}; + +#[test] +fn dataset_delete_undo_restores_identity_and_persistent_references() { + let mut app = sample_app(); + let mut inserted = Dataset::Nmr(Box::new(NmrDataset::load(synthetic_1d()))); + let inserted_id = DatasetId::new(); + inserted.set_resource_id(inserted_id); + let action = Action::insert_dataset_with_default_canvas( + &app, + inserted, + "referenced".to_owned(), + DEFAULT_CANVAS_SIZE_MM, + ); + + let plot = app.doc.canvases[0].objects[0].plot_mut().unwrap(); + plot.binding.series.push(SeriesBinding::new(inserted_id)); + plot.projections.top = AxisProjection { + source: ProjectionSource::Attached(inserted_id), + visible: true, + }; + app.doc.datasets[0].set_lineage(Some(DatasetLineage::new( + DerivationKind::Projection, + [inserted_id], + ))); + + app.execute_action(action); + let canvas_id = app.doc.canvases[1].resource_id; + let object_id = app.doc.canvases[1].objects[0].id; + assert_eq!(app.doc.dataset_index(inserted_id), Some(1)); + + app.undo(); + assert!(app.doc.dataset_index(inserted_id).is_none()); + assert!(app.doc.canvas_index(canvas_id).is_none()); + + app.redo(); + assert_eq!(app.doc.dataset_index(inserted_id), Some(1)); + assert_eq!(app.doc.canvas_index(canvas_id), Some(1)); + assert!(app.doc.canvases[1].object(object_id).is_some()); + let plot = app.doc.canvases[0].objects[0].plot().unwrap(); + assert!(plot.binding.contains_dataset(inserted_id)); + assert_eq!( + plot.projections.top.source, + ProjectionSource::Attached(inserted_id) + ); + assert_eq!( + app.doc.datasets[0].lineage().unwrap().sources, + vec![inserted_id] + ); +} + +#[test] +fn canvas_dataset_ids_follow_first_appearance_and_page_indices_follow_document_order() { + let mut app = sample_app(); + let ids: [DatasetId; 3] = [ + "ffffffff-ffff-4fff-8fff-ffffffffffff".parse().unwrap(), + "00000000-0000-4000-8000-000000000001".parse().unwrap(), + "77777777-7777-4777-8777-777777777777".parse().unwrap(), + ]; + app.doc.datasets[0].set_resource_id(ids[0]); + for id in &ids[1..] { + let mut dataset = Dataset::Nmr(Box::new(NmrDataset::load(synthetic_1d()))); + dataset.set_resource_id(*id); + app.doc.datasets.push(dataset); + } + + let canvas = &mut app.doc.canvases[0]; + canvas.objects[0].plot_mut().unwrap().binding.series = vec![ + SeriesBinding::new(ids[2]), + SeriesBinding::new(ids[0]), + SeriesBinding::new(ids[2]), + ]; + let object_id = canvas.allocate_object_id(); + let second_plot = app.build_plot_object( + 1, + ObjectFrame::new(0.0, 0.0, 40.0, 30.0), + object_id, + "Plot 2".to_owned(), + ); + app.doc.canvases[0].objects.push(second_plot); + + let expected_ids = vec![ids[2], ids[0], ids[1]]; + assert_eq!(app.doc.canvases[0].dataset_ids(), expected_ids); + assert_eq!(app.doc.canvases[0].dataset_ids(), expected_ids); + assert_eq!(app.doc.page_dataset_indices(0), vec![0, 1, 2]); + assert_eq!(app.doc.page_dataset_indices(0), vec![0, 1, 2]); +} + +#[test] +fn syncing_integral_curves_ignores_a_stale_dataset_index() { + let mut app = sample_app(); + let stale_index = app.doc.datasets.len(); + + app.sync_integral_curves_for(stale_index); +} diff --git a/crates/core/src/actions/tests/stack.rs b/crates/core/src/actions/tests/stack.rs index 57f22a8..48a52e9 100644 --- a/crates/core/src/actions/tests/stack.rs +++ b/crates/core/src/actions/tests/stack.rs @@ -17,7 +17,10 @@ fn stacked_figure_is_domain_generic_with_offset_scale_and_hide() { for (app, domain) in [(&nmr, DataDomain::Nmr1d), (&table, DataDomain::Table)] { let chart = ChartSpec::default_for(domain); let binding = DataBinding { - series: vec![SeriesBinding::new(0), SeriesBinding::new(1)], + series: vec![ + SeriesBinding::new(app.doc.datasets[0].resource_id()), + SeriesBinding::new(app.doc.datasets[1].resource_id()), + ], }; let sup = app.build_binding_figure(&binding, &chart, &StackSpec::default(), size); assert!(sup.show_legend, "a combined figure shows a legend"); @@ -75,7 +78,10 @@ fn field_overlay_stacks_two_2d_contours_in_distinct_colors() { )))); let (a, b) = (app.doc.datasets.len() - 2, app.doc.datasets.len() - 1); let binding = DataBinding { - series: vec![SeriesBinding::new(a), SeriesBinding::new(b)], + series: vec![ + SeriesBinding::new(app.doc.datasets[a].resource_id()), + SeriesBinding::new(app.doc.datasets[b].resource_id()), + ], }; let chart = ChartSpec::default_for(DataDomain::Nmr2d); let stack = StackSpec { @@ -142,22 +148,28 @@ fn selecting_canvas_populates_data_selection_with_its_datasets() { let object = app.doc.canvases[0].objects[0].id; let binding = crate::state::DataBinding { series: vec![ - crate::state::SeriesBinding::new(0), - crate::state::SeriesBinding::new(1), + crate::state::SeriesBinding::new(app.doc.datasets[0].resource_id()), + crate::state::SeriesBinding::new(app.doc.datasets[1].resource_id()), ], }; app.execute_action(Action::set_data_binding( 0, object, - crate::state::DataBinding::single(0), + crate::state::DataBinding::single(app.doc.datasets[0].resource_id()), binding, )); - assert_eq!(app.doc.canvases[0].dataset_indices(), vec![0, 1]); + let mut dataset_indices = app.doc.canvases[0] + .dataset_ids() + .into_iter() + .filter_map(|id| app.doc.dataset_index(id)) + .collect::>(); + dataset_indices.sort_unstable(); + assert_eq!(dataset_indices, vec![0, 1]); // Selecting the canvas mirrors its datasets into the Data-list multi-select, // so a qualifying page can be stacked immediately. - app.session.ui.data_selection = app.doc.canvases[0].dataset_indices(); + app.session.ui.data_selection = dataset_indices; assert_eq!(app.stackable_selection(), Some(vec![0, 1])); } @@ -170,20 +182,29 @@ fn plot_object_reports_every_bound_dataset_for_selection_mirroring() { .push(Dataset::Nmr(Box::new(NmrDataset::load(synthetic_1d())))); let object = app.doc.canvases[0].objects[0].id; let binding = DataBinding { - series: vec![SeriesBinding::new(0), SeriesBinding::new(1)], + series: vec![ + SeriesBinding::new(app.doc.datasets[0].resource_id()), + SeriesBinding::new(app.doc.datasets[1].resource_id()), + ], }; app.execute_action(Action::set_data_binding( 0, object, - DataBinding::single(0), + DataBinding::single(app.doc.datasets[0].resource_id()), binding, )); // The board-click handler mirrors this into the Data list, so a stacked plot // must surface both of its datasets, not just the primary. let obj = app.doc.canvases[0].object(object).unwrap(); - assert_eq!(obj.dataset_indices(), vec![0, 1]); - assert_eq!(obj.dataset(), Some(0)); + assert_eq!( + obj.dataset_ids(), + vec![ + app.doc.datasets[0].resource_id(), + app.doc.datasets[1].resource_id() + ] + ); + assert_eq!(obj.dataset(), Some(app.doc.datasets[0].resource_id())); } #[test] @@ -196,7 +217,10 @@ fn shear_sign_flips_the_pseudo_3d_lean_direction() { .push(Dataset::Table(Box::new(second_table()))); let chart = ChartSpec::default_for(DataDomain::Table); let binding = DataBinding { - series: vec![SeriesBinding::new(0), SeriesBinding::new(1)], + series: vec![ + SeriesBinding::new(table.doc.datasets[0].resource_id()), + SeriesBinding::new(table.doc.datasets[1].resource_id()), + ], }; let size = [120.0, 80.0]; let right = StackSpec { diff --git a/crates/core/src/actions/tests/tiling.rs b/crates/core/src/actions/tests/tiling.rs index 668ac98..3d5f1f7 100644 --- a/crates/core/src/actions/tests/tiling.rs +++ b/crates/core/src/actions/tests/tiling.rs @@ -1,7 +1,9 @@ use crate::actions::Action; use crate::actions::tests::{push_canvas, sample_app}; use crate::layout::compute_tiling_plan; -use crate::state::{AxisOverrides, AxisRange, ObjectFrame, TileDropCacheKey, TileDropPreview}; +use crate::state::{ + AxisOverrides, AxisRange, ObjectFrame, ObjectId, TileDropCacheKey, TileDropPreview, +}; /// A drop of canvas 0's plot onto canvas 1 (which already has one plot) transfers /// ownership and reframes both into a two-way split, undoably. @@ -114,11 +116,11 @@ fn cancelling_interaction_clears_tile_preview_cache() { app.session.ui.tile_drop = Some(TileDropPreview { cache_key: TileDropCacheKey { source_canvas: 0, - source_object: 1, + source_object: ObjectId::new(1), target_canvas: 1, target_page_pt: [100.0, 80.0], target_layout: crate::layout::PageLayout::default(), - target_existing_ids: vec![2], + target_existing_ids: vec![ObjectId::new(2)], region: crate::layout::TilingDropRegion::Left, pointer_cell: None, }, diff --git a/crates/core/src/actions/transfer.rs b/crates/core/src/actions/transfer.rs index d75c6e8..427b5ed 100644 --- a/crates/core/src/actions/transfer.rs +++ b/crates/core/src/actions/transfer.rs @@ -44,7 +44,7 @@ impl Action { let mut removed = Vec::with_capacity(picked.len()); for (offset, &(slot, object)) in picked.iter().enumerate() { let mut clone = object.clone(); - clone.id = dst.next_object_id + offset as ObjectId; + clone.id = dst.next_object_id.checked_advance(offset as u64); if let Some(g) = clone.group { let mapped = match group_map.iter().find(|(old, _)| *old == g) { Some(&(_, new)) => new, @@ -173,7 +173,7 @@ impl PlotxApp { let ids: Vec = inserted.iter().map(|o| o.id).collect(); if let Some(dst) = self.doc.canvases.get_mut(to) { for object in inserted { - dst.next_object_id = dst.next_object_id.max(object.id + 1); + dst.next_object_id = dst.next_object_id.max(object.id.checked_advance(1)); if let Some(group) = object.group { dst.next_group_id = dst.next_group_id.max(group + 1); } @@ -183,7 +183,12 @@ impl PlotxApp { } self.session.active_canvas = Some(to); self.session.ui.selection = Selection::Objects(ids); - let active = self.doc.canvases.get(to).and_then(|c| c.active_dataset()); + let active = self + .doc + .canvases + .get(to) + .and_then(|c| c.active_dataset()) + .and_then(|id| self.doc.dataset_index(id)); self.set_active_dataset(active); self.session.view = PrimaryView::Canvas; self.clear_transfer_transients(); @@ -219,14 +224,15 @@ impl PlotxApp { // index despite earlier insertions. for (slot, object) in removed { let at = (*slot).min(src.objects.len()); - src.next_object_id = src.next_object_id.max(object.id + 1); + src.next_object_id = src.next_object_id.max(object.id.checked_advance(1)); src.objects.insert(at, object.clone()); } } self.session.active_canvas = active_before; let active = active_before .and_then(|ci| self.doc.canvases.get(ci)) - .and_then(|c| c.active_dataset()); + .and_then(|c| c.active_dataset()) + .and_then(|id| self.doc.dataset_index(id)); self.set_active_dataset(active); self.set_selection(selection_before.clone()); self.clear_transfer_transients(); @@ -266,7 +272,7 @@ impl PlotxApp { let ids: Vec = inserted.iter().map(|o| o.id).collect(); if let Some(dst) = self.doc.canvases.get_mut(to) { for object in inserted { - dst.next_object_id = dst.next_object_id.max(object.id + 1); + dst.next_object_id = dst.next_object_id.max(object.id.checked_advance(1)); if let Some(group) = object.group { dst.next_group_id = dst.next_group_id.max(group + 1); } @@ -293,7 +299,12 @@ impl PlotxApp { let to = *target_index_after; self.session.active_canvas = Some(to); self.session.ui.selection = Selection::Objects(ids); - let active = self.doc.canvases.get(to).and_then(|c| c.active_dataset()); + let active = self + .doc + .canvases + .get(to) + .and_then(|c| c.active_dataset()) + .and_then(|id| self.doc.dataset_index(id)); self.set_active_dataset(active); self.session.view = PrimaryView::Canvas; self.clear_transfer_transients(); @@ -348,14 +359,15 @@ impl PlotxApp { { for (slot, object) in removed { let at = (*slot).min(src.objects.len()); - src.next_object_id = src.next_object_id.max(object.id + 1); + src.next_object_id = src.next_object_id.max(object.id.checked_advance(1)); src.objects.insert(at, object.clone()); } } self.session.active_canvas = active_before; let active = active_before .and_then(|ci| self.doc.canvases.get(ci)) - .and_then(|c| c.active_dataset()); + .and_then(|c| c.active_dataset()) + .and_then(|id| self.doc.dataset_index(id)); self.set_active_dataset(active); self.set_selection(selection_before.clone()); self.clear_transfer_transients(); diff --git a/crates/core/src/automation/resources.rs b/crates/core/src/automation/resources.rs index 7ef7e78..57545a0 100644 --- a/crates/core/src/automation/resources.rs +++ b/crates/core/src/automation/resources.rs @@ -49,13 +49,16 @@ impl<'a> ProjectResourceProvider<'a> { } fn dataset_descriptor(&self, index: usize, dataset: &Dataset) -> ResourceDescriptor { - let id = dataset.resource_id().to_owned(); + let id = dataset.resource_id(); + // Only emit lineage sources that still exist: an undo can leave a + // dangling DatasetId in the document (kept for redo), but a descriptor + // naming a resource `resolve_target` would reject is inconsistent. let lineage = dataset .lineage() .into_iter() .flat_map(|lineage| lineage.sources.iter()) - .filter_map(|source| self.app.doc.datasets.get(*source)) - .map(|source| source.resource_id().to_owned()) + .filter(|source| self.app.doc.dataset_index(**source).is_some()) + .map(ToString::to_string) .collect(); let mut capabilities = vec![cap(CAP_RENAME), cap(CAP_PREVIEW)]; if matches!(dataset, Dataset::Nmr(_) | Dataset::Nmr2D(_)) { @@ -72,7 +75,7 @@ impl<'a> ProjectResourceProvider<'a> { .schema .columns .iter() - .map(|column| child_ref(&id, &column.id.to_string(), KIND_TABLE_COLUMN)); + .map(|column| child_ref(id, &column.id.to_string(), KIND_TABLE_COLUMN)); metadata.insert("table_id".into(), snapshot.table_id.to_string()); metadata.insert( "table_revision".into(), @@ -135,7 +138,7 @@ impl<'a> ProjectResourceProvider<'a> { } }; ResourceDescriptor { - resource: top_ref(&id, KIND_DATASET), + resource: top_ref(id, KIND_DATASET), name: dataset.display_name(), capabilities, children, @@ -154,7 +157,7 @@ impl<'a> ProjectResourceProvider<'a> { .iter() .map(|object| { child_ref( - &canvas.resource_id, + canvas.resource_id, &object.id.to_string(), KIND_CANVAS_OBJECT, ) @@ -163,7 +166,7 @@ impl<'a> ProjectResourceProvider<'a> { let mut metadata = BTreeMap::new(); metadata.insert("index_hint".to_owned(), index.to_string()); ResourceDescriptor { - resource: top_ref(&canvas.resource_id, KIND_CANVAS), + resource: top_ref(canvas.resource_id, KIND_CANVAS), name: canvas.name.clone(), capabilities: vec![ cap(CAP_RENAME), @@ -176,10 +179,10 @@ impl<'a> ProjectResourceProvider<'a> { units: vec!["mm".to_owned()], metadata, lineage: canvas - .dataset_indices() + .dataset_ids() .into_iter() - .filter_map(|dataset| self.app.doc.datasets.get(dataset)) - .map(|dataset| dataset.resource_id().to_owned()) + .filter(|dataset| self.app.doc.dataset_index(*dataset).is_some()) + .map(|dataset| dataset.to_string()) .collect(), revision: self.revision(), } @@ -202,13 +205,13 @@ impl ResourceProvider for ProjectResourceProvider<'_> { .datasets .iter() .flat_map(|dataset| { - let child = dataset.resource_id().to_owned(); + let child = dataset.resource_id(); dataset .lineage() .into_iter() .flat_map(|lineage| lineage.sources.iter()) - .filter_map(|source| self.app.doc.datasets.get(*source)) - .map(move |source| format!("{} derives_from {}", child, source.resource_id())) + .filter(|source| self.app.doc.dataset_index(**source).is_some()) + .map(move |source| format!("{child} derives_from {source}")) }) .collect(); ProjectBlueprint { @@ -258,7 +261,7 @@ impl ResourceProvider for ProjectResourceProvider<'_> { for object in &canvas.objects { descriptors.push(ResourceDescriptor { resource: child_ref( - &canvas.resource_id, + canvas.resource_id, &object.id.to_string(), KIND_CANVAS_OBJECT, ), @@ -269,10 +272,10 @@ impl ResourceProvider for ProjectResourceProvider<'_> { units: Vec::new(), metadata: BTreeMap::new(), lineage: object - .dataset_indices() + .dataset_ids() .into_iter() - .filter_map(|dataset| self.app.doc.datasets.get(dataset)) - .map(|dataset| dataset.resource_id().to_owned()) + .filter(|dataset| self.app.doc.dataset_index(*dataset).is_some()) + .map(|dataset| dataset.to_string()) .collect(), revision: self.revision(), }); @@ -297,7 +300,7 @@ impl ResourceProvider for ProjectResourceProvider<'_> { .active_canvas .and_then(|index| self.app.doc.canvases.get(index)) { - selected.push(top_ref(&canvas.resource_id, KIND_CANVAS)); + selected.push(top_ref(canvas.resource_id, KIND_CANVAS)); } selected } @@ -307,8 +310,8 @@ impl ResourceProvider for ProjectResourceProvider<'_> { AutomationError::InvalidSelector(format!("resource {} does not exist", target.id)) })?; let Some(dataset) = self.app.doc.datasets.iter().find(|dataset| { - dataset.resource_id() == target.id - || target.parent_id.as_deref() == Some(dataset.resource_id()) + dataset.resource_id().to_string() == target.id + || target.parent_id.as_deref() == Some(dataset.resource_id().to_string().as_str()) }) else { return Ok(DataPreview { target: target.clone(), @@ -334,7 +337,7 @@ impl ResourceProvider for ProjectResourceProvider<'_> { .doc .canvases .iter() - .find(|canvas| canvas.resource_id == canvas_id) + .find(|canvas| canvas.resource_id.to_string() == canvas_id) .ok_or_else(|| { AutomationError::InvalidSelector("render.preview requires a canvas".to_owned()) })?; @@ -673,20 +676,21 @@ fn finite_json(value: f64) -> serde_json::Value { } } -fn top_ref(id: &str, kind: &str) -> ResourceRef { +fn top_ref(id: impl ToString, kind: &str) -> ResourceRef { ResourceRef { - id: id.to_owned(), + id: id.to_string(), kind: ResourceKindId::new(kind), parent_id: None, local_id: None, } } -fn child_ref(parent: &str, local: &str, kind: &str) -> ResourceRef { +fn child_ref(parent: impl ToString, local: &str, kind: &str) -> ResourceRef { + let parent = parent.to_string(); ResourceRef { id: format!("{parent}/{local}"), kind: ResourceKindId::new(kind), - parent_id: Some(parent.to_owned()), + parent_id: Some(parent), local_id: Some(local.to_owned()), } } diff --git a/crates/core/src/automation/tests.rs b/crates/core/src/automation/tests.rs index 1ad466c..eba5399 100644 --- a/crates/core/src/automation/tests.rs +++ b/crates/core/src/automation/tests.rs @@ -85,7 +85,7 @@ fn query_reports_reasons_pagination_and_stale_frozen_sets_are_rejected() { request( "resource.rename", serde_json::json!({"name":"next"}), - vec![id], + vec![id.to_string()], 0, ), ) @@ -111,7 +111,7 @@ fn unknown_tool_parameters_are_rejected_and_registry_ids_are_unique() { request( "resource.rename", serde_json::json!({"name":"ok", "surprise":true}), - vec![id], + vec![id.to_string()], 0, ), ) @@ -138,7 +138,7 @@ fn data_transform_executes_the_same_persisted_relplan_and_is_undoable() { )), columns: vec![signal], }); - let resource_id = source.resource_id.clone(); + let resource_id = source.resource_id; let tool_plan = plan_tool( &app, request( @@ -148,7 +148,7 @@ fn data_transform_executes_the_same_persisted_relplan_and_is_undoable() { "name": "Projected", "memory_limit_bytes": 16 * 1024 * 1024, }), - vec![resource_id.clone()], + vec![resource_id.to_string()], app.doc.automation_revision, ), ) @@ -198,7 +198,7 @@ fn data_transform_executes_the_same_persisted_relplan_and_is_undoable() { "memory_limit_bytes": 16 * 1024 * 1024, }), targets: TargetSelector::Explicit { - ids: vec![resource_id], + ids: vec![resource_id.to_string()], }, dependencies: Vec::new(), bindings: Vec::new(), @@ -245,13 +245,13 @@ fn composite_validation_prevents_partial_application() { fn dag_executes_by_output_binding_and_collapses_to_one_undo() { let mut app = app_with_table_and_canvas(); let dataset_id = app.doc.datasets[0].resource_id().to_owned(); - let canvas_id = app.doc.canvases[0].resource_id.clone(); + let canvas_id = app.doc.canvases[0].resource_id; let workflow = WorkflowDefinition { schema: WORKFLOW_SCHEMA.to_owned(), inputs: BTreeMap::from([( "targets".to_owned(), WorkflowInput::Resources { - ids: vec![dataset_id, canvas_id], + ids: vec![dataset_id.to_string(), canvas_id.to_string()], }, )]), nodes: vec![ @@ -345,7 +345,7 @@ fn dag_validation_rejects_cycles_missing_ports_and_wrong_parameters() { fn stable_ids_rows_columns_runs_and_revision_survive_project_roundtrip() { let mut app = app_with_table_and_canvas(); let dataset_id = app.doc.datasets[0].resource_id().to_owned(); - let canvas_id = app.doc.canvases[0].resource_id.clone(); + let canvas_id = app.doc.canvases[0].resource_id; let table = app.doc.datasets[0].as_table().unwrap(); let row_ids = table.typed_rows(usize::MAX, &[]).unwrap().row_ids; let column_ids = table @@ -494,8 +494,8 @@ fn simulated_agent_observes_plans_modifies_renders_exports_and_undoes() { ); assert!(targets.total_matches >= 2); let ids = vec![ - app.doc.datasets[0].resource_id().to_owned(), - app.doc.canvases[0].resource_id.clone(), + app.doc.datasets[0].resource_id().to_string(), + app.doc.canvases[0].resource_id.to_string(), ]; let output = std::env::temp_dir().join(format!("plotx-agent-{}", uuid::Uuid::new_v4())); let workflow = WorkflowDefinition { diff --git a/crates/core/src/automation/tools.rs b/crates/core/src/automation/tools.rs index 63e761d..b357dfe 100644 --- a/crates/core/src/automation/tools.rs +++ b/crates/core/src/automation/tools.rs @@ -217,7 +217,7 @@ fn execute_rename(app: &mut PlotxApp, plan: &ToolPlan) -> Result() + && let Ok(object_id) = local.parse::() && let Some(object) = app.doc.canvases[canvas].object(object_id) { actions.push(Action::rename_object( @@ -267,7 +267,7 @@ fn execute_scheme(app: &mut PlotxApp, plan: &ToolPlan) -> Result>(); let skipped = scheme_plan .targets() @@ -278,7 +278,7 @@ fn execute_scheme(app: &mut PlotxApp, plan: &ToolPlan) -> Result Result Result Result Result Option { app.doc .datasets .iter() - .position(|dataset| dataset.resource_id() == id) + .position(|dataset| dataset.resource_id().to_string() == id) } fn canvas_index(app: &PlotxApp, id: &str) -> Option { app.doc .canvases .iter() - .position(|canvas| canvas.resource_id == id) + .position(|canvas| canvas.resource_id.to_string() == id) } fn serde_result( diff --git a/crates/core/src/automation/types.rs b/crates/core/src/automation/types.rs index 24f5a1f..8c99d69 100644 --- a/crates/core/src/automation/types.rs +++ b/crates/core/src/automation/types.rs @@ -1,6 +1,10 @@ use serde::{Deserialize, Serialize}; use std::collections::BTreeMap; use std::path::PathBuf; +use std::{error::Error, fmt}; + +use crate::state::{CanvasId, DatasetId, SeriesId}; +use plotx_processing::StepId; #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)] #[serde(transparent)] @@ -37,6 +41,114 @@ pub struct ResourceRef { pub local_id: Option, } +#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde( + tag = "kind", + content = "id", + rename_all = "snake_case", + deny_unknown_fields +)] +pub enum ComponentRef { + Series(SeriesId), + ProcessingStep(StepId), +} + +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct TargetRef { + pub resource: ResourceRef, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub component: Option, +} + +impl TargetRef { + pub fn resource(resource: ResourceRef) -> Self { + Self { + resource, + component: None, + } + } +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum ResourceIdError { + WrongKind { + expected: &'static str, + actual: String, + }, + ChildResource, + InvalidUuid(String), +} + +impl fmt::Display for ResourceIdError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::WrongKind { expected, actual } => { + write!(f, "expected resource kind {expected}, got {actual}") + } + Self::ChildResource => f.write_str("expected a top-level resource"), + Self::InvalidUuid(value) => write!(f, "invalid UUID resource id {value}"), + } + } +} + +impl Error for ResourceIdError {} + +fn top_level_uuid( + resource: &ResourceRef, + expected_kind: &'static str, +) -> Result { + if resource.kind.0 != expected_kind { + return Err(ResourceIdError::WrongKind { + expected: expected_kind, + actual: resource.kind.0.clone(), + }); + } + if resource.parent_id.is_some() || resource.local_id.is_some() { + return Err(ResourceIdError::ChildResource); + } + uuid::Uuid::parse_str(&resource.id) + .map_err(|_| ResourceIdError::InvalidUuid(resource.id.clone())) +} + +impl From for ResourceRef { + fn from(id: DatasetId) -> Self { + Self { + id: id.to_string(), + kind: ResourceKindId::new(crate::automation::KIND_DATASET), + parent_id: None, + local_id: None, + } + } +} + +impl TryFrom<&ResourceRef> for DatasetId { + type Error = ResourceIdError; + + fn try_from(resource: &ResourceRef) -> Result { + top_level_uuid(resource, crate::automation::KIND_DATASET).map(Self::from_uuid) + } +} + +impl From for ResourceRef { + fn from(id: CanvasId) -> Self { + Self { + id: id.to_string(), + kind: ResourceKindId::new(crate::automation::KIND_CANVAS), + parent_id: None, + local_id: None, + } + } +} + +impl TryFrom<&ResourceRef> for CanvasId { + type Error = ResourceIdError; + + fn try_from(resource: &ResourceRef) -> Result { + top_level_uuid(resource, crate::automation::KIND_CANVAS).map(Self::from_uuid) + } +} + #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] #[serde(deny_unknown_fields)] pub struct ResourceDescriptor { @@ -331,6 +443,10 @@ pub struct TableRevisionRecord { pub followed_latest: bool, } +#[cfg(test)] +#[path = "types_tests.rs"] +mod tests; + #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] #[serde(deny_unknown_fields)] pub struct TablePlanRunRecord { diff --git a/crates/core/src/automation/types_tests.rs b/crates/core/src/automation/types_tests.rs new file mode 100644 index 0000000..8ed7773 --- /dev/null +++ b/crates/core/src/automation/types_tests.rs @@ -0,0 +1,79 @@ +use super::*; + +#[test] +fn typed_global_ids_round_trip_through_resource_dtos() { + let dataset = DatasetId::new(); + let canvas = CanvasId::new(); + + let dataset_ref = ResourceRef::from(dataset); + let canvas_ref = ResourceRef::from(canvas); + + assert_eq!(DatasetId::try_from(&dataset_ref), Ok(dataset)); + assert_eq!(CanvasId::try_from(&canvas_ref), Ok(canvas)); +} + +#[test] +fn typed_resource_conversion_rejects_wrong_kind_children_and_invalid_uuids() { + let dataset = DatasetId::new(); + let dataset_ref = ResourceRef::from(dataset); + + assert!(matches!( + CanvasId::try_from(&dataset_ref), + Err(ResourceIdError::WrongKind { .. }) + )); + + let mut child = dataset_ref.clone(); + child.parent_id = Some(dataset.to_string()); + child.local_id = Some("child".into()); + assert_eq!( + DatasetId::try_from(&child), + Err(ResourceIdError::ChildResource) + ); + + let invalid = ResourceRef { + id: "not-a-uuid".into(), + kind: ResourceKindId::new(crate::automation::KIND_DATASET), + parent_id: None, + local_id: None, + }; + assert!(matches!( + DatasetId::try_from(&invalid), + Err(ResourceIdError::InvalidUuid(_)) + )); +} + +#[test] +fn target_ref_serializes_resource_and_typed_components() { + let resource = ResourceRef::from(DatasetId::new()); + let resource_only = TargetRef::resource(resource.clone()); + let json = serde_json::to_value(&resource_only).unwrap(); + assert!(json.get("component").is_none()); + + let series_target = TargetRef { + resource: resource.clone(), + component: Some(ComponentRef::Series(SeriesId::new(42))), + }; + assert_eq!( + serde_json::to_value(&series_target).unwrap()["component"], + serde_json::json!({"kind": "series", "id": 42}) + ); + + let step_target = TargetRef { + resource, + component: Some(ComponentRef::ProcessingStep(StepId(9))), + }; + let encoded = serde_json::to_string(&step_target).unwrap(); + assert_eq!( + serde_json::from_str::(&encoded).unwrap(), + step_target + ); +} + +#[test] +fn target_ref_rejects_unknown_component_kinds() { + let json = serde_json::json!({ + "resource": ResourceRef::from(DatasetId::new()), + "component": {"kind": "field", "id": 1} + }); + assert!(serde_json::from_value::(json).is_err()); +} diff --git a/crates/core/src/automation/workflow.rs b/crates/core/src/automation/workflow.rs index c19de4b..138c98e 100644 --- a/crates/core/src/automation/workflow.rs +++ b/crates/core/src/automation/workflow.rs @@ -460,9 +460,9 @@ fn typed_table_revisions(app: &PlotxApp, role: &str) -> BTreeMap Pa mod tests { use super::*; use crate::state::{ - CanvasDocument, CanvasObject, CanvasObjectKind, ObjectFrame, ShapeKind, ShapeObject, + CanvasDocument, CanvasObject, CanvasObjectKind, ObjectFrame, ObjectId, ShapeKind, + ShapeObject, }; fn canvas(name: &str, size_mm: [f32; 2]) -> CanvasDocument { @@ -520,7 +521,7 @@ mod tests { fn canvas_with_shape(frame: ObjectFrame) -> CanvasDocument { let mut canvas = canvas("page", [100.0, 80.0]); canvas.objects.push(CanvasObject { - id: 1, + id: ObjectId::new(1), name: "shape".into(), frame, locked: false, diff --git a/crates/core/src/export/precheck.rs b/crates/core/src/export/precheck.rs index c330d96..092bb91 100644 --- a/crates/core/src/export/precheck.rs +++ b/crates/core/src/export/precheck.rs @@ -199,7 +199,7 @@ mod tests { use super::*; use crate::state::{ AxisOverrides, AxisProjections, CanvasObject, CanvasObjectKind, CanvasViewport, ChartSpec, - DataBinding, ObjectFrame, PanelMeta, PlotObject, StackSpec, + DataBinding, ObjectFrame, ObjectId, PanelMeta, PlotObject, StackSpec, }; use plotx_figure::{Axis, Figure}; @@ -267,14 +267,14 @@ mod tests { panel.visible = false; let mut canvas = CanvasDocument::new("Hidden axes".to_owned(), [200.0, 100.0]); canvas.objects.push(CanvasObject { - id: 1, + id: ObjectId::new(1), name: "Plot".to_owned(), frame: ObjectFrame::new(0.0, 0.0, 100.0, 100.0), locked: false, visible: true, group: None, kind: CanvasObjectKind::Plot(Box::new(PlotObject { - binding: DataBinding::single(0), + binding: DataBinding::single(crate::state::DatasetId::new()), chart: ChartSpec::default(), stack: StackSpec::default(), projections: AxisProjections::default(), diff --git a/crates/core/src/layout.rs b/crates/core/src/layout.rs index 7162b0e..0b8a05d 100644 --- a/crates/core/src/layout.rs +++ b/crates/core/src/layout.rs @@ -1,8 +1,6 @@ //! Page-layout geometry: a panel grid plus the snapping math that pulls a dragged //! or resized object onto page edges, centre lines, margins and cell boundaries. - use crate::state::{MM_TO_PT, ObjectFrame, ObjectId}; - mod visual_spacing; pub use visual_spacing::{ GutterPreset, LayoutItem, OccupiedGrid, SpacingMode, TilingDropRegion, arrange_grid, @@ -495,10 +493,10 @@ mod tests { }; let g = 5.0 * MM_TO_PT; // Pointer well to the right of centre → left/right split, newcomer right. - let plan = compute_tiling_plan(page, &layout, &[7], [360.0, 150.0]); + let plan = compute_tiling_plan(page, &layout, &[ObjectId::new(7)], [360.0, 150.0]); assert_eq!(plan.existing.len(), 1); let (id, ex) = plan.existing[0]; - assert_eq!(id, 7); + assert_eq!(id, ObjectId::new(7)); let nc = plan.newcomer; assert!(nc.x > ex.x, "newcomer takes the right half"); assert!( @@ -512,7 +510,8 @@ mod tests { #[test] fn multi_plot_retile_grids_all_including_newcomer() { let page = [400.0, 300.0]; - let plan = compute_tiling_plan(page, &PageLayout::default(), &[1, 2], [5.0, 5.0]); + let ids = [ObjectId::new(1), ObjectId::new(2)]; + let plan = compute_tiling_plan(page, &PageLayout::default(), &ids, [5.0, 5.0]); assert_eq!(plan.existing.len(), 2, "both existing plots reframed"); let frames = [plan.existing[0].1, plan.existing[1].1, plan.newcomer]; for i in 0..frames.len() { @@ -524,8 +523,7 @@ mod tests { assert!(f.x >= -0.01 && f.y >= -0.01); assert!(f.x + f.width <= page[0] + 0.01 && f.y + f.height <= page[1] + 0.01); } - let bottom_right = - compute_tiling_plan(page, &PageLayout::default(), &[1, 2], [395.0, 295.0]); + let bottom_right = compute_tiling_plan(page, &PageLayout::default(), &ids, [395.0, 295.0]); assert!(bottom_right.newcomer.x > page[0] * 0.5 && bottom_right.newcomer.y > page[1] * 0.5); } @@ -606,11 +604,11 @@ mod tests { }; let items = [ LayoutItem { - id: 1, + id: ObjectId::new(1), insets: [20.0; 4], }, LayoutItem { - id: 2, + id: ObjectId::new(2), insets: [30.0; 4], }, ]; @@ -630,11 +628,11 @@ mod tests { }; let items = [ LayoutItem { - id: 1, + id: ObjectId::new(1), insets: [10.0, 4.0, 18.0, 24.0], }, LayoutItem { - id: 2, + id: ObjectId::new(2), insets: [8.0, 5.0, 16.0, 6.0], }, ]; @@ -660,21 +658,21 @@ mod tests { }; let full = [ LayoutItem { - id: 1, + id: ObjectId::new(1), insets: [20.0; 4], }, LayoutItem { - id: 2, + id: ObjectId::new(2), insets: [20.0; 4], }, ]; let simple = [ LayoutItem { - id: 1, + id: ObjectId::new(1), insets: [5.0; 4], }, LayoutItem { - id: 2, + id: ObjectId::new(2), insets: [5.0; 4], }, ]; @@ -738,8 +736,8 @@ mod tests { #[test] fn align_left_and_right_pin_to_bounding_edges() { let frames = vec![ - (1u64, ObjectFrame::new(10.0, 0.0, 20.0, 10.0)), - (2, ObjectFrame::new(50.0, 40.0, 40.0, 10.0)), + (ObjectId::new(1), ObjectFrame::new(10.0, 0.0, 20.0, 10.0)), + (ObjectId::new(2), ObjectFrame::new(50.0, 40.0, 40.0, 10.0)), ]; let left = align(&frames, Align::Left); assert!(approx(left[0].1.x, 10.0) && approx(left[1].1.x, 10.0)); @@ -751,8 +749,8 @@ mod tests { #[test] fn align_hcenter_centres_each_frame_on_bbox_centre() { let frames = vec![ - (1u64, ObjectFrame::new(0.0, 0.0, 20.0, 10.0)), - (2, ObjectFrame::new(80.0, 0.0, 20.0, 10.0)), + (ObjectId::new(1), ObjectFrame::new(0.0, 0.0, 20.0, 10.0)), + (ObjectId::new(2), ObjectFrame::new(80.0, 0.0, 20.0, 10.0)), ]; // bbox spans 0..100, centre 50. let out = align(&frames, Align::HCenter); @@ -762,9 +760,9 @@ mod tests { #[test] fn distribute_horizontal_equalises_centre_spacing() { let frames = vec![ - (1u64, ObjectFrame::new(0.0, 0.0, 10.0, 10.0)), - (2, ObjectFrame::new(12.0, 0.0, 10.0, 10.0)), - (3, ObjectFrame::new(90.0, 0.0, 10.0, 10.0)), + (ObjectId::new(1), ObjectFrame::new(0.0, 0.0, 10.0, 10.0)), + (ObjectId::new(2), ObjectFrame::new(12.0, 0.0, 10.0, 10.0)), + (ObjectId::new(3), ObjectFrame::new(90.0, 0.0, 10.0, 10.0)), ]; // Centres: 5, 17, 95 → after: 5, 50, 95 (step 45); middle x = 45. let out = distribute(&frames, Distribute::Horizontal); @@ -776,8 +774,8 @@ mod tests { #[test] fn distribute_needs_three_frames() { let frames = vec![ - (1u64, ObjectFrame::new(0.0, 0.0, 10.0, 10.0)), - (2, ObjectFrame::new(90.0, 0.0, 10.0, 10.0)), + (ObjectId::new(1), ObjectFrame::new(0.0, 0.0, 10.0, 10.0)), + (ObjectId::new(2), ObjectFrame::new(90.0, 0.0, 10.0, 10.0)), ]; let out = distribute(&frames, Distribute::Horizontal); assert_eq!(out, frames); diff --git a/crates/core/src/layout/visual_spacing.rs b/crates/core/src/layout/visual_spacing.rs index bcdad5f..aea45ba 100644 --- a/crates/core/src/layout/visual_spacing.rs +++ b/crates/core/src/layout/visual_spacing.rs @@ -416,9 +416,9 @@ fn split_plan( mod tests { use super::*; - fn item(id: ObjectId, inset: f32) -> LayoutItem { + fn item(id: u64, inset: f32) -> LayoutItem { LayoutItem { - id, + id: ObjectId::new(id), insets: [inset; 4], } } @@ -568,9 +568,9 @@ mod tests { assert_ne!(narrow.newcomer, wide.newcomer); } - fn frame(id: ObjectId, col: u32, row: u32) -> (ObjectId, ObjectFrame) { + fn frame(id: u64, col: u32, row: u32) -> (ObjectId, ObjectFrame) { ( - id, + ObjectId::new(id), ObjectFrame::new(col as f32 * 20.0, row as f32 * 30.0, 10.0, 10.0), ) } @@ -585,7 +585,7 @@ mod tests { ]) .unwrap(); assert_eq!((grid.rows, grid.cols), (2, 2)); - assert_eq!(grid.ids, vec![1, 2, 3, 4]); + assert_eq!(grid.ids, [1, 2, 3, 4].map(ObjectId::new)); } #[test] @@ -599,18 +599,21 @@ mod tests { ]) .unwrap(); assert_eq!((grid.rows, grid.cols), (2, 3)); - assert_eq!(grid.ids, vec![1, 2, 3, 4, 5]); + assert_eq!(grid.ids, [1, 2, 3, 4, 5].map(ObjectId::new)); } #[test] fn occupied_grid_accepts_single_row_and_single_column() { let row = infer_occupied_grid(&[frame(2, 1, 0), frame(1, 0, 0), frame(3, 2, 0)]).unwrap(); - assert_eq!((row.rows, row.cols, row.ids), (1, 3, vec![1, 2, 3])); + assert_eq!( + (row.rows, row.cols, row.ids), + (1, 3, [1, 2, 3].map(ObjectId::new).to_vec()) + ); let column = infer_occupied_grid(&[frame(3, 0, 2), frame(1, 0, 0), frame(2, 0, 1)]).unwrap(); assert_eq!( (column.rows, column.cols, column.ids), - (3, 1, vec![1, 2, 3]) + (3, 1, [1, 2, 3].map(ObjectId::new).to_vec()) ); } @@ -627,8 +630,8 @@ mod tests { #[test] fn occupied_grid_rejects_two_objects_in_one_tolerance_cell() { let frames = [ - (1, ObjectFrame::new(0.0, 0.0, 10.0, 10.0)), - (2, ObjectFrame::new(0.5, 0.5, 10.0, 10.0)), + (ObjectId::new(1), ObjectFrame::new(0.0, 0.0, 10.0, 10.0)), + (ObjectId::new(2), ObjectFrame::new(0.5, 0.5, 10.0, 10.0)), ]; assert!(infer_occupied_grid(&frames).is_none()); } diff --git a/crates/core/src/project/convert.rs b/crates/core/src/project/convert.rs index b77951c..0060f9b 100644 --- a/crates/core/src/project/convert.rs +++ b/crates/core/src/project/convert.rs @@ -4,7 +4,6 @@ use super::electrophysiology_convert::{ electrophysiology_from_object, electrophysiology_to_objects, }; use super::*; - pub fn dataset_to_objects( dataset: &Dataset, data_id: &str, @@ -153,7 +152,6 @@ pub fn dataset_to_objects( } }) } - pub fn object_to_dataset( zip: &mut zip::ZipArchive, data: &DataObject, @@ -302,9 +300,7 @@ pub fn object_to_dataset( ))), } } - use crate::state::{AxisProjection, AxisProjections, ProjectionSource}; - fn projections_to_dto(p: &AxisProjections, datasets: &[Dataset]) -> Result> { if p.is_empty() { return Ok(None); @@ -314,7 +310,6 @@ fn projections_to_dto(p: &AxisProjections, datasets: &[Dataset]) -> Result, + datasets: &[Dataset], ) -> AxisProjections { AxisProjections { - top: axis_projection_from_dto(dto.top.as_ref(), recipe_to_dataset), - left: axis_projection_from_dto(dto.left.as_ref(), recipe_to_dataset), + top: axis_projection_from_dto(dto.top.as_ref(), recipe_to_dataset, datasets), + left: axis_projection_from_dto(dto.left.as_ref(), recipe_to_dataset, datasets), } } - fn axis_projection_from_dto( dto: Option<&AxisProjectionDto>, recipe_to_dataset: &HashMap, + datasets: &[Dataset], ) -> AxisProjection { let Some(dto) = dto else { return AxisProjection::default(); @@ -370,6 +366,8 @@ fn axis_projection_from_dto( .attached .as_ref() .and_then(|id| recipe_to_dataset.get(id).copied()) + .and_then(|index| datasets.get(index)) + .map(Dataset::resource_id) .map(ProjectionSource::Attached) .unwrap_or(ProjectionSource::None), "sum" => ProjectionSource::Sum, @@ -382,7 +380,6 @@ fn axis_projection_from_dto( visible: dto.visible, } } - pub fn canvas_to_view( datasets: &[Dataset], canvas: &CanvasDocument, @@ -420,8 +417,16 @@ pub fn canvas_to_view( }; match &object.kind { CanvasObjectKind::Plot(plot) => { - let primary = plot.primary_dataset(); - let primary_dataset = datasets.get(primary).ok_or_else(|| { + let primary = plot.primary_dataset().ok_or_else(|| { + ProjectError::Invalid(format!( + "view {view_id} plot {} has no primary dataset", + object.id + )) + })?; + let primary_dataset = datasets + .iter() + .find(|dataset| dataset.resource_id() == primary) + .ok_or_else(|| { ProjectError::Invalid(format!( "view {view_id} plot {} references missing primary dataset {primary}", object.id @@ -442,7 +447,10 @@ pub fn canvas_to_view( .series .iter() .map(|sb| { - let dataset = datasets.get(sb.dataset).ok_or_else(|| { + let dataset = datasets + .iter() + .find(|dataset| dataset.resource_id() == sb.dataset) + .ok_or_else(|| { ProjectError::Invalid(format!( "view {view_id} plot {} references missing series dataset {}", object.id, sb.dataset @@ -508,6 +516,7 @@ pub fn canvas_to_view( .filter(|input| !input.is_empty()) .collect(), name: canvas.name.clone(), + next_object_id: canvas.next_object_id.get(), caption: canvas.caption.clone(), caption_visible: canvas.caption_visible, panel_label_style: Some(canvas.panel_label_style.as_key().to_owned()), @@ -528,7 +537,6 @@ pub fn canvas_to_view( snapshot: None, }) } - pub fn view_to_canvas( app: &mut PlotxApp, zip: &mut zip::ZipArchive, @@ -564,7 +572,7 @@ pub fn view_to_canvas( for view_object in &view.objects { let object_id = view_object .id - .parse::() + .parse::() .map_err(|_| ProjectError::Invalid(format!("invalid object id {}", view_object.id)))?; let frame = view_object.frame.into_frame(); let kind = match view_object.kind.as_str() { @@ -586,12 +594,16 @@ pub fn view_to_canvas( }) }; let binding = if view_object.series.is_empty() { - DataBinding::single(resolve(&view_object.input)?) + let index = resolve(&view_object.input)?; + DataBinding::single(app.doc.datasets[index].resource_id()) } else { let mut series = Vec::with_capacity(view_object.series.len()); for sb in &view_object.series { series.push(SeriesBinding { - dataset: resolve(&sb.input)?, + dataset: { + let index = resolve(&sb.input)?; + app.doc.datasets[index].resource_id() + }, color: sb.color.map(|c| plotx_figure::Color::rgb(c[0], c[1], c[2])), label: sb.label.clone(), scale: sb.scale, @@ -605,7 +617,15 @@ pub fn view_to_canvas( .clone() .map(StackDto::into_spec) .unwrap_or_default(); - let di = binding.primary_dataset(); + let dataset_id = binding.primary_dataset().ok_or_else(|| { + ProjectError::Invalid(format!("view {} has an empty data binding", view.id)) + })?; + let di = app.doc.dataset_index(dataset_id).ok_or_else(|| { + ProjectError::Invalid(format!( + "view {} references missing dataset {dataset_id}", + view.id + )) + })?; let domain = app .doc .datasets @@ -647,7 +667,7 @@ pub fn view_to_canvas( let projections = view_object .projections .as_ref() - .map(|dto| projections_from_dto(dto, recipe_to_dataset)) + .map(|dto| projections_from_dto(dto, recipe_to_dataset, &app.doc.datasets)) .unwrap_or_default(); let mut figure = if let Some(snapshot) = &view_object.snapshot { read_json(zip, &snapshot.figure).unwrap_or_else(|_| { @@ -706,14 +726,16 @@ pub fn view_to_canvas( group: view_object.group, kind, }); - max_id = max_id.max(object_id); + max_id = max_id.max(object_id.get()); max_group = max_group.max(view_object.group.unwrap_or(0)); } - canvas.next_object_id = max_id + 1; + let repaired_next = ObjectId::new(max_id) + .try_advance(1) + .ok_or_else(|| ProjectError::Invalid("object id space exhausted".to_owned()))?; + canvas.next_object_id = ObjectId::new(view.next_object_id).max(repaired_next); canvas.next_group_id = max_group + 1; Ok(canvas) } - fn text_box_from(view_object: &ViewCanvasObject, panel: bool) -> TextBox { view_object .text @@ -727,7 +749,6 @@ fn text_box_from(view_object: &ViewCanvasObject, panel: bool) -> TextBox { } }) } - pub fn dimension_from_1d(data: &NmrData) -> Dimension { Dimension { id: "f2".to_owned(), @@ -744,7 +765,6 @@ pub fn dimension_from_1d(data: &NmrData) -> Dimension { group_delay: Some(data.group_delay), } } - pub fn dimension_from_dim( id: &str, role: &str, diff --git a/crates/core/src/project/dto.rs b/crates/core/src/project/dto.rs index eb9dda1..f7f7c74 100644 --- a/crates/core/src/project/dto.rs +++ b/crates/core/src/project/dto.rs @@ -263,6 +263,7 @@ pub struct ViewObject { #[serde(default)] pub inputs: Vec, pub name: String, + pub next_object_id: u64, #[serde(default, skip_serializing_if = "String::is_empty")] pub caption: String, #[serde(default = "caption_visible_default")] @@ -713,8 +714,8 @@ pub struct ViewportDto { #[derive(Clone, Serialize, Deserialize)] pub struct SelectionDto { - pub dataset: usize, - pub canvas: usize, + pub dataset: String, + pub canvas: String, pub object: String, pub x_range: RangeDto, #[serde(skip_serializing_if = "Option::is_none")] @@ -774,8 +775,8 @@ impl RangeDto { impl SelectionDto { pub fn from_selection(selection: &AnalysisSelection) -> Self { Self { - dataset: selection.dataset, - canvas: selection.canvas, + dataset: selection.dataset.to_string(), + canvas: selection.canvas.to_string(), object: selection.object.to_string(), x_range: RangeDto::from_range(selection.x_range), y_range: selection.y_range.map(RangeDto::from_range), @@ -784,8 +785,8 @@ impl SelectionDto { pub fn to_selection(&self) -> Option { Some(AnalysisSelection { - dataset: self.dataset, - canvas: self.canvas, + dataset: self.dataset.parse().ok()?, + canvas: self.canvas.parse().ok()?, object: self.object.parse().ok()?, x_range: self.x_range.into_range(), y_range: self.y_range.map(RangeDto::into_range), diff --git a/crates/core/src/project/electrophysiology_tests.rs b/crates/core/src/project/electrophysiology_tests.rs index 1025b66..5ce3e60 100644 --- a/crates/core/src/project/electrophysiology_tests.rs +++ b/crates/core/src/project/electrophysiology_tests.rs @@ -39,7 +39,7 @@ fn project_roundtrip_preserves_raw_data_and_settings() { .remove("resource_id"); let legacy_recording: crate::state::ElectrophysiologyDataset = serde_json::from_value(legacy_metadata).unwrap(); - assert!(!legacy_recording.resource_id.is_empty()); + assert!(!legacy_recording.resource_id.to_string().is_empty()); let mut app = PlotxApp::new(); app.doc .datasets diff --git a/crates/core/src/project/lineage_tests.rs b/crates/core/src/project/lineage_tests.rs index f8a417c..3e1349a 100644 --- a/crates/core/src/project/lineage_tests.rs +++ b/crates/core/src/project/lineage_tests.rs @@ -13,10 +13,15 @@ fn project_roundtrip_maps_multi_source_lineage_by_data_id() { app.doc .datasets .push(Dataset::Nmr(Box::new(NmrDataset::load(synthetic_1d())))); + let sources = [ + app.doc.datasets[1].resource_id(), + app.doc.datasets[0].resource_id(), + app.doc.datasets[1].resource_id(), + ]; let mut derived = Dataset::Nmr(Box::new(NmrDataset::load(synthetic_1d()))); derived.set_lineage(Some(DatasetLineage::new( DerivationKind::SpectrumArithmetic, - [1, 0, 1], + sources, ))); app.doc.datasets.push(derived); @@ -30,7 +35,10 @@ fn project_roundtrip_maps_multi_source_lineage_by_data_id() { loaded.doc.datasets[2].lineage(), Some(&DatasetLineage::new( DerivationKind::SpectrumArithmetic, - [1, 0] + [ + loaded.doc.datasets[1].resource_id(), + loaded.doc.datasets[0].resource_id(), + ] )) ); } @@ -41,7 +49,7 @@ fn provenance_without_explicit_v1_lineage_stays_unlinked() { app.doc .datasets .push(Dataset::Nmr(Box::new(NmrDataset::load(synthetic_1d())))); - let source_resource = app.doc.datasets[0].resource_id().to_owned(); + let source_resource = app.doc.datasets[0].resource_id().to_string(); let mut table = materialized_float_series_table( ("x".into(), "".into(), vec![Some(0.0)]), Vec::new(), @@ -129,9 +137,10 @@ fn v1_table_roundtrip_preserves_units_missing_uncertainty_and_lineage() { ); source.name = Some("measurements.csv".into()); table.import_sources.push(source); + let source_id = app.doc.datasets[0].resource_id(); table.lineage = Some(DatasetLineage::new( DerivationKind::WindowStatisticsTable, - [0], + [source_id], )); app.doc.datasets.push(Dataset::Table(Box::new(table))); @@ -178,7 +187,7 @@ fn v1_table_roundtrip_preserves_units_missing_uncertainty_and_lineage() { table.lineage, Some(DatasetLineage::new( DerivationKind::WindowStatisticsTable, - [0] + [loaded.doc.datasets[0].resource_id()] )) ); } diff --git a/crates/core/src/project/mod.rs b/crates/core/src/project/mod.rs index 476a126..b9f4b28 100644 --- a/crates/core/src/project/mod.rs +++ b/crates/core/src/project/mod.rs @@ -2,8 +2,8 @@ use crate::layout::PageLayout; use crate::state::{ AnalysisSelection, AxisRange, CanvasDocument, CanvasObject, CanvasObjectKind, CanvasViewport, DataBinding, Dataset, DatasetLineage, DerivationKind, Nmr2DDataset, NmrDataset, ObjectFrame, - PanelMeta, PlotObject, PlotxApp, PrimaryView, Region, RegionMetric, SeriesBinding, ShapeKind, - ShapeObject, StackMode, StackSpec, TextAlign, TextBox, Tool, + ObjectId, PanelMeta, PlotObject, PlotxApp, PrimaryView, Region, RegionMetric, SeriesBinding, + ShapeKind, ShapeObject, StackMode, StackSpec, TextAlign, TextBox, Tool, }; use num_complex::Complex64; use plotx_figure::Color; @@ -238,7 +238,7 @@ fn save_project_impl( let mut bindings = Vec::with_capacity(doc.datasets.len()); let mut written_table_blocks = std::collections::BTreeSet::new(); for dataset in &doc.datasets { - let data_id = dataset.resource_id().to_owned(); + let data_id = dataset.resource_id().to_string(); let recipe_id = format!("recipe_{data_id}"); let data_path = format!("objects/{data_id}/object.json"); let recipe_path = format!("objects/{recipe_id}/object.json"); @@ -280,8 +280,9 @@ fn save_project_impl( .iter() .map(|source| { doc.datasets - .get(*source) - .map(|dataset| dataset.resource_id().to_owned()) + .iter() + .find(|dataset| dataset.resource_id() == *source) + .map(|dataset| dataset.resource_id().to_string()) .ok_or_else(|| { ProjectError::Invalid(format!( "dataset {data_id} references missing lineage source {source}" @@ -305,7 +306,7 @@ fn save_project_impl( let mut view_order = Vec::with_capacity(doc.canvases.len()); for canvas in &doc.canvases { - let view_id = canvas.resource_id.clone(); + let view_id = canvas.resource_id.to_string(); let view_path = format!("views/{view_id}.json"); let mut view = canvas_to_view(&doc.datasets, canvas, &view_id)?; if include_view_snapshots { @@ -356,11 +357,11 @@ fn save_project_impl( active_data: workspace_state .active_dataset .and_then(|i| doc.datasets.get(i)) - .map(|dataset| dataset.resource_id().to_owned()), + .map(|dataset| dataset.resource_id().to_string()), active_view: workspace_state .active_canvas .and_then(|i| doc.canvases.get(i)) - .map(|canvas| canvas.resource_id.clone()), + .map(|canvas| canvas.resource_id.to_string()), primary_view: workspace_state.primary_view.clone(), tool: workspace_state.tool.clone(), analysis_selection: workspace_state.analysis_selection.clone(), @@ -430,7 +431,9 @@ pub fn load_project(path: &Path) -> Result { ProjectError::Invalid(format!("missing recipe object {}", binding.recipe)) })?; let mut dataset = object_to_dataset(&mut zip, data, recipe)?; - dataset.set_resource_id(binding.data.clone()); + dataset.set_resource_id(binding.data.parse().map_err(|_| { + ProjectError::Invalid(format!("dataset has invalid stable id {}", binding.data)) + })?); let di = app.doc.datasets.len(); app.doc.datasets.push(dataset); recipe_to_dataset.insert(binding.recipe.clone(), di); @@ -455,7 +458,9 @@ pub fn load_project(path: &Path) -> Result { .ok_or_else(|| ProjectError::Invalid(format!("missing view object {view_id}")))?; let mut canvas = view_to_canvas(&mut app, &mut zip, view_id, view, index, &recipe_to_dataset)?; - canvas.resource_id = view_id.clone(); + canvas.resource_id = view_id.parse().map_err(|_| { + ProjectError::Invalid(format!("canvas has invalid stable id {view_id}")) + })?; app.doc.canvases.push(canvas); } @@ -514,11 +519,11 @@ fn validate_resource_ids(doc: &crate::state::Document) -> Result<()> { for (kind, id) in doc .datasets .iter() - .map(|dataset| ("dataset", dataset.resource_id())) + .map(|dataset| ("dataset", dataset.resource_id().to_string())) .chain( doc.canvases .iter() - .map(|canvas| ("canvas", canvas.resource_id.as_str())), + .map(|canvas| ("canvas", canvas.resource_id.to_string())), ) { if id.is_empty() @@ -589,18 +594,19 @@ fn resolve_dataset_lineage( } let mut sources = Vec::with_capacity(dto.sources.len()); for source_id in &dto.sources { - let source = data_to_dataset.get(source_id).copied().ok_or_else(|| { + let source_index = data_to_dataset.get(source_id).copied().ok_or_else(|| { ProjectError::Invalid(format!( "dataset {} references missing lineage source {source_id}", binding.data )) })?; - if source == di { + if source_index == di { return Err(ProjectError::Invalid(format!( "dataset {} cannot derive from itself", binding.data ))); } + let source = datasets[source_index].resource_id(); if !sources.contains(&source) { sources.push(source); } @@ -634,7 +640,16 @@ fn validate_lineage_acyclic(datasets: &[Dataset], bindings: &[DatasetBinding]) - state[di] = 1; if let Some(lineage) = datasets[di].lineage() { for &source in &lineage.sources { - visit(source, datasets, state, bindings)?; + let source_index = datasets + .iter() + .position(|dataset| dataset.resource_id() == source) + .ok_or_else(|| { + ProjectError::Invalid(format!( + "dataset {} references missing lineage source {source}", + bindings[di].data + )) + })?; + visit(source_index, datasets, state, bindings)?; } } state[di] = 2; diff --git a/crates/core/src/project/reference_tests.rs b/crates/core/src/project/reference_tests.rs index 3ba5335..b4efe0d 100644 --- a/crates/core/src/project/reference_tests.rs +++ b/crates/core/src/project/reference_tests.rs @@ -1,6 +1,6 @@ use super::tests::{first_plot_mut, sample_app, temp_project}; use super::*; -use crate::state::{DatasetLineage, DerivationKind, ProjectionSource, SeriesBinding}; +use crate::state::{DatasetId, DatasetLineage, DerivationKind, ProjectionSource, SeriesBinding}; fn save_error(app: &PlotxApp, name: &str) -> String { let path = temp_project(name); @@ -13,36 +13,120 @@ fn save_error(app: &PlotxApp, name: &str) -> String { #[test] fn save_rejects_missing_lineage_source() { let mut app = sample_app(); - app.doc.datasets[0].set_lineage(Some(DatasetLineage::new(DerivationKind::Projection, [99]))); + let missing = DatasetId::new(); + app.doc.datasets[0].set_lineage(Some(DatasetLineage::new( + DerivationKind::Projection, + [missing], + ))); let error = save_error(&app, "missing-lineage-source"); - assert!(error.contains("missing lineage source 99"), "{error}"); + assert!( + error.contains("references missing lineage source"), + "{error}" + ); } #[test] fn save_rejects_missing_primary_and_series_datasets() { let mut missing_primary = sample_app(); - first_plot_mut(&mut missing_primary).binding.series[0].dataset = 99; + let missing = DatasetId::new(); + first_plot_mut(&mut missing_primary).binding.series[0].dataset = missing; let error = save_error(&missing_primary, "missing-primary-dataset"); - assert!(error.contains("missing primary dataset 99"), "{error}"); + assert!( + error.contains(&format!("missing primary dataset {missing}")), + "{error}" + ); let mut missing_series = sample_app(); + let missing = DatasetId::new(); first_plot_mut(&mut missing_series) .binding .series - .push(SeriesBinding::new(99)); + .push(SeriesBinding::new(missing)); let error = save_error(&missing_series, "missing-series-dataset"); - assert!(error.contains("missing series dataset 99"), "{error}"); + assert!( + error.contains(&format!("missing series dataset {missing}")), + "{error}" + ); } #[test] fn save_rejects_missing_attached_projection_dataset() { let mut app = sample_app(); - first_plot_mut(&mut app).projections.top.source = ProjectionSource::Attached(99); + let missing = DatasetId::new(); + first_plot_mut(&mut app).projections.top.source = ProjectionSource::Attached(missing); let error = save_error(&app, "missing-projection-dataset"); assert!( - error.contains("axis projection references missing dataset 99"), + error.contains(&format!( + "axis projection references missing dataset {missing}" + )), + "{error}" + ); +} + +#[test] +fn object_allocator_roundtrip_preserves_deleted_high_water_mark() { + let mut app = sample_app(); + let canvas = &mut app.doc.canvases[0]; + let first_unused = canvas.allocate_object_id(); + let high_water = canvas.next_object_id; + assert!(canvas.object(first_unused).is_none()); + + let path = temp_project("object_allocator"); + let _ = std::fs::remove_file(&path); + save_project(&app, &path, false).unwrap(); + let loaded = load_project(&path).unwrap(); + let _ = std::fs::remove_file(&path); + + assert_eq!(loaded.doc.canvases[0].next_object_id, high_water); +} + +#[test] +fn loading_a_maximum_object_id_reports_exhaustion() { + let app = sample_app(); + let path = temp_project("maximum-object-id"); + let _ = std::fs::remove_file(&path); + save_project(&app, &path, false).unwrap(); + let file = std::fs::File::open(&path).unwrap(); + let mut zip = zip::ZipArchive::new(file).unwrap(); + let view: ViewObject = serde_json::from_value(serde_json::json!({ + "id": "view-max-object-id", + "role": "canvas", + "classification": { + "domain": "visualization", + "object": "page" + }, + "name": "Maximum object id", + "next_object_id": 1, + "layout": { "size_mm": [120.0, 80.0] }, + "objects": [{ + "id": u64::MAX.to_string(), + "name": "Label", + "kind": "text", + "frame": { "x": 0.0, "y": 0.0, "width": 10.0, "height": 10.0 }, + "locked": false, + "visible": true + }] + })) + .unwrap(); + let mut loading_app = PlotxApp::new(); + + let error = match view_to_canvas( + &mut loading_app, + &mut zip, + "view-max-object-id", + &view, + 0, + &HashMap::new(), + ) { + Ok(_) => panic!("an exhausted object id space must be rejected"), + Err(error) => error, + }; + let _ = std::fs::remove_file(&path); + + assert!( + matches!(error, ProjectError::Invalid(ref message) if message.contains("object id space exhausted")), "{error}" ); } diff --git a/crates/core/src/project/schema_tests.rs b/crates/core/src/project/schema_tests.rs index 4f6c099..5e4e858 100644 --- a/crates/core/src/project/schema_tests.rs +++ b/crates/core/src/project/schema_tests.rs @@ -1,4 +1,4 @@ -use super::tests::temp_project; +use super::tests::{sample_app, temp_project}; use super::*; #[test] @@ -20,6 +20,49 @@ fn pre_release_projects_are_written_with_v1_schema() { std::fs::remove_file(path).unwrap(); } +#[test] +fn identity_schema_snapshot_uses_typed_text_and_persists_object_allocator() { + let mut app = sample_app(); + let dataset = app.doc.datasets[0].resource_id(); + let canvas = app.doc.canvases[0].resource_id; + let object = app.doc.canvases[0].objects[0].id; + app.session.ui.analysis_selection = Some(AnalysisSelection { + dataset, + canvas, + object, + x_range: AxisRange::new(1.0, 2.0), + y_range: None, + }); + + let path = temp_project("identity_schema_snapshot"); + let _ = std::fs::remove_file(&path); + save_project(&app, &path, false).unwrap(); + let file = File::open(&path).unwrap(); + let mut zip = zip::ZipArchive::new(file).unwrap(); + let manifest: Manifest = read_json(&mut zip, "manifest.json").unwrap(); + let view: serde_json::Value = read_json(&mut zip, &manifest.views[0].path).unwrap(); + let workspace: serde_json::Value = read_json(&mut zip, &manifest.workspace).unwrap(); + + assert_eq!( + view["next_object_id"], + app.doc.canvases[0].next_object_id.get() + ); + assert_eq!(view["objects"][0]["id"], object.to_string()); + assert_eq!( + workspace["analysis_selection"]["dataset"], + dataset.to_string() + ); + assert_eq!( + workspace["analysis_selection"]["canvas"], + canvas.to_string() + ); + assert_eq!( + workspace["analysis_selection"]["object"], + object.to_string() + ); + std::fs::remove_file(path).unwrap(); +} + #[test] fn pre_release_loader_rejects_non_v1_schema_without_a_migration_chain() { let manifest = Manifest { diff --git a/crates/core/src/project/tests.rs b/crates/core/src/project/tests.rs index 2d212fe..200ef4d 100644 --- a/crates/core/src/project/tests.rs +++ b/crates/core/src/project/tests.rs @@ -121,8 +121,8 @@ pub(super) fn sample_app() -> PlotxApp { app.session.active_canvas = Some(0); app.session.tool = Tool::SelectRegion; app.session.ui.analysis_selection = Some(AnalysisSelection { - dataset: 0, - canvas: 0, + dataset: app.doc.datasets[0].resource_id(), + canvas: app.doc.canvases[0].resource_id, object: id, x_range: AxisRange::new(1.0, 2.0), y_range: None, @@ -421,7 +421,7 @@ fn project_roundtrip_preserves_axis_projections() { app.build_plot_object(1, ObjectFrame::new(0.0, 0.0, w, h), id, "Plot 2".to_owned()); object.plot_mut().unwrap().projections = AxisProjections { top: AxisProjection { - source: ProjectionSource::Attached(0), + source: ProjectionSource::Attached(app.doc.datasets[0].resource_id()), visible: true, }, left: AxisProjection { @@ -442,7 +442,10 @@ fn project_roundtrip_preserves_axis_projections() { .plot() .unwrap() .projections; - assert_eq!(proj.top.source, ProjectionSource::Attached(0)); + assert_eq!( + proj.top.source, + ProjectionSource::Attached(loaded.doc.datasets[0].resource_id()) + ); assert!(proj.top.visible); assert_eq!(proj.left.source, ProjectionSource::Skyline); assert!(!proj.left.visible); @@ -579,9 +582,9 @@ fn project_roundtrip_preserves_overlay_binding() { app.build_plot_object(0, ObjectFrame::new(0.0, 0.0, w, h), id, "Plot 1".to_owned()); object.plot_mut().unwrap().binding = crate::state::DataBinding { series: vec![ - crate::state::SeriesBinding::new(0), + crate::state::SeriesBinding::new(app.doc.datasets[0].resource_id()), crate::state::SeriesBinding { - dataset: 1, + dataset: app.doc.datasets[1].resource_id(), color: Some(Color::rgb(10, 20, 30)), label: Some("treated".to_owned()), scale: 1.0, @@ -602,8 +605,14 @@ fn project_roundtrip_preserves_overlay_binding() { let binding = &first_plot(&loaded).binding; assert_eq!(binding.series.len(), 2); - assert_eq!(binding.series[0].dataset, 0); - assert_eq!(binding.series[1].dataset, 1); + assert_eq!( + binding.series[0].dataset, + loaded.doc.datasets[0].resource_id() + ); + assert_eq!( + binding.series[1].dataset, + loaded.doc.datasets[1].resource_id() + ); assert_eq!(binding.series[1].color, Some(Color::rgb(10, 20, 30))); assert_eq!(binding.series[1].label.as_deref(), Some("treated")); assert!(first_plot(&loaded).figure.show_legend); @@ -626,9 +635,9 @@ fn project_roundtrip_preserves_stack_spec_and_series_fields() { let plot = object.plot_mut().unwrap(); plot.binding = DataBinding { series: vec![ - SeriesBinding::new(0), + SeriesBinding::new(app.doc.datasets[0].resource_id()), SeriesBinding { - dataset: 1, + dataset: app.doc.datasets[1].resource_id(), color: None, label: None, scale: 2.5, diff --git a/crates/core/src/project/typed_table.rs b/crates/core/src/project/typed_table.rs index dcd79c8..3debb59 100644 --- a/crates/core/src/project/typed_table.rs +++ b/crates/core/src/project/typed_table.rs @@ -228,7 +228,9 @@ pub(crate) fn table_dataset_from_v1( }) .collect::>>()?; let mut dataset = crate::state::TableDataset { - resource_id: data.id.clone(), + resource_id: data.id.parse().map_err(|_| { + ProjectError::Invalid(format!("table has invalid stable id {}", data.id)) + })?, provenance: sidecar.provenance, meta: sidecar.meta, curve_fit_analyses: sidecar.curve_fit_analyses, diff --git a/crates/core/src/state/afm.rs b/crates/core/src/state/afm.rs index 747f8b5..b08af40 100644 --- a/crates/core/src/state/afm.rs +++ b/crates/core/src/state/afm.rs @@ -4,7 +4,7 @@ use std::sync::Arc; #[derive(Clone)] pub struct AfmDataset { - pub resource_id: String, + pub resource_id: DatasetId, pub data: Arc, pub name: Option, pub selected_channel: usize, @@ -18,7 +18,7 @@ impl AfmDataset { [forces.grid_width / 2, forces.grid_height / 2] }); Self { - resource_id: uuid::Uuid::new_v4().to_string(), + resource_id: DatasetId::new(), data: Arc::new(data), name: None, selected_channel: 0, diff --git a/crates/core/src/state/app_impl.rs b/crates/core/src/state/app_impl.rs index 38cd1be..454f266 100644 --- a/crates/core/src/state/app_impl.rs +++ b/crates/core/src/state/app_impl.rs @@ -128,7 +128,20 @@ impl PlotxApp { if binding.series.len() > 1 && self.series_stackable(binding) { self.build_stacked_figure(binding, stack, size_mm) } else { - let primary = binding.primary_dataset(); + let Some(primary_id) = binding.primary_dataset() else { + return Figure::new( + "", + plotx_figure::Axis::new("x", 0.0, 1.0), + plotx_figure::Axis::new("y", 0.0, 1.0), + ); + }; + let Some(primary) = self.doc.dataset_index(primary_id) else { + return Figure::new( + "", + plotx_figure::Axis::new("x", 0.0, 1.0), + plotx_figure::Axis::new("y", 0.0, 1.0), + ); + }; let domain = self.doc.datasets[primary].domain(); let mut fig = self.build_full_canvas_figure(primary, chart, size_mm); // A single-series colour override (e.g. a theme's primary trace colour) @@ -183,7 +196,12 @@ impl PlotxApp { size_mm: [f32; 2], ) -> Figure { let mut fig = self.build_binding_figure(binding, chart, stack, size_mm); - self.apply_axis_projections(&mut fig, binding.primary_dataset(), projections); + if let Some(dataset) = binding + .primary_dataset() + .and_then(|id| self.doc.dataset_index(id)) + { + self.apply_axis_projections(&mut fig, dataset, projections); + } fig } @@ -218,7 +236,12 @@ impl PlotxApp { } let slice = match &cfg.source { ProjectionSource::None => return None, - ProjectionSource::Attached(other) => return self.attached_axis_trace(*other), + ProjectionSource::Attached(other) => { + return self + .doc + .dataset_index(*other) + .and_then(|index| self.attached_axis_trace(index)); + } ProjectionSource::Sum => spec.project(kind, ProjectionMode::Sum), ProjectionSource::Skyline => spec.project(kind, ProjectionMode::Skyline), ProjectionSource::Slice(index) => spec.slice(kind, *index), @@ -290,6 +313,9 @@ impl PlotxApp { } pub fn rebuild_canvases_for(&mut self, dataset: usize) { + let Some(dataset_id) = self.doc.datasets.get(dataset).map(Dataset::resource_id) else { + return; + }; for ci in 0..self.doc.canvases.len() { let ids: Vec = self.doc.canvases[ci] .objects @@ -297,7 +323,7 @@ impl PlotxApp { .filter_map(|object| { object .plot() - .filter(|plot| plot.binding.contains_dataset(dataset)) + .filter(|plot| plot.binding.contains_dataset(dataset_id)) .map(|_| object.id) }) .collect(); diff --git a/crates/core/src/state/app_impl_analysis.rs b/crates/core/src/state/app_impl_analysis.rs index 2b75d19..9183fc7 100644 --- a/crates/core/src/state/app_impl_analysis.rs +++ b/crates/core/src/state/app_impl_analysis.rs @@ -174,7 +174,7 @@ impl PlotxApp { /// per region (x = the raw indirect ruler, y = the region reduced by its /// metric). `None` when the dataset is not a series or has no regions. fn build_region_table(&self, dataset: usize) -> Option { - let source_resource = self.doc.datasets.get(dataset)?.resource_id().to_owned(); + let source_resource = self.doc.datasets.get(dataset)?.resource_id().to_string(); let d2 = self.doc.datasets.get(dataset).and_then(Dataset::as_nmr2d)?; let (Processed2D::Stack(stack), Some(axis)) = (&d2.processed, &d2.data.pseudo_axis) else { return None; @@ -234,7 +234,7 @@ impl PlotxApp { /// The `Dataset::Table` linked to `source` (its provenance points back), if any. pub fn region_table_index(&self, source: usize) -> Option { - let source_resource = self.doc.datasets.get(source)?.resource_id(); + let source_resource = self.doc.datasets.get(source)?.resource_id().to_string(); self.doc.datasets.iter().position(|d| { d.as_table() .and_then(|t| t.provenance.as_ref()) @@ -312,7 +312,7 @@ impl PlotxApp { let mut tds = table; tds.lineage = Some(DatasetLineage::new( DerivationKind::LiveRegionTable, - [dataset], + [self.doc.datasets[dataset].resource_id()], )); tds.name = Some(format!( "{} — regions", @@ -341,7 +341,7 @@ impl PlotxApp { tds.provenance = None; tds.lineage = Some(DatasetLineage::new( DerivationKind::FrozenRegionTable, - [dataset], + [self.doc.datasets[dataset].resource_id()], )); tds.name = Some(format!( "{} — regions (frozen)", @@ -558,6 +558,7 @@ impl PlotxApp { } pub fn analysis_range_for(&self, dataset: usize) -> Option { + let dataset = self.doc.datasets.get(dataset)?.resource_id(); self.session .ui .analysis_selection @@ -567,12 +568,12 @@ impl PlotxApp { .or_else(|| self.visible_range_for_dataset(dataset)) } - fn visible_range_for_dataset(&self, dataset: usize) -> Option { + fn visible_range_for_dataset(&self, dataset: DatasetId) -> Option { let ci = self.session.active_canvas?; let canvas = self.doc.canvases.get(ci)?; let object_id = canvas.active_plot_object_id()?; let plot = canvas.object(object_id)?.plot()?; - (plot.primary_dataset() == dataset).then_some(plot.viewport.view_x) + (plot.primary_dataset() == Some(dataset)).then_some(plot.viewport.view_x) } } diff --git a/crates/core/src/state/app_impl_analysis_tests.rs b/crates/core/src/state/app_impl_analysis_tests.rs index 10bb308..af110d6 100644 --- a/crates/core/src/state/app_impl_analysis_tests.rs +++ b/crates/core/src/state/app_impl_analysis_tests.rs @@ -49,11 +49,17 @@ fn live_and_frozen_region_tables_record_lineage() { assert_eq!( app.doc.datasets[1].lineage(), - Some(&DatasetLineage::new(DerivationKind::LiveRegionTable, [0])) + Some(&DatasetLineage::new( + DerivationKind::LiveRegionTable, + [app.doc.datasets[0].resource_id()] + )) ); assert_eq!( app.doc.datasets[2].lineage(), - Some(&DatasetLineage::new(DerivationKind::FrozenRegionTable, [0])) + Some(&DatasetLineage::new( + DerivationKind::FrozenRegionTable, + [app.doc.datasets[0].resource_id()] + )) ); assert!(app.doc.datasets[1].as_table().unwrap().provenance.is_some()); assert!(app.doc.datasets[2].as_table().unwrap().provenance.is_none()); diff --git a/crates/core/src/state/app_impl_arithmetic.rs b/crates/core/src/state/app_impl_arithmetic.rs index 9c577b6..126a6d4 100644 --- a/crates/core/src/state/app_impl_arithmetic.rs +++ b/crates/core/src/state/app_impl_arithmetic.rs @@ -107,6 +107,10 @@ impl PlotxApp { name: String, sources: impl IntoIterator, ) { + let sources = sources + .into_iter() + .filter_map(|index| self.doc.datasets.get(index).map(Dataset::resource_id)) + .collect::>(); let slice = Slice1D { ppm: result.ppm, values: result.values, diff --git a/crates/core/src/state/app_impl_linefit.rs b/crates/core/src/state/app_impl_linefit.rs index 5882e11..a788690 100644 --- a/crates/core/src/state/app_impl_linefit.rs +++ b/crates/core/src/state/app_impl_linefit.rs @@ -257,7 +257,10 @@ impl PlotxApp { let unit = source.trace_x_unit(); let source_name = source.display_name(); let mut table = line_fit_parameter_table(&fit, &unit); - table.lineage = Some(DatasetLineage::new(DerivationKind::LineFitTable, [dataset])); + table.lineage = Some(DatasetLineage::new( + DerivationKind::LineFitTable, + [self.doc.datasets[dataset].resource_id()], + )); table.name = Some(format!("{source_name} — peak fit")); table.board_pos = super::app_impl_analysis::next_sheet_pos_after_new_canvas(self); let action = Action::insert_dataset_with_default_canvas( diff --git a/crates/core/src/state/app_impl_multiplet.rs b/crates/core/src/state/app_impl_multiplet.rs index dbcf4a2..d8d4416 100644 --- a/crates/core/src/state/app_impl_multiplet.rs +++ b/crates/core/src/state/app_impl_multiplet.rs @@ -126,7 +126,7 @@ impl PlotxApp { let mut tds = multiplet_summary_table(&multiplets); tds.lineage = Some(DatasetLineage::new( DerivationKind::MultipletTable, - [dataset], + [self.doc.datasets[dataset].resource_id()], )); tds.name = Some(format!( "{} — multiplets", diff --git a/crates/core/src/state/app_impl_peaks.rs b/crates/core/src/state/app_impl_peaks.rs index 2c7ea9d..78b2508 100644 --- a/crates/core/src/state/app_impl_peaks.rs +++ b/crates/core/src/state/app_impl_peaks.rs @@ -101,11 +101,15 @@ impl PlotxApp { /// primary dataset is `dataset`. Overlay-only datasets never contribute /// integral curves. pub fn sync_integral_curves_for(&mut self, dataset: usize) { - let curves = self - .doc - .datasets - .get(dataset) - .and_then(Dataset::as_nmr) + // Resolve the dataset once: the index can be stale (e.g. a cancelled + // integral drag after an import was undone), so a miss must return, not + // index-panic on the next line. + let Some(ds) = self.doc.datasets.get(dataset) else { + return; + }; + let dataset_id = ds.resource_id(); + let curves = ds + .as_nmr() .map(NmrDataset::integral_curves) .unwrap_or_default(); for canvas in &mut self.doc.canvases { @@ -113,9 +117,11 @@ impl PlotxApp { let Some(plot) = object.plot_mut() else { continue; }; - if plot.binding.primary_dataset() == dataset && plot.binding.primary_visible() { + if plot.binding.primary_dataset() == Some(dataset_id) + && plot.binding.primary_visible() + { plot.figure.integral_curves.clone_from(&curves); - } else if plot.binding.primary_dataset() == dataset { + } else if plot.binding.primary_dataset() == Some(dataset_id) { plot.figure.integral_curves.clear(); } } diff --git a/crates/core/src/state/app_impl_slice.rs b/crates/core/src/state/app_impl_slice.rs index 8b9a6f0..06efa0b 100644 --- a/crates/core/src/state/app_impl_slice.rs +++ b/crates/core/src/state/app_impl_slice.rs @@ -40,7 +40,7 @@ impl NmrDataset { steps: vec![ProcessingStep::new(StepKind::Fft, StepSource::Default)], }; Self { - resource_id: uuid::Uuid::new_v4().to_string(), + resource_id: DatasetId::new(), data, base: spectrum.clone(), pipeline, @@ -130,7 +130,10 @@ impl PlotxApp { kind: DerivationKind, ) { let mut ds = Dataset::Nmr(Box::new(NmrDataset::from_slice(slice, name.clone()))); - ds.set_lineage(Some(DatasetLineage::new(kind, [source]))); + ds.set_lineage(Some(DatasetLineage::new( + kind, + [self.doc.datasets[source].resource_id()], + ))); let action = Action::insert_dataset_with_default_canvas( self, ds, @@ -201,11 +204,17 @@ mod tests { assert_eq!( app.doc.datasets[1].lineage(), - Some(&DatasetLineage::new(DerivationKind::Slice, [0])) + Some(&DatasetLineage::new( + DerivationKind::Slice, + [app.doc.datasets[0].resource_id()] + )) ); assert_eq!( app.doc.datasets[2].lineage(), - Some(&DatasetLineage::new(DerivationKind::Projection, [0])) + Some(&DatasetLineage::new( + DerivationKind::Projection, + [app.doc.datasets[0].resource_id()] + )) ); } } diff --git a/crates/core/src/state/app_impl_statistics.rs b/crates/core/src/state/app_impl_statistics.rs index e201f8f..26168da 100644 --- a/crates/core/src/state/app_impl_statistics.rs +++ b/crates/core/src/state/app_impl_statistics.rs @@ -186,7 +186,7 @@ impl PlotxApp { let mut table = data; table.lineage = Some(DatasetLineage::new( DerivationKind::StatisticsTable, - [dataset], + [self.doc.datasets[dataset].resource_id()], )); table.name = Some(format!("{source_name} — {}", analysis.title)); table.board_pos = super::app_impl_analysis::next_sheet_pos_after_new_canvas(self); diff --git a/crates/core/src/state/app_impl_statistics_tests.rs b/crates/core/src/state/app_impl_statistics_tests.rs index 670de80..db4eebc 100644 --- a/crates/core/src/state/app_impl_statistics_tests.rs +++ b/crates/core/src/state/app_impl_statistics_tests.rs @@ -256,7 +256,10 @@ fn add_to_board_materializes_a_derived_table_with_lineage() { let derived = app.doc.datasets.last().unwrap(); assert_eq!( derived.lineage(), - Some(&DatasetLineage::new(DerivationKind::StatisticsTable, [0])) + Some(&DatasetLineage::new( + DerivationKind::StatisticsTable, + [app.doc.datasets[0].resource_id()] + )) ); // One column per named group keeps the source column identity. let table = derived.as_table().unwrap(); diff --git a/crates/core/src/state/board.rs b/crates/core/src/state/board.rs index 077715e..0d3d273 100644 --- a/crates/core/src/state/board.rs +++ b/crates/core/src/state/board.rs @@ -155,10 +155,11 @@ pub fn tidy_board_layout(app: &PlotxApp) -> Vec<(FrameRef, [f32; 2])> { /// for semantic jumps between an extracted table, its source spectrum, and its /// fit chart. pub fn page_frame_showing_dataset(app: &PlotxApp, di: usize) -> Option { + let dataset_id = app.doc.datasets.get(di)?.resource_id(); app.doc .canvases .iter() - .position(|c| c.objects.iter().any(|o| o.dataset() == Some(di))) + .position(|c| c.objects.iter().any(|o| o.dataset() == Some(dataset_id))) .map(FrameRef::Page) } @@ -223,12 +224,7 @@ fn sync_data_selection_from_frames(app: &mut PlotxApp) { let mut datasets: Vec = Vec::new(); for frame in frames { let indices = match frame { - FrameRef::Page(ci) => app - .doc - .canvases - .get(ci) - .map(|c| c.dataset_indices()) - .unwrap_or_default(), + FrameRef::Page(ci) => app.doc.page_dataset_indices(ci), FrameRef::Sheet(di) => vec![di], }; for di in indices { diff --git a/crates/core/src/state/dataset_identity.rs b/crates/core/src/state/dataset_identity.rs index c01deff..cf2e61a 100644 --- a/crates/core/src/state/dataset_identity.rs +++ b/crates/core/src/state/dataset_identity.rs @@ -1,17 +1,17 @@ -use super::Dataset; +use super::{Dataset, DatasetId}; impl Dataset { - pub fn resource_id(&self) -> &str { + pub fn resource_id(&self) -> DatasetId { match self { - Dataset::Nmr(dataset) => &dataset.resource_id, - Dataset::Nmr2D(dataset) => &dataset.resource_id, - Dataset::Table(dataset) => &dataset.resource_id, - Dataset::Electrophysiology(dataset) => &dataset.resource_id, - Dataset::Afm(dataset) => &dataset.resource_id, + Dataset::Nmr(dataset) => dataset.resource_id, + Dataset::Nmr2D(dataset) => dataset.resource_id, + Dataset::Table(dataset) => dataset.resource_id, + Dataset::Electrophysiology(dataset) => dataset.resource_id, + Dataset::Afm(dataset) => dataset.resource_id, } } - pub(crate) fn set_resource_id(&mut self, id: String) { + pub(crate) fn set_resource_id(&mut self, id: DatasetId) { match self { Dataset::Nmr(dataset) => dataset.resource_id = id, Dataset::Nmr2D(dataset) => dataset.resource_id = id, diff --git a/crates/core/src/state/datasets.rs b/crates/core/src/state/datasets.rs index 0ca10e6..77d6bf2 100644 --- a/crates/core/src/state/datasets.rs +++ b/crates/core/src/state/datasets.rs @@ -26,7 +26,7 @@ pub struct PhaseDrag { pub struct NmrDataset { /// Stable automation and persistence identity. Array positions remain a UI /// implementation detail and must never escape into saved references. - pub resource_id: String, + pub resource_id: DatasetId, pub data: NmrData, pub base: Spectrum, pub pipeline: AxisPipeline, @@ -62,7 +62,7 @@ impl NmrDataset { let base = fft::transform_base(&data, &pipeline, group_delay_correct); let spectrum = reapply(&base, &pipeline); Self { - resource_id: uuid::Uuid::new_v4().to_string(), + resource_id: DatasetId::new(), data, base, pipeline, @@ -106,7 +106,7 @@ impl NmrDataset { #[derive(Clone)] pub struct Nmr2DDataset { /// Stable automation and persistence identity. - pub resource_id: String, + pub resource_id: DatasetId, pub data: Arc, pub params: Params2D, /// Recipe used to produce `base`. While an async retransform is pending, @@ -169,7 +169,7 @@ impl Nmr2DDataset { let processed = reapply_2d(&base, ¶ms); let processed_figure = Arc::new(build_processed_figure(&processed, preset)); Self { - resource_id: uuid::Uuid::new_v4().to_string(), + resource_id: DatasetId::new(), data: Arc::new(data), base_params: params.clone(), base_stale: false, diff --git a/crates/core/src/state/document.rs b/crates/core/src/state/document.rs index 8550e00..faab840 100644 --- a/crates/core/src/state/document.rs +++ b/crates/core/src/state/document.rs @@ -21,8 +21,8 @@ pub struct ZoomDrag { #[derive(Clone, Debug, PartialEq)] pub struct AnalysisSelection { - pub dataset: usize, - pub canvas: usize, + pub dataset: DatasetId, + pub canvas: CanvasId, pub object: ObjectId, pub x_range: AxisRange, pub y_range: Option, @@ -166,7 +166,7 @@ pub struct NamedView { /// Only `dataset` is required. #[derive(Clone, Debug, PartialEq)] pub struct SeriesBinding { - pub dataset: usize, + pub dataset: DatasetId, pub color: Option, pub label: Option, pub scale: f64, @@ -174,9 +174,9 @@ pub struct SeriesBinding { } impl SeriesBinding { - pub fn new(dataset: usize) -> Self { + pub fn new(dataset: impl Into) -> Self { Self { - dataset, + dataset: dataset.into(), color: None, label: None, scale: 1.0, @@ -232,14 +232,14 @@ pub struct DataBinding { } impl DataBinding { - pub fn single(dataset: usize) -> Self { + pub fn single(dataset: impl Into) -> Self { Self { series: vec![SeriesBinding::new(dataset)], } } - pub fn primary_dataset(&self) -> usize { - self.series.first().map(|s| s.dataset).unwrap_or(0) + pub fn primary_dataset(&self) -> Option { + self.series.first().map(|s| s.dataset) } /// Result overlays belonging to the primary dataset follow the visibility @@ -248,11 +248,11 @@ impl DataBinding { self.series.first().is_some_and(|series| series.visible) } - pub fn dataset_indices(&self) -> Vec { + pub fn dataset_ids(&self) -> Vec { self.series.iter().map(|s| s.dataset).collect() } - pub fn contains_dataset(&self, dataset: usize) -> bool { + pub fn contains_dataset(&self, dataset: DatasetId) -> bool { self.series.iter().any(|s| s.dataset == dataset) } } @@ -264,7 +264,7 @@ impl DataBinding { pub enum ProjectionSource { #[default] None, - Attached(usize), + Attached(DatasetId), Sum, Skyline, Slice(usize), @@ -307,7 +307,7 @@ impl AxisProjections { } /// Every dataset index an `Attached` source references, for save-time id mapping. - pub fn attached_datasets(&self) -> Vec { + pub fn attached_datasets(&self) -> Vec { [&self.top, &self.left] .iter() .filter_map(|a| match a.source { @@ -566,15 +566,15 @@ impl CanvasObject { } } - pub fn dataset(&self) -> Option { - self.plot().map(|plot| plot.primary_dataset()) + pub fn dataset(&self) -> Option { + self.plot().and_then(|plot| plot.primary_dataset()) } /// Every dataset this object binds (all series of a plot; empty for non-plots). /// Drives mirroring a board selection into the Data list. - pub fn dataset_indices(&self) -> Vec { + pub fn dataset_ids(&self) -> Vec { self.plot() - .map(|plot| plot.binding.dataset_indices()) + .map(|plot| plot.binding.dataset_ids()) .unwrap_or_default() } } @@ -648,7 +648,7 @@ pub fn document_items(canvas: &CanvasDocument) -> Vec Self { Self { - resource_id: uuid::Uuid::new_v4().to_string(), + resource_id: CanvasId::new(), name, size_mm, size_preset_id: None, @@ -692,23 +692,23 @@ impl CanvasDocument { caption_visible: true, panel_label_style: PanelLabelStyle::default(), layout: crate::layout::PageLayout::default(), - next_object_id: 1, + next_object_id: ObjectId::new(1), next_group_id: 1, } } - /// The sorted, de-duplicated dataset indices every plot on this page binds, - /// used to mirror a page selection into the Data list's multi-select. - pub fn dataset_indices(&self) -> Vec { - let mut ids: Vec = self - .objects + /// The de-duplicated dataset ids every plot on this page binds, in first- + /// encounter (page z-fill) order. Deterministic: never depends on DatasetId + /// ordering. Callers that need document order should resolve indices through + /// [`PlotxApp::page_dataset_indices`], which sorts by document position. + pub fn dataset_ids(&self) -> Vec { + let mut seen = std::collections::HashSet::new(); + self.objects .iter() .filter_map(CanvasObject::plot) - .flat_map(|plot| plot.binding.dataset_indices()) - .collect(); - ids.sort_unstable(); - ids.dedup(); - ids + .flat_map(|plot| plot.binding.dataset_ids()) + .filter(|id| seen.insert(*id)) + .collect() } /// The plot objects' ids in list (z / fill) order — the order the grid @@ -732,7 +732,7 @@ impl CanvasDocument { pub fn allocate_object_id(&mut self) -> ObjectId { let id = self.next_object_id; - self.next_object_id += 1; + self.next_object_id = self.next_object_id.checked_advance(1); id } @@ -786,7 +786,7 @@ impl CanvasDocument { .or_else(|| self.first_plot_object_id()) } - pub fn active_dataset(&self) -> Option { + pub fn active_dataset(&self) -> Option { self.active_plot_object_id() .and_then(|id| self.object(id)) .and_then(CanvasObject::dataset) diff --git a/crates/core/src/state/document_identity.rs b/crates/core/src/state/document_identity.rs new file mode 100644 index 0000000..135f641 --- /dev/null +++ b/crates/core/src/state/document_identity.rs @@ -0,0 +1,41 @@ +use super::{CanvasId, DatasetId, Document}; + +impl Document { + pub fn dataset_index(&self, id: DatasetId) -> Option { + self.datasets + .iter() + .position(|dataset| dataset.resource_id() == id) + } + + /// The dataset owning `id`, or `None` if it no longer exists (e.g. a binding + /// left dangling by an undo). The one accessor every lookup should route + /// through instead of `expect`/`usize::MAX` improvisation. + pub fn dataset_by_id(&self, id: DatasetId) -> Option<&crate::state::Dataset> { + self.datasets + .iter() + .find(|dataset| dataset.resource_id() == id) + } + + pub fn canvas_index(&self, id: CanvasId) -> Option { + self.canvases + .iter() + .position(|canvas| canvas.resource_id == id) + } + + /// The datasets plotted on canvas `ci`, as document-ordered indices — the + /// order the Data list mirror and stack-primary selection depend on. + /// Deterministic regardless of DatasetId ordering; empty if `ci` is stale. + pub fn page_dataset_indices(&self, ci: usize) -> Vec { + let Some(canvas) = self.canvases.get(ci) else { + return Vec::new(); + }; + let mut indices: Vec = canvas + .dataset_ids() + .into_iter() + .filter_map(|id| self.dataset_index(id)) + .collect(); + indices.sort_unstable(); + indices.dedup(); + indices + } +} diff --git a/crates/core/src/state/electrophysiology.rs b/crates/core/src/state/electrophysiology.rs index 939b25b..7623d55 100644 --- a/crates/core/src/state/electrophysiology.rs +++ b/crates/core/src/state/electrophysiology.rs @@ -1,8 +1,8 @@ use super::*; use plotx_analysis::electrophysiology::{self, PeakMode, TimeWindow}; -fn new_resource_id() -> String { - uuid::Uuid::new_v4().to_string() +fn new_resource_id() -> DatasetId { + DatasetId::new() } #[derive(Clone, Debug, Default, serde::Serialize, serde::Deserialize)] @@ -74,7 +74,7 @@ impl Default for ElectrophysiologyProcessing { #[derive(Clone, Debug, serde::Serialize, serde::Deserialize)] pub struct ElectrophysiologyDataset { #[serde(default = "new_resource_id")] - pub resource_id: String, + pub resource_id: DatasetId, pub data: ElectrophysiologyData, pub name: Option, pub metadata: RecordingMetadata, diff --git a/crates/core/src/state/identity.rs b/crates/core/src/state/identity.rs new file mode 100644 index 0000000..f07030c --- /dev/null +++ b/crates/core/src/state/identity.rs @@ -0,0 +1,125 @@ +use serde::{Deserialize, Serialize}; +use std::fmt; +use std::str::FromStr; +use uuid::Uuid; + +macro_rules! uuid_id { + ($name:ident) => { + #[derive( + Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize, + )] + #[serde(transparent)] + pub struct $name(Uuid); + + impl $name { + pub fn new() -> Self { + Self(Uuid::new_v4()) + } + + pub const fn from_uuid(value: Uuid) -> Self { + Self(value) + } + + pub const fn as_uuid(self) -> Uuid { + self.0 + } + } + + impl Default for $name { + fn default() -> Self { + Self::new() + } + } + + impl fmt::Display for $name { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.0.fmt(f) + } + } + + impl FromStr for $name { + type Err = uuid::Error; + + fn from_str(value: &str) -> Result { + Uuid::parse_str(value).map(Self) + } + } + }; +} + +uuid_id!(DatasetId); +uuid_id!(CanvasId); + +macro_rules! local_id { + ($name:ident) => { + #[derive( + Clone, + Copy, + Debug, + Default, + PartialEq, + Eq, + PartialOrd, + Ord, + Hash, + Serialize, + Deserialize, + )] + #[serde(transparent)] + pub struct $name(u64); + + impl $name { + pub const fn new(value: u64) -> Self { + Self(value) + } + + pub const fn get(self) -> u64 { + self.0 + } + + /// Advance a runtime allocator. Overflow means 2^64 ids were minted + /// in one session — a true local invariant violation, so it panics. + /// Never call this on data parsed from a file; use [`try_advance`]. + pub fn checked_advance(self, amount: u64) -> Self { + Self( + self.0 + .checked_add(amount) + .expect("owner-local identity allocator overflow"), + ) + } + + /// Fallible advance for load paths: returns `None` on overflow so the + /// caller can reject a corrupt file instead of aborting the app. + pub fn try_advance(self, amount: u64) -> Option { + self.0.checked_add(amount).map(Self) + } + } + + impl fmt::Display for $name { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.0.fmt(f) + } + } + + impl From<$name> for u64 { + fn from(value: $name) -> Self { + value.0 + } + } + + impl FromStr for $name { + type Err = std::num::ParseIntError; + + fn from_str(value: &str) -> Result { + value.parse().map(Self) + } + } + }; +} + +local_id!(SeriesId); +local_id!(ObjectId); + +#[cfg(test)] +#[path = "identity_tests.rs"] +mod tests; diff --git a/crates/core/src/state/identity_tests.rs b/crates/core/src/state/identity_tests.rs new file mode 100644 index 0000000..9159fd7 --- /dev/null +++ b/crates/core/src/state/identity_tests.rs @@ -0,0 +1,22 @@ +use super::*; + +#[test] +fn global_ids_round_trip_as_uuid_text() { + let dataset = DatasetId::new(); + let canvas = CanvasId::new(); + + assert_eq!(dataset.to_string().parse(), Ok(dataset)); + assert_eq!(canvas.to_string().parse(), Ok(canvas)); + assert_eq!( + serde_json::to_string(&dataset).unwrap(), + format!("\"{dataset}\"") + ); +} + +#[test] +fn owner_local_ids_are_distinct_types() { + let series = SeriesId::new(7); + + assert_eq!(series.get(), 7); + assert_eq!(serde_json::to_string(&series).unwrap(), "7"); +} diff --git a/crates/core/src/state/lineage.rs b/crates/core/src/state/lineage.rs index ecbc286..9f5f463 100644 --- a/crates/core/src/state/lineage.rs +++ b/crates/core/src/state/lineage.rs @@ -1,4 +1,4 @@ -use super::Dataset; +use super::{Dataset, DatasetId}; use serde::{Deserialize, Serialize}; /// Why a dataset was materialized from one or more earlier datasets. @@ -42,13 +42,17 @@ impl DerivationKind { #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] pub struct DatasetLineage { pub kind: DerivationKind, - pub sources: Vec, + pub sources: Vec, } impl DatasetLineage { - pub fn new(kind: DerivationKind, sources: impl IntoIterator) -> Self { + pub fn new>( + kind: DerivationKind, + sources: impl IntoIterator, + ) -> Self { let mut unique = Vec::new(); for source in sources { + let source = source.into(); if !unique.contains(&source) { unique.push(source); } diff --git a/crates/core/src/state/mod.rs b/crates/core/src/state/mod.rs index e22360a..3c48102 100644 --- a/crates/core/src/state/mod.rs +++ b/crates/core/src/state/mod.rs @@ -46,8 +46,10 @@ mod datasets; mod datasets_2d_figure; mod datasets_2d_maps; mod document; +mod document_identity; mod electrophysiology; mod fit_selection; +mod identity; mod interaction; mod lineage; mod linefit; @@ -91,6 +93,7 @@ pub use datasets::*; pub(crate) use datasets_2d_figure::{build_processed_figure, build_processed_figure_cancellable}; pub use document::*; pub use electrophysiology::*; +pub use identity::*; pub use interaction::*; pub use lineage::*; pub use linefit::*; @@ -123,7 +126,6 @@ pub const DEFAULT_CANVAS_SIZE_MM: [f32; 2] = NATURE_SINGLE_COLUMN.size_mm(); const PX_PER_IN: f32 = 96.0; const MM_PER_IN: f32 = 25.4; -pub type ObjectId = u64; pub type GroupId = u64; #[cfg(test)] @@ -133,12 +135,16 @@ mod tests { #[test] fn selection_multi_reports_primary_and_membership() { - let sel = Selection::Objects(vec![3, 7, 9]); - assert_eq!(sel.object(), Some(3)); - assert_eq!(sel.objects(), &[3, 7, 9]); - assert!(sel.contains(7)); - assert!(!sel.contains(4)); - assert_eq!(Selection::single(5).objects(), &[5]); + let ids = [3, 7, 9].map(ObjectId::new); + let sel = Selection::Objects(ids.to_vec()); + assert_eq!(sel.object(), Some(ObjectId::new(3))); + assert_eq!(sel.objects(), &ids); + assert!(sel.contains(ObjectId::new(7))); + assert!(!sel.contains(ObjectId::new(4))); + assert_eq!( + Selection::single(ObjectId::new(5)).objects(), + &[ObjectId::new(5)] + ); } #[test] diff --git a/crates/core/src/state/page_fit.rs b/crates/core/src/state/page_fit.rs index f84ca2c..eca0492 100644 --- a/crates/core/src/state/page_fit.rs +++ b/crates/core/src/state/page_fit.rs @@ -153,7 +153,7 @@ mod tests { fn page_with_text(size_mm: [f32; 2], frame: ObjectFrame) -> CanvasDocument { let mut canvas = CanvasDocument::new("page".into(), size_mm); canvas.objects.push(CanvasObject { - id: 1, + id: ObjectId::new(1), name: "t".to_owned(), frame, locked: false, @@ -161,7 +161,7 @@ mod tests { group: None, kind: CanvasObjectKind::Text(TextBox::label("x".to_owned())), }); - canvas.next_object_id = 2; + canvas.next_object_id = ObjectId::new(2); canvas } diff --git a/crates/core/src/state/plot_object.rs b/crates/core/src/state/plot_object.rs index ba7e1d1..4771854 100644 --- a/crates/core/src/state/plot_object.rs +++ b/crates/core/src/state/plot_object.rs @@ -1,8 +1,8 @@ -use super::{CanvasViewport, PlotObject}; +use super::{CanvasViewport, DatasetId, PlotObject}; use plotx_figure::Figure; impl PlotObject { - pub fn primary_dataset(&self) -> usize { + pub fn primary_dataset(&self) -> Option { self.binding.primary_dataset() } diff --git a/crates/core/src/state/stack.rs b/crates/core/src/state/stack.rs index 1b46962..9c67902 100644 --- a/crates/core/src/state/stack.rs +++ b/crates/core/src/state/stack.rs @@ -8,16 +8,20 @@ impl PlotxApp { let Some(domain) = binding .series .first() - .and_then(|s| self.doc.datasets.get(s.dataset)) + .and_then(|s| self.doc.dataset_index(s.dataset)) + .and_then(|index| self.doc.datasets.get(index)) .map(Dataset::domain) else { return false; }; domain.stack_kind().is_some() - && binding - .series - .iter() - .all(|s| self.doc.datasets.get(s.dataset).map(Dataset::domain) == Some(domain)) + && binding.series.iter().all(|s| { + self.doc + .dataset_index(s.dataset) + .and_then(|index| self.doc.datasets.get(index)) + .map(Dataset::domain) + == Some(domain) + }) } /// Combine a stackable binding into one figure. Dispatches on the primary's @@ -29,7 +33,11 @@ impl PlotxApp { stack: &StackSpec, size_mm: [f32; 2], ) -> Figure { - let domain = self.doc.datasets[binding.primary_dataset()].domain(); + let primary = binding + .primary_dataset() + .and_then(|id| self.doc.dataset_index(id)) + .expect("validated data binding has a primary dataset"); + let domain = self.doc.datasets[primary].domain(); match (domain.stack_kind(), stack.mode) { (Some(StackKind::Field), _) => self.build_contour_overlay(binding, size_mm), _ => self.build_line_stack(binding, stack, size_mm), @@ -45,11 +53,14 @@ impl PlotxApp { stack: &StackSpec, size_mm: [f32; 2], ) -> Figure { - let domain = self.doc.datasets[binding.primary_dataset()].domain(); + let primary = binding + .primary_dataset() + .and_then(|id| self.doc.dataset_index(id)) + .expect("validated data binding has a primary dataset"); + let domain = self.doc.datasets[primary].domain(); let line_chart = ChartSpec::default_for(domain); // The primary's line figure supplies the axis labels and orientation. - let mut fig = - self.build_full_canvas_figure(binding.primary_dataset(), &line_chart, size_mm); + let mut fig = self.build_full_canvas_figure(primary, &line_chart, size_mm); let x_span = (fig.x.max - fig.x.min).abs().max(f64::MIN_POSITIVE); fig.series.clear(); fig.error_bars.clear(); @@ -59,10 +70,13 @@ impl PlotxApp { let mut prepared: Vec<(usize, Vec, Vec)> = Vec::new(); let mut global_peak = 0.0f64; for (i, sb) in binding.series.iter().enumerate() { - if !sb.visible || self.doc.datasets.get(sb.dataset).is_none() { + let Some(dataset) = self.doc.dataset_index(sb.dataset) else { + continue; + }; + if !sb.visible { continue; } - let part = self.build_full_canvas_figure(sb.dataset, &line_chart, size_mm); + let part = self.build_full_canvas_figure(dataset, &line_chart, size_mm); let mut series = part.series; let mut error_bars = part.error_bars; let peak = series @@ -158,16 +172,23 @@ impl PlotxApp { /// labels and orientation; hidden series are skipped. fn build_contour_overlay(&self, binding: &DataBinding, size_mm: [f32; 2]) -> Figure { let chart = ChartSpec::default_for(DataDomain::Nmr2d); - let mut fig = self.build_full_canvas_figure(binding.primary_dataset(), &chart, size_mm); + let primary = binding + .primary_dataset() + .and_then(|id| self.doc.dataset_index(id)) + .expect("validated data binding has a primary dataset"); + let mut fig = self.build_full_canvas_figure(primary, &chart, size_mm); fig.contours.clear(); let (mut x_min, mut x_max) = (fig.x.min, fig.x.max); let (mut y_min, mut y_max) = (fig.y.min, fig.y.max); let mut merged = false; for (i, sb) in binding.series.iter().enumerate() { - if !sb.visible || self.doc.datasets.get(sb.dataset).is_none() { + let Some(dataset) = self.doc.dataset_index(sb.dataset) else { + continue; + }; + if !sb.visible { continue; } - let part = self.build_full_canvas_figure(sb.dataset, &chart, size_mm); + let part = self.build_full_canvas_figure(dataset, &chart, size_mm); let color = sb .color .unwrap_or(OVERLAY_PALETTE[i % OVERLAY_PALETTE.len()]); @@ -196,8 +217,7 @@ impl PlotxApp { pub fn series_label(&self, sb: &SeriesBinding) -> String { sb.label.clone().unwrap_or_else(|| { self.doc - .datasets - .get(sb.dataset) + .dataset_by_id(sb.dataset) .map(Dataset::display_name) .unwrap_or_default() }) @@ -207,20 +227,19 @@ impl PlotxApp { /// same stackable domain not already bound. Empty when the plot's primary is /// not a stackable domain. pub fn stack_candidates(&self, binding: &DataBinding) -> Vec { - let Some(domain) = self - .doc - .datasets - .get(binding.primary_dataset()) + let Some(domain) = binding + .primary_dataset() + .and_then(|id| self.doc.dataset_by_id(id)) .map(Dataset::domain) .filter(|d| d.stack_kind().is_some()) else { return Vec::new(); }; - let bound = binding.dataset_indices(); + let bound = binding.dataset_ids(); (0..self.doc.datasets.len()) .filter(|di| { self.doc.datasets.get(*di).map(Dataset::domain) == Some(domain) - && !bound.contains(di) + && !bound.contains(&self.doc.datasets[*di].resource_id()) }) .collect() } @@ -314,7 +333,11 @@ impl PlotxApp { }; let domain = self.doc.datasets[sel[0]].domain(); let binding = DataBinding { - series: sel.iter().map(|&d| SeriesBinding::new(d)).collect(), + series: sel + .iter() + .filter_map(|&d| self.doc.datasets.get(d)) + .map(|dataset| SeriesBinding::new(dataset.resource_id())) + .collect(), }; let mode = match domain.stack_kind() { Some(StackKind::Field) => StackMode::ColorOverlay, diff --git a/crates/core/src/state/table.rs b/crates/core/src/state/table.rs index 5beb9d4..cf5a4e7 100644 --- a/crates/core/src/state/table.rs +++ b/crates/core/src/state/table.rs @@ -6,8 +6,8 @@ use plotx_io::DiffusionMeta; use serde::{Deserialize, Serialize}; use std::collections::BTreeMap; -fn new_resource_id() -> String { - uuid::Uuid::new_v4().to_string() +fn new_resource_id() -> crate::state::DatasetId { + crate::state::DatasetId::new() } /// A column points into a table-level analysis so multiple responses and @@ -181,7 +181,7 @@ pub const SHEET_MAX_ROWS: usize = 24; /// App-layer wrapper around an immutable typed table revision. #[derive(Clone)] pub struct TableDataset { - pub resource_id: String, + pub resource_id: crate::state::DatasetId, /// Executable extraction recipe used to refresh this immutable table. pub provenance: Option, /// Domain constants consumed by analysis bindings. diff --git a/crates/core/src/state/table_execution_job.rs b/crates/core/src/state/table_execution_job.rs index f9c8b7e..16c9d58 100644 --- a/crates/core/src/state/table_execution_job.rs +++ b/crates/core/src/state/table_execution_job.rs @@ -1,6 +1,6 @@ use super::{ - TypedTableState, execute_typed_plan, execute_typed_plan_cancellable, refresh_typed_plan, - refresh_typed_plan_cancellable, + DatasetId, TypedTableState, execute_typed_plan, execute_typed_plan_cancellable, + refresh_typed_plan, refresh_typed_plan_cancellable, }; use plotx_data::TableId; use std::collections::BTreeSet; @@ -9,7 +9,7 @@ use std::sync::{Arc, mpsc}; use std::time::{Duration, Instant}; pub struct TableTransformJob { - input_datasets: Vec, + input_datasets: Vec, epoch: u64, name: String, started_at: Instant, @@ -37,6 +37,10 @@ impl crate::state::PlotxApp { if self.session.table_transform_job.is_some() || self.session.table_refresh_job.is_some() { return Err("A table transform is already running.".into()); } + let input_datasets: Vec = input_datasets + .into_iter() + .map(|index| self.doc.datasets[index].resource_id()) + .collect(); let inputs = self.typed_inputs(&input_datasets)?; let cancel = Arc::new(AtomicBool::new(false)); let worker_cancel = Arc::clone(&cancel); @@ -66,16 +70,18 @@ impl crate::state::PlotxApp { Ok(()) } - fn typed_inputs(&self, datasets: &[usize]) -> Result, String> { + fn typed_inputs(&self, datasets: &[DatasetId]) -> Result, String> { datasets .iter() - .map(|index| { + .map(|id| { self.doc - .datasets - .get(*index) + .dataset_by_id(*id) .and_then(crate::state::Dataset::as_table) .map(|table| table.typed_state.clone()) - .ok_or_else(|| format!("Dataset {} is not a data table.", index + 1)) + .ok_or_else(|| { + "A source data table for this refresh is missing or is no longer a table." + .to_owned() + }) }) .collect() } @@ -145,7 +151,7 @@ impl crate::state::PlotxApp { pub fn start_table_refresh( &mut self, dataset: usize, - input_datasets: Vec, + input_datasets: Vec, memory_limit_bytes: u64, ) -> Result<(), String> { if self.session.table_transform_job.is_some() || self.session.table_refresh_job.is_some() { @@ -263,7 +269,11 @@ impl crate::state::PlotxApp { name: String, memory_limit_bytes: u64, ) -> Result { - let inputs = self.typed_inputs(input_datasets)?; + let input_ids: Vec = input_datasets + .iter() + .map(|&index| self.doc.datasets[index].resource_id()) + .collect(); + let inputs = self.typed_inputs(&input_ids)?; let refs = inputs.iter().collect::>(); let typed = execute_typed_plan( plan, @@ -275,7 +285,7 @@ impl crate::state::PlotxApp { .map_err(|error| error.to_string())?; let index = self.doc.datasets.len(); let job = TableTransformJob { - input_datasets: input_datasets.to_vec(), + input_datasets: input_ids, epoch: self.session.dataset_epoch, name, started_at: Instant::now(), @@ -299,7 +309,11 @@ impl crate::state::PlotxApp { .and_then(crate::state::Dataset::as_table) .map(|table| table.typed_state.clone()) .ok_or_else(|| "Select a derived data table to refresh.".to_owned())?; - let inputs = self.typed_inputs(input_datasets)?; + let input_ids: Vec = input_datasets + .iter() + .map(|&index| self.doc.datasets[index].resource_id()) + .collect(); + let inputs = self.typed_inputs(&input_ids)?; let refs = inputs.iter().collect::>(); let refreshed = refresh_typed_plan(&derived, &refs, memory_limit_bytes, &BTreeSet::new()) .map_err(|error| error.to_string())?; @@ -320,6 +334,20 @@ mod tests { use crate::state::{Dataset, FloatSeries, materialized_float_series_table}; use plotx_data::{Relation, SnapshotRead}; + #[test] + fn typed_inputs_reports_a_missing_source_dataset() { + let app = crate::state::PlotxApp::new(); + let missing = DatasetId::new(); + + let error = match app.typed_inputs(&[missing]) { + Ok(_) => panic!("a missing source dataset must be rejected"), + Err(error) => error, + }; + + assert!(error.contains("source data table"), "{error}"); + assert!(error.contains("missing"), "{error}"); + } + #[test] fn background_transform_commits_only_after_polling_completion() { let source = materialized_float_series_table( diff --git a/crates/core/src/state/tile_drop.rs b/crates/core/src/state/tile_drop.rs index a0a577e..bac9a2d 100644 --- a/crates/core/src/state/tile_drop.rs +++ b/crates/core/src/state/tile_drop.rs @@ -50,7 +50,7 @@ mod tests { let preview = TileDropPreview { cache_key: TileDropCacheKey { source_canvas: 0, - source_object: 1, + source_object: ObjectId::new(1), target_canvas: 1, target_page_pt: [100.0; 2], target_layout: crate::layout::PageLayout::default(), diff --git a/crates/core/src/workflow.rs b/crates/core/src/workflow.rs index 2a75261..f5c83ec 100644 --- a/crates/core/src/workflow.rs +++ b/crates/core/src/workflow.rs @@ -236,7 +236,7 @@ pub fn build_dataset_figure(dataset: &Dataset, chart: &ChartSpec, size_mm: [f32; pub fn build_plot_object( dataset: &Dataset, - dataset_index: usize, + _dataset_index: usize, frame: ObjectFrame, id: ObjectId, name: String, @@ -258,7 +258,7 @@ pub fn build_plot_object( visible: true, group: None, kind: CanvasObjectKind::Plot(Box::new(PlotObject { - binding: DataBinding::single(dataset_index), + binding: DataBinding::single(dataset.resource_id()), chart, stack: StackSpec::default(), projections: AxisProjections::default(), @@ -498,7 +498,7 @@ mod tests { let (dataset, source) = dataset_from_acquisition(acquisition()); assert_eq!(dataset.kind_label(), "NMR 1D"); let canvas = build_default_canvas(&dataset, &source); - assert_eq!(canvas.dataset_indices(), vec![0]); + assert_eq!(canvas.dataset_ids(), vec![dataset.resource_id()]); assert_eq!(canvas.objects.len(), 1); } diff --git a/crates/processing/Cargo.toml b/crates/processing/Cargo.toml index 36e803a..fb9f0fe 100644 --- a/crates/processing/Cargo.toml +++ b/crates/processing/Cargo.toml @@ -16,3 +16,4 @@ plotx-analysis.workspace = true num-complex.workspace = true rustfft.workspace = true thiserror.workspace = true +serde.workspace = true diff --git a/crates/processing/src/lib.rs b/crates/processing/src/lib.rs index 96c4c55..d524691 100644 --- a/crates/processing/src/lib.rs +++ b/crates/processing/src/lib.rs @@ -290,7 +290,10 @@ impl BinParams { /// A stable identifier for a step, so callers can address it across edits and /// reorders. Mint fresh ids with [`StepId::fresh`]. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +#[derive( + Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, serde::Serialize, serde::Deserialize, +)] +#[serde(transparent)] pub struct StepId(pub u64); impl StepId {