fix(debug): retry incomplete debug worker on session restore and log failures - #213
Conversation
…failures On session restore, non-success rows with missing debug paths (NULL debug_json_path) caused the Open JSON/Excel buttons in DebugInfoDialog to remain permanently greyed out. This happened when the debug worker was cancelled or failed before completing in a prior session, and the stale NULL paths persisted in gui.db. - Restart the debug worker on session restore for any non-success rows whose debug_status is not 'done' or 'error'. - Log update_debug_info() failures so silent SQL UPDATE failures are visible in stderr instead of leaving buttons permanently disabled.
There was a problem hiding this comment.
🟡 Changes recommended
The new session-restore retry can unintentionally reset already-complete debug rows to pending/NULL paths because the debug worker currently reinitializes all non-success rows.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Fixes a session-restore edge case in the statement results/debug workflow where Debug Information “Open JSON/Excel” actions can remain disabled due to persisted incomplete debug metadata in gui.db.
Changes:
- Re-runs the debug worker after session restore when any non-success result row has incomplete debug status.
- Logs
update_debug_info()SQL update failures during debug generation so silent persistence errors are visible.
File summaries
| File | Description |
|---|---|
src/openstan/presenters/statement_result_presenter.py |
Detects incomplete debug state after restore and retries debug generation; adds stderr logging on debug-info DB update failure. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
__start_debug_worker was resetting ALL non-success rows to 'pending' and reprocessing them, which overwrote valid debug_json_path values from prior sessions. Filter to only rows whose debug_status is not 'done' or 'error' so completed debug data is preserved.
There was a problem hiding this comment.
🟡 Changes recommended
Pending-state DB updates still ignore failures (remaining silent in a key code path) and the new restore-retry behavior lacks regression test coverage.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 2
- Review effort level: Lite
- Log update_debug_info() failures when marking rows as 'pending' in __start_debug_worker, matching the logging added for 'done' updates. - Add 6 unit tests covering the session-restore debug worker retry: triggers on None/pending status, skips done/error, respects project_path guard, handles mixed statuses.
There was a problem hiding this comment.
🟢 Approval recommended
The changes directly address the reported failure mode, constrain reprocessing to incomplete rows, and include focused unit tests for the new retry logic.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Lite
Problem
When viewing Debug Information for review/failing statements, the Open JSON and Open Excel buttons remain permanently greyed out after session restore, even though the debug files were created correctly.
This occurs when the debug worker was cancelled or failed before completing in a prior session — the stale
debug_json_path = NULLvalues persist ingui.db, and the worker is never re-run.Closes #212
Changes
load_results_from_db: After restoring rows from the DB on session restart, check if any non-success rows have incomplete debug status (debug_statusnot "done" or "error"). If so, restart the debug worker so those rows get their debug files generated.__on_debug_entry_done: Logupdate_debug_info()failures so silent SQL UPDATE failures are visible in stderr instead of leaving buttons permanently disabled.Root cause confirmed by
Deleting and recreating the installed version's
gui.dbresolved the issue, confirming the stale NULL paths were the cause.