Skip to content

Commit ad4101f

Browse files
committed
Fix: Harden History replay and recomputation consistency
Post-merge auditing exposed cases where plugin provenance could be lost during History operations, partial recomputation failures could leave outputs in an inconsistent state, and multi-action edits could replay dependent work more than once. History execution now preserves feature identity, reports failures explicitly, and applies dependent updates in a deterministic, transactional way. * [FIX] : Preserve plugin origins when replaying actions, remapping metadata, recomputing transformations, and refreshing analyses * [FIX] : Stop dependent cascades after failed actions and keep affected actions stale while allowing independent branches to continue * [FIX] : Validate multi-output recomputation before updating objects and roll back all outputs when an update cannot be completed * [FIX] : Isolate multi-object analysis persistence failures, continue with remaining objects, and distinguish empty results from failed execution * [CHG] : Deduplicate and globally order multi-action edits, restore action state on cancellation, and cascade analyses after upstream changes * [NEW] : Add regression coverage for plugin provenance, transactional recomputation, cascade failures, and multi-action History edits * [CHG] : Synchronize French translations for recomputation failure messages
1 parent c80af93 commit ad4101f

11 files changed

Lines changed: 1231 additions & 213 deletions

File tree

datalab/adapters_metadata/common.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ class ResultData:
5555
results: list[BaseResultAdapter] | None = None
5656
ylabels: list[str] | None = None
5757
short_ids: list[str] | None = None
58+
execution_success: bool = True
5859

5960
def __bool__(self) -> bool:
6061
"""Return True if there are results stored"""

datalab/gui/panel/base.py

