From c797bcb7832310a19525646b031271e0922d2dcf Mon Sep 17 00:00:00 2001 From: goodboy Date: Thu, 18 Jun 2026 13:37:55 -0400 Subject: [PATCH] Address Copilot review nits on PR #462 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All 5 flagged items were valid (4 real bugs + 1 dead assert), - fix an inverted `sys.version_info < (3, 14)` guard in `ipc._linux` — the "`cffi` has no 3.14 support" import note now fires on 3.14+ (where it applies) instead of on older pys. - use `os.environ.get('PYTHON_COLORS')` in the `sync_bp` example so it doesn't `KeyError` when run outside the test harness. - correct `dump_task_tree()`'s docstring: the `/tmp` + `/dev/tty` tee is gated on `write_file`/`write_tty`, not "unconditional". - tidy the `ActorTooSlowError` message spacing in `cancel_actor`. - replace a tautological `applied is True or applied is False` in `test_patches` with `isinstance(applied, bool)` (the value is order-dependent across the module). Review: PR #462 (copilot-pull-request-reviewer) https://github.com/goodboy/tractor/pull/462#pullrequestreview-4527179852 (this patch was generated in some part by [`claude-code`][claude-code-gh]) [claude-code-gh]: https://github.com/anthropics/claude-code --- examples/debugging/sync_bp.py | 2 +- tests/trionics/test_patches.py | 2 +- tractor/devx/_stackscope.py | 19 ++++++++++--------- tractor/ipc/_linux.py | 2 +- tractor/runtime/_portal.py | 6 +++--- 5 files changed, 16 insertions(+), 15 deletions(-) diff --git a/examples/debugging/sync_bp.py b/examples/debugging/sync_bp.py index 64a6e14b..8c4ba6e9 100644 --- a/examples/debugging/sync_bp.py +++ b/examples/debugging/sync_bp.py @@ -12,7 +12,7 @@ import tractor # disable `pbdp` prompt colors # for prompt matching in test. def disable_pdbp_color(): - if os.environ['PYTHON_COLORS'] == '0': + if os.environ.get('PYTHON_COLORS') == '0': from tractor.devx.debug import _repl _repl.TractorConfig.use_pygments = False diff --git a/tests/trionics/test_patches.py b/tests/trionics/test_patches.py index 9f2b942f..36bf1a59 100644 --- a/tests/trionics/test_patches.py +++ b/tests/trionics/test_patches.py @@ -59,7 +59,7 @@ def test_wakeup_socketpair_drain_eof_patch_works(): # First call MUST return True; idempotent guard # prevents False on subsequent calls within the # same process. - assert applied is True or applied is False # idempotent + assert isinstance(applied, bool) # idempotent (order-dependent value) # Cap wall-clock at 2s; SIGALRM raises in main # thread which interrupts the C-level recv loop diff --git a/tractor/devx/_stackscope.py b/tractor/devx/_stackscope.py index 35f22a49..e6ea00e5 100644 --- a/tractor/devx/_stackscope.py +++ b/tractor/devx/_stackscope.py @@ -71,17 +71,18 @@ def dump_task_tree( Do a classic `stackscope.extract()` task-tree dump to console at `.devx()` level. - Also unconditionally tee the rendered tree to two - capture-bypassing sinks so SIGUSR1 dumps remain visible - when the parent process has captured stdio (e.g. pytest's - default `--capture=fd`): + When `write_file`/`write_tty` are set, ALSO tee the rendered + tree to capture-bypassing sinks so SIGUSR1 dumps remain + visible when the parent process has captured stdio (e.g. + pytest's default `--capture=fd`); the SIGUSR1 handler passes + `write_file=True` for exactly this reason: - - `/tmp/tractor-stackscope-.log` (append-mode, always - written) — guaranteed-readable artifact even under CI + - `write_file` -> `/tmp/tractor-stackscope-.log` + (append-mode) — guaranteed-readable artifact even under CI / `nohup` / no-tty conditions. `tail -f` to follow. - - `/dev/tty` if a controlling terminal is attached — - best-effort, ignored if the device is missing or write - fails. pytest never captures the tty. + - `write_tty` -> `/dev/tty` if a controlling terminal is + attached — best-effort, ignored if the device is missing + or write fails. pytest never captures the tty. ''' import os diff --git a/tractor/ipc/_linux.py b/tractor/ipc/_linux.py index cd7de870..f7434ceb 100644 --- a/tractor/ipc/_linux.py +++ b/tractor/ipc/_linux.py @@ -24,7 +24,7 @@ import sys try: import cffi except ImportError as ie: - if sys.version_info < (3, 14): + if sys.version_info >= (3, 14): ie.add_note( f'The `cffi` pkg has no 3.14 support yet.\n' ) diff --git a/tractor/runtime/_portal.py b/tractor/runtime/_portal.py index e0577d69..738599ac 100644 --- a/tractor/runtime/_portal.py +++ b/tractor/runtime/_portal.py @@ -340,9 +340,9 @@ class Portal: raise_on_timeout ): raise ActorTooSlowError( - f'Peer {peer_id} did not ack `Actor.cancel()`' - f'-RPC within bounded wait of ' - f'{cancel_timeout!r}s' + f'Peer {peer_id} did not ack its ' + f'`Actor.cancel()` RPC within bounded wait ' + f'of {cancel_timeout!r}s' ) # legacy fire-and-forget path: log + return False so