refactor(macos): extract the fail-soft degrade-and-return pattern in presentation.py - #373
Merged
Merged
Conversation
…presentation.py show_thumbnail, before_capture, after_capture, and encode_observation each wrapped their body in an identical try/except Exception: await self._degrade(reason, error); return default. Pull that shape into a shared _fail_soft(reason, default) decorator so the four methods keep only their distinct bodies and reason/default values. No public behavior change: MacOSPresentationController's signatures and return values are unchanged, and the existing presentation/computer test suites (109 tests) plus the full non-slow suite (862 tests) pass unmodified. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F9iAjBxPM94GjhCENMjXP8
dhruvbatra
added a commit
that referenced
this pull request
Sep 4, 2026
…_present_status (#378) present and _present_status each wrapped their body in the identical except asyncio.CancelledError: raise / except Exception as error: await self._degrade(f"presentation_failed:{type(error).__name__}", error) shape. Extract a shared _fail_soft_cancellable decorator (the cancellation-aware counterpart to the existing _fail_soft decorator from #373) so both methods keep only their distinct dispatch bodies. No behavior change: the try/except previously wrapped the code below the early-return guards in present, and those guards cannot raise, so moving them inside the decorated body is equivalent. Full suite (914 passed / 3 skipped / 1 deselected) and ruff check/format pass unmodified. Claude-Session: https://claude.ai/code/session_01NoGweAuMDdcoLXYHWgfY3j Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
MacOSPresentationController.show_thumbnail,.before_capture,.after_capture, and.encode_observationeach wrapped their entire body in an identical shape:This pulls that shape into a single
_fail_soft(reason, default)decorator (module-level,functools.wraps-preserving) so each of the four methods keeps only its distinct body plus its ownreason/default. Net: four duplicated try/except blocks become one shared helper, applied via decorator.Why this is safe / no public behavior change
yutori/navigator/macos/presentation.py— no function signatures, return types, or observable behavior change.functools.wrapskeeps__name__/__doc__/introspection intact.trywas ever wrapped around them either way).self._degradeis still resolved dynamically viaselfat call time, so existing tests that monkeypatchcontroller._degradeon the instance continue to work unchanged.presentand_present_statuswere deliberately left alone — they interleave multiple branches and must re-raiseasyncio.CancelledErrorbefore degrading, so they don't fit this single-exit shape and keep their own inlinetry/except.CancelledError-first shape (present,_present_status), and did not touch any other file.What I verified
ruff check .andruff format --checkpass on the changed file (repo-wideruff format --checkreports 13 pre-existing unrelated files with drift, none of which is this one — confirmed viagit diffthat onlypresentation.pychanged).pytest tests/test_navigator_macos_presentation.py tests/test_navigator_macos_computer.py -v— 109/109 pass, including the existing fail-soft/degrade coverage (test_capture_ids_are_monotonic_and_a_stale_transition_degrades,test_status_mode_thumbnail_rejection_degrades_fail_soft, etc.) unmodified.pytest -m "not slow"— 862 passed, 9 skipped (pre-existing), 1 deselected (slow marker).🤖 Generated with Claude Code
https://claude.ai/code/session_01F9iAjBxPM94GjhCENMjXP8
Generated by Claude Code
Note
Low Risk
Internal refactor in one file with no signature or observable behavior change; existing presentation fail-soft tests cover the same paths.
Overview
Introduces a module-level
_fail_soft(reason, default)async decorator that centralizes the shared “catch any exception →await self._degrade(reason, error)→ return default” pattern used by macOS presentation operations.show_thumbnail,before_capture,after_capture, andencode_observationnow use the decorator instead of four copy-pastedtry/exceptblocks; each method keeps its guard clauses and success-path logic unchanged.presentand_present_statusare intentionally unchanged because they must re-raiseasyncio.CancelledErrorbefore degrading.Reviewed by Cursor Bugbot for commit 924b13e. Bugbot is set up for automated code reviews on this repo. Configure here.