Lines changed: 52 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -895,7 +895,7 @@ def apply_analysis_parameters(
895895
obj: SignalObj | ImageObj | None = None,
896896
interactive: bool = True,
897897
param: gds.DataSet | None = None,
898-
) -> None:
898+
) -> bool:
899899
"""Apply analysis parameters: re-run the 1-to-0 analysis in place.
900900
901901
Args:
@@ -911,7 +911,7 @@ def apply_analysis_parameters(
911911
editor = self.analysis_param_editor
912912
obj = obj or self.current_analysis_obj
913913
if obj is None:
914-
return
914+
return False
915915

916916
# Extract analysis parameters
917917
proc_params = extract_analysis_parameters(obj)
@@ -920,7 +920,7 @@ def apply_analysis_parameters(
920920
QW.QMessageBox.warning(
921921
self, _("Error"), _("Analysis metadata is incomplete.")
922922
)
923-
return
923+
return False
924924

925925
func_name = proc_params.func_name
926926

@@ -929,21 +929,22 @@ def apply_analysis_parameters(
929929
# the stored analysis parameters.
930930
if param is None:
931931
param = editor.dataset if editor is not None else proc_params.param
932+
recompute_param = copy.deepcopy(param)
932933

933934
# Disable ROI creation during re-analysis: detection functions store
934935
# create_rois=True in their parameters, but re-running should only
935936
# update analysis results, not recreate ROIs (which would make them
936937
# impossible to delete or modify).
937-
if hasattr(param, "create_rois"):
938-
param.create_rois = False
938+
if hasattr(recompute_param, "create_rois"):
939+
recompute_param.create_rois = False
939940

940941
# Re-run the analysis in place (no history entry: runs under replaying)
941942
processor = self.__get_processor_associated_to(obj)
942943
try:
943-
processor.recompute_1_to_0(
944+
success = processor.recompute_1_to_0(
944945
func_name,
945946
obj,
946-
param,
947+
recompute_param,
947948
plugin_origin=proc_params.plugin_origin,
948949
)
949950
except Exception as exc: # pylint: disable=broad-exception-caught
@@ -954,7 +955,13 @@ def apply_analysis_parameters(
954955
_("Error"),
955956
_("Failed to recompute analysis:\n%s") % str(exc),
956957
)
957-
return
958+
return False
959+
if not success:
960+
if interactive:
961+
QW.QMessageBox.warning(
962+
self, _("Error"), _("Failed to recompute analysis.")
963+
)
964+
return False
958965

959966
# Propagate the edited param to the History panel: mutate the matching
960967
# analysis action (snapshot originals first) and refresh its tree
@@ -965,7 +972,7 @@ def apply_analysis_parameters(
965972
action = hpanel.find_analysis_action(get_uuid(obj), func_name)
966973
if action is not None:
967974
action.snapshot_kwargs()
968-
action.kwargs["param"] = copy.deepcopy(param)
975+
action.kwargs["param"] = copy.deepcopy(recompute_param)
969976
hpanel.refresh_action(action)
970977

971978
# Refresh the object display after re-analysis
@@ -983,6 +990,7 @@ def apply_analysis_parameters(
983990
self.current_analysis_obj, set_current=True
984991
),
985992
)
993+
return True
986994

987995
def __get_processor_associated_to(
988996
self, obj: SignalObj | ImageObj
@@ -3111,23 +3119,54 @@ def recompute_1_to_1_objects(
31113119
return recomputed_uuids, True
31123120
return recomputed_uuids, False
31133121

3114-
def recompute_1_to_0_objects(self, objects: list[SignalObj | ImageObj]) -> None:
3122+
def recompute_1_to_0_objects(
3123+
self, objects: list[SignalObj | ImageObj]
3124+
) -> tuple[set[str], bool]:
31153125
"""Recompute 1-to-0 analysis operations for the given objects.
31163126
31173127
Args:
31183128
objects: Objects with stored 1-to-0 analysis parameters
31193129
"""
31203130
if not objects:
3121-
return
3131+
return set(), False
3132+
recomputed_uuids: set[str] = set()
31223133
with create_progress_bar(
31233134
self, _("Recomputing analyses"), max_=len(objects)
31243135
) as progress:
31253136
for index, obj in enumerate(objects):
31263137
progress.setValue(index + 1)
31273138
QW.QApplication.processEvents()
31283139
if progress.wasCanceled():
3129-
break
3130-
self.processor.recompute_analysis(obj)
3140+
return recomputed_uuids, True
3141+
try:
3142+
success = self.processor.recompute_analysis(obj)
3143+
message = _("Analysis computation failed.")
3144+
except Exception as exc: # pylint: disable=broad-exception-caught
3145+
success = False
3146+
message = str(exc)
3147+
if success:
3148+
recomputed_uuids.add(get_uuid(obj))
3149+
continue
3150+
if execenv.unattended:
3151+
continue
3152+
failtxt = _("Failed to recompute analysis")
3153+
if index == len(objects) - 1:
3154+
QW.QMessageBox.warning(
3155+
self,
3156+
_("Recompute"),
3157+
f"{failtxt} '{obj.title}':\n{message}",
3158+
)
3159+
else:
3160+
conttxt = _("Do you want to continue with the next object?")
3161+
answer = QW.QMessageBox.warning(
3162+
self,
3163+
_("Recompute"),
3164+
f"{failtxt} '{obj.title}':\n{message}\n\n{conttxt}",
3165+
QW.QMessageBox.Yes | QW.QMessageBox.No,
3166+
)
3167+
if answer == QW.QMessageBox.No:
3168+
return recomputed_uuids, True
3169+
return recomputed_uuids, False
31313170

31323171
def select_source_objects(self) -> None:
31333172
"""Select source objects associated with the selected object's processing.

datalab/gui/panel/history/chainmodel.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@ def remap_processing_parameters(
150150
param=parameters.param,
151151
source_uuid=source_uuid,
152152
source_uuids=source_uuids,
153+
plugin_origin=parameters.plugin_origin,
153154
)
154155

155156

datalab/gui/panel/history/interactive_replay.py

Lines changed: 60 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,10 @@
1414

1515
from datalab.config import _
1616
from datalab.env import execenv
17+
from datalab.gui.panel.history import chain as hchain
1718
from datalab.gui.panel.history import recompute as hrec
1819
from datalab.history import HistoryAction, HistorySession
20+
from datalab.history.core import copy_history_value
1921

2022
if TYPE_CHECKING:
2123
from datalab.gui.panel.history.panel import HistoryPanel
@@ -124,11 +126,13 @@ def prompt_edit_action_params(
124126

125127

126128
def edit_mode_replay_actions(panel: HistoryPanel, actions: list[HistoryAction]) -> None:
127-
"""Edit and recompute only the selected actions, in session order.
129+
"""Edit selected actions and recompute their affected branches once.
128130
129-
Each selected action gets exactly one parameter dialog; non-selected
130-
downstream actions are left untouched (no automatic cascade). A
131-
re-entrance guard prevents nested prompt loops.
131+
Each selected action gets exactly one parameter dialog. Recomputable
132+
selected actions are always included, while accepted parameter edits also
133+
include all downstream dependent actions. The resulting global plan is
134+
deduplicated and executed in session order. A re-entrance guard prevents
135+
nested prompt loops.
132136
"""
133137
# Deduplicate and sort the selected actions in their session order
134138
ordered = order_selected_actions(panel, actions)
@@ -137,8 +141,16 @@ def edit_mode_replay_actions(panel: HistoryPanel, actions: list[HistoryAction])
137141
with panel.runtime.execution.replaying_edits() as started:
138142
if not started:
139143
return
144+
entry_states = {
145+
action.uuid: (
146+
copy_history_value(action.kwargs),
147+
copy_history_value(action.saved_kwargs),
148+
)
149+
for action in ordered
150+
}
140151
edited_actions: list[HistoryAction] = []
141152
recomputable: list[HistoryAction] = []
153+
deferred_actions: list[HistoryAction] = []
142154
for action in ordered:
143155
is_creation = (
144156
action.kind == HistoryAction.KIND_UI
@@ -148,26 +160,54 @@ def edit_mode_replay_actions(panel: HistoryPanel, actions: list[HistoryAction])
148160
action.kind == HistoryAction.KIND_COMPUTE and action.pattern is not None
149161
)
150162
if not is_creation and not is_compute:
151-
with panel.replaying(), panel.output_suppressed():
152-
action.replay(panel.mainwindow, restore_selection=True, edit=True)
163+
deferred_actions.append(action)
153164
continue
154165
result = prompt_edit_action_params(panel, action)
155166
if result is False:
156-
for done in edited_actions:
157-
done.restore_kwargs()
158-
panel.tree.refresh_action_item(done)
167+
for selected_action in ordered:
168+
kwargs, saved_kwargs = entry_states[selected_action.uuid]
169+
selected_action.kwargs = kwargs
170+
selected_action.saved_kwargs = saved_kwargs
171+
panel.tree.refresh_action_item(selected_action)
159172
return
160173
if result is True:
161174
edited_actions.append(action)
162175
recomputable.append(action)
163176

164177
for action in edited_actions:
165178
panel.tree.refresh_action_item(action)
166-
for action in recomputable:
167-
hrec.recompute_action_in_place(panel, action)
179+
planned = list(recomputable)
180+
for action in edited_actions:
181+
planned.extend(hchain.get_downstream_actions(panel, action))
182+
planned = order_selected_actions(panel, planned)
183+
execution_plan = order_selected_actions(panel, deferred_actions + planned)
184+
for action in planned:
185+
action.is_stale = True
168186
panel.tree.refresh_action_item(action)
169-
if edited_actions:
170-
hrec.recompute_cascade(panel, edited_actions[0])
187+
QW.QApplication.processEvents()
188+
blocked_outputs: set[str] = set()
189+
try:
190+
for action in execution_plan:
191+
if action in deferred_actions:
192+
with panel.replaying(), panel.output_suppressed():
193+
action.replay(
194+
panel.mainwindow, restore_selection=True, edit=True
195+
)
196+
continue
197+
if hchain.action_consumes_any(action, blocked_outputs):
198+
blocked_outputs.update(
199+
panel.runtime.objects.action_output_uuids.get(action.uuid, [])
200+
)
201+
continue
202+
success = hrec.recompute_action_in_place(panel, action)
203+
action.is_stale = not success
204+
panel.tree.refresh_action_item(action)
205+
if not success:
206+
blocked_outputs.update(
207+
panel.runtime.objects.action_output_uuids.get(action.uuid, [])
208+
)
209+
finally:
210+
hrec.flush_cascade_warnings(panel)
171211
QW.QApplication.processEvents()
172212

173213

@@ -214,6 +254,11 @@ def restore_action_params(
214254
continue
215255
action.restore_kwargs()
216256
panel.tree.refresh_action_item(action)
217-
hrec.recompute_action_in_place(panel, action)
218-
hrec.recompute_cascade(panel, action)
257+
success = hrec.recompute_action_in_place(panel, action)
258+
action.is_stale = not success
259+
panel.tree.refresh_action_item(action)
260+
if not success:
261+
break
262+
if not isinstance(item, HistorySession):
263+
hrec.recompute_cascade(panel, action)
219264
panel.ui.update_actions_state()

0 commit comments

Comments
 (0)