Skip to content

Commit fdec144

Browse files
tobixenclaude
andcommitted
fix: remove stray breakpoint() from _procrastinate (code review #13)
Postponing a task that has parents could drop the user straight into pdb: import inspect stack_depth = len(inspect.stack()) if stack_depth > 13: breakpoint() This is item #13 of docs/code-review-2026-06-12.md, the one correctness bug from that review left on hold. It is reachable in normal use - the guard sits in the recursive "postpone the parent too" path, entered from `interactive check-due` and `dismiss-panic`, and a plain `edit --postpone` already sits at ~12 frames, so a couple of levels of recursion crosses the threshold. With no tty attached it does not even look like a debugger, just a hang. The stack-depth probe was a debugging aid for the recursion, not a feature; drop the whole block. Marked fixed in the review document, and covered by a test asserting the function stays free of breakpoint() and inspect.stack(). Prompt: /review-and-push create a pull request from this branch [review-and-push skill; the branch already had draft PR #43, so the skill ran its full review procedure over the branch instead of opening a new PR] Followup-Prompt: (answer to a clarifying question) A1-A5 plus CHANGELOG and the timespec tests [= fix all five confirmed bugs from the clean-context code review, add the missing CHANGELOG entries, and add regression tests for the timespec fixes] Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Reviewed-by: Tobias Brox <tobias@redpill-linpro.com>
1 parent c78b07e commit fdec144

3 files changed

Lines changed: 13 additions & 5 deletions

File tree

‎docs/code-review-2026-06-12.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -391,7 +391,7 @@ then the remaining crash-on-edge-case items **#6-#15**. The duplication of
391391
| 10 | `timeline_suggestion` crashes on all-day / DTEND-less events | ✅ Fixed | `panic_planning.py:112` |
392392
| 11 | `interactive check-due --limit` is a string, not int | ✅ Fixed | `cli.py:519` — added `type=int` |
393393
| 12 | `obj.icalendar_component_UID` crashes in inconsistency-log path | ✅ Fixed | `lib.py:355` |
394-
| 13 | Leftover `breakpoint()` in `_procrastinate` | ⏸ On hold | `lib.py:241-244` |
394+
| 13 | Leftover `breakpoint()` in `_procrastinate` | ✅ Fixed | `lib.py:241-244` |
395395
| 14 | `time_tracking` string iterated char-by-char | ✅ Fixed | `lib.py:165` — wrap scalar in list |
396396
| 15 | `dismiss-panic` double-prefixes lookahead (`++60d`) | ✅ Fixed | `cli.py:538` + `commands.py:466` |
397397
| C1 | `_interactive_edit` duplicated in `commands.py` (stale copy) | ✅ Fixed | Deleted copy, import canonical from `interactive` |

‎plann/lib.py‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -329,10 +329,6 @@ def _procrastinate(objs, delay, check_dependent="error", with_children=False, wi
329329
else:
330330
err_callback(f"{summary} could not be postponed due to parent {_summary(p)} with due {_ensure_ts(p['DUE'])} and priority {p.get('priority', 0)}")
331331
if p_postponable and (p_auto_postponable or confirm_callback("procrastinate parent?")):
332-
import inspect
333-
stack_depth = len(inspect.stack())
334-
if stack_depth > 13:
335-
breakpoint()
336332
_procrastinate([parent], new_due+max(parent.get_duration()+x.get_duration()+datetime.timedelta(minutes=1), datetime.timedelta(minutes=1)), check_dependent=check_dependent, err_callback=err_callback, confirm_callback=confirm_callback, recursivity=recursivity+1)
337333
_procrastinate([x], new_due, check_dependent=check_dependent, err_callback=err_callback, confirm_callback=confirm_callback, recursivity=recursivity+1)
338334
elif check_dependent == "return":

‎tests/test_lib.py‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -479,3 +479,15 @@ def test_list_top_down_caches_relative_fetches():
479479
## without caching, p is fetched once per child (and more via recursion)
480480
assert cal.fetch_counts['p'] == 1, dict(cal.fetch_counts)
481481
assert all(count <= 1 for count in cal.fetch_counts.values()), dict(cal.fetch_counts)
482+
483+
484+
def test_procrastinate_has_no_breakpoint():
485+
"""A stray breakpoint() drops the user into pdb (or appears to hang when
486+
there is no tty). Code review item #13 - the recursive postpone-parent
487+
path in _procrastinate had one guarded by an inspect.stack() depth check,
488+
which is reachable from `interactive check-due` / `dismiss-panic`."""
489+
import inspect as _inspect
490+
491+
source = _inspect.getsource(_procrastinate)
492+
assert 'breakpoint()' not in source
493+
assert 'inspect.stack()' not in source

0 commit comments

Comments
 (0)