fix: standardize error handling for bulk prep/correction reapply - #260
fix: standardize error handling for bulk prep/correction reapply#260iabaako wants to merge 7 commits into
Conversation
Prep reapply-all previously let a failing step raise uncaught, crashing the page (e.g. a re-import that drops a column an earlier step used). Correction reapply-all silently swallowed failures with a bare except. Both now collect per-item failures, skip just the failing item, keep applying the rest, and surface one shared st.warning at the UI boundary, matching the existing pattern for interactive single-item actions. Closes #253 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR standardizes bulk reapply behavior for Prep steps and Corrections so that per-item failures are skipped and surfaced (rather than crashing Prep or silently disappearing in Corrections). It also adds a consistent status-based UX to both logs and fixes a correction-log caching lag after adding new corrections.
Changes:
- Introduces a shared
ReapplyFailuretype plus shared UI helpers to warn once with a consolidated list of skipped items and to style status cells consistently. - Updates Prep and Corrections bulk reapply to continue on failures, persist refreshed log rows with Successful/Failed statuses, and return failure details to the UI layer.
- Adds/updates tests to cover partial-failure scenarios, status backfilling for legacy logs, and the correction-log cache invalidation behavior.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/views/test_prep_view.py | Adds a view test asserting the Prep Change Log renders the new status column and preserves column ordering. |
| tests/views/test_correction_view.py | Adds unit tests for _build_correction_log_display (status backfill + column ordering). |
| tests/utils/test_reapply_utils.py | Adds tests for shared status styling and the consolidated warning banner helper. |
| tests/utils/test_prep_utils.py | Adds coverage for updated remove-columns messaging when some requested columns are missing. |
| tests/processing/test_prep.py | Expands prep processing tests for partial missing column removal and bulk reapply outcomes (success/failure/continuation). |
| tests/processing/test_corrections.py | Expands corrections processing tests for status logging, cache invalidation, and multiple failure reasons during bulk reapply. |
| src/datasure/views/prep_view.py | Displays Prep Change Log with a status column and styled status cells; surfaces bulk reapply failures after step removal. |
| src/datasure/views/import_view.py | Collects and warns about downstream prep/correction reapply failures after import refreshes. |
| src/datasure/views/correction_view.py | Adds a display helper to backfill/order status columns and styles the Correction Log status cells; surfaces reapply failures after removal. |
| src/datasure/utils/reapply_utils.py | New shared helpers: ReapplyFailure, highlight_status, and warn_reapply_failures. |
| src/datasure/utils/prep_utils.py | Updates remove-columns confirmation message generation to reflect partial removals and missing-column skips. |
| src/datasure/processing/prep.py | Implements per-action outcomes for bulk reapply, partial missing-column tolerance for removals, and persists refreshed prep logs with status. |
| src/datasure/processing/corrections.py | Implements correction reapply failure detection/reporting, persists refreshed correction logs with status + reason, and clears cached log/summary on updates. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…rt' and 'import from'' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
Suppressed comments (3)
src/datasure/views/import_view.py:121
- In the bulk-import flow, the status label is always set to "Data loaded successfully!" even when downstream reapply failures were collected and a warning is shown right after. This is user-facing and can be misleading during partial failure scenarios; consider updating the label when
all_failuresis non-empty.
status.update(
label="Data loaded successfully!", state="complete", expanded=True
)
src/datasure/processing/corrections.py:704
- For unexpected exceptions while applying a correction row, returning
str(e)can yield an empty string and loses the exception type. Including the exception class name makes the failure reason clearer instatus_reasonand the aggregated warning.
except Exception as e:
# Skip corrections that fail (data may have changed)
return data, str(e)
src/datasure/processing/prep.py:896
- When a prep action fails with an unexpected exception, the stored
errorisstr(e), which can be empty and omits the exception type. Including the exception class name makes the refreshed log and warning banner much more actionable for debugging.
except Exception as e:
outcomes.append(
PrepReapplyOutcome(
prep_args=action.prep_args, status="Failed", error=str(e)
)
| """Change Log renders a status column and styles a Failed row.""" | ||
| import importlib | ||
|
|
||
| import datasure.views.prep_view as pv_mod |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/datasure/processing/prep.py:1102
- When building
ReapplyFailureentries for failed prep steps,step=_generate_action_description(outcome.prep_args)will often produce a successful “✓ … removed … remaining” message (and may includeremaining_count=None) because failed outcomes keep the original loggedprep_argswithout counts. This makes the warning banner confusing/misleading for failures. Consider using a request-oriented description (e.g., action + requested columns) and ensurereasonis never None.
return [
ReapplyFailure(
step=_generate_action_description(outcome.prep_args), reason=outcome.error
)
for outcome in outcomes
if outcome.status == "Failed"
]
b844d3d to
11b69e2
Compare
|
wkirui
left a comment
There was a problem hiding this comment.
This works as expected!
One quirk that I noticed and which is already in the pipeline for fixing is inverted data transformation steps: Removing row by condition - equal to and not equal to functionalities are inverted.
A non-related issue found: When I upload two data sources, I found the following error.
2026-08-12 16:04:09.076 Uncaught app execution
Traceback (most recent call last):
File "C:\Users\Wesley\Desktop\Projects\Python\datasure\.venv\Lib\site-packages\streamlit\runtime\scriptrunner\exec_code.py", line 129, in exec_func_with_error_handling
result = func()
^^^^^^
File "C:\Users\Wesley\Desktop\Projects\Python\datasure\.venv\Lib\site-packages\streamlit\runtime\scriptrunner\script_runner.py", line 789, in code_to_exec
exec(code, module.__dict__) # noqa: S102
^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "C:\Users\Wesley\Desktop\Projects\Python\datasure\src\datasure\app.py", line 210, in <module>
nav_menu.run()
File "C:\Users\Wesley\Desktop\Projects\Python\datasure\.venv\Lib\site-packages\streamlit\navigation\page.py", line 490, in run
exec(code, module.__dict__) # noqa: S102
^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "C:\Users\Wesley\Desktop\Projects\Python\datasure\src\datasure\views\import_view.py", line 342, in <module>
render_local_file_form(project_id)
File "C:\Users\Wesley\Desktop\Projects\Python\datasure\src\datasure\connectors\local.py", line 234, in render_local_file_form
_handle_form_submission(
File "C:\Users\Wesley\Desktop\Projects\Python\datasure\src\datasure\connectors\local.py", line 294, in _handle_form_submission
import_log = pl.concat([import_log, new_row_df], how="vertical")
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "C:\Users\Wesley\Desktop\Projects\Python\datasure\.venv\Lib\site-packages\polars\functions\eager.py", line 287, in concat
out = wrap_df(plr.concat_df(elems))
^^^^^^^^^^^^^^^^^^^^
polars.exceptions.ShapeError: unable to vstack, column names don't match: "source" and "alias"
How I got here:
- Load a server dataset from SurveyCTO
- Load a separate local csv file
Actually trying to reproduce this, the data prep page shows that the data from the local file don't exist yet I have loaded it.
Claude suggested changing this line in src/connectors/local.y:
import_log = pl.concat([import_log, new_row_df], how="vertical") to
import_log = pl.concat([import_log, new_row_df], how="diagonal") or how = diagonal_relaxed



Pull Request Summary 🚀
What does this PR do? 📝
statuscolumn (Successful/Failed, color-coded text) to both the Prep change log and the Correction log, refreshed every time a reapply runs.Why is this change needed? 🤔
Prep and correction bulk-reapply previously handled per-item failures inconsistently and didn't match the UX already used for interactive single-item actions (see #253): a prep step that referenced a column dropped by a later re-import crashed the whole page, and a correction that failed to reapply was dropped with
except Exception: passand no user-facing signal. On top of that, there was no way to tell — after the fact — which steps/corrections in a log had actually applied successfully versus silently failed, which matters for data quality auditing.How was this implemented? 🛠️
datasure.utils.reapply_utilsmodule (ReapplyFailure,warn_reapply_failures,highlight_status) so prep and corrections report and display failures the same way.processing/prep.py:PrepProcessor.execute_all_actionsnow collects aPrepReapplyOutcomeper action (status + error) instead of raising through;RemoveColumnsOperationsplits requested columns into existing/missing instead of failing on any miss; the persisted prep log is rebuilt after each reapply with a freshstatus/description per step while keeping each step's original request intact (so a column that reappears later is still requested for removal, and replication scripts stay accurate).processing/corrections.py:_apply_correction_rownow checks column existence and compares the current value against what was recorded at log time before applying (_current_value_mismatch);_reapply_all_correctionsrecordsstatus/status_reasonper row and rewrites the correction log;add_correction_entrynow clears the correction log/summary caches so new entries appear immediately, and marks new entries "Successful" (with backfill for logs saved before the status columns existed).views/prep_view.pyandviews/correction_view.py: Change Log tables now select/display thestatuscolumn right afteraction, styled via a shared pandas Styler helper (text color, not background —st.dataframeis kept overst.tableso sorting/CSV download still work).correction_view.py's log-building logic was extracted into a plain_build_correction_log_displayhelper for direct unit testing, sincerender_correction_logis@st.fragment-wrapped and can't be exercised through the view-test mock.How to test or reproduce? 🧪
uv run python -m pytest— full suite passes.just lint-py/just fmt-python/uv tool run pre-commit run --all-files— clean.Successfulstatus with an updated description.statusflips toFailedwith a reason, and a warning banner summarizes it. Verified end-to-end against real DuckDB-backed project data, not just mocks.Screenshots (if applicable) 📷
Warning on import page after loading new dataset with missing columns or keys

New prep log with status

New corrections log with status and status reason

Checklist ✅
🤖 Generated with Claude Code