Skip to content

PromisingContext::get_trace()#79

Merged
teremterem merged 150 commits intomainfrom
promise-cancellations
Mar 25, 2026
Merged

PromisingContext::get_trace()#79
teremterem merged 150 commits intomainfrom
promise-cancellations

Conversation

@teremterem
Copy link
Copy Markdown
Owner

@teremterem teremterem commented Mar 18, 2026


Open with Devin

Summary by CodeRabbit

  • New Features

    • Tracing & inspection: get_trace(), format_trace(), print_trace(), collect_remaining_children().
    • Top-level sync entrypoints: PromisingFunction.run() and protected_run().
    • New RECURSIVELY sentinel as a documented default for recursive child-awaiting.
  • Behavior Changes

    • await_children / collect_remaining_children now default to recursive (full descendant subtree).
    • context decorator usage standardized around parameterized form (per-call overrides supported).
  • Improvements

    • Clearer event-loop mismatch detection and sentinel-usage errors.
    • Restructured documented error hierarchy (PromisingError / ContextError / EventLoopError).
  • Tests & Docs

    • Expanded tests and updated documentation for tracing, asyncio/run integration, decorator robustness, and APIs.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 18, 2026

📝 Walkthrough

Walkthrough

Adds a decorator/descriptor framework and per-call override settings, introduces PromisingFunction.run/protected_run, named sentinels (RECURSIVELY) and trace/child-inspection APIs, reorganizes error hierarchy, enforces event-loop identity on awaiting Promises, changes default child-awaiting to recursive, and adds many tests and examples.

Changes

Cohort / File(s) Summary
Decorator framework & plumbing
promising/decorator_support.py, promising/utils.py, promising/promising_function.py
New DecoratorSupportPromisingDecorator plumbing; PromisingFunction now inherits PromisingDecorator; async detection moved to asyncio.iscoroutinefunction; per-call settings routed via _SETTINGS_AS_DICT_KEY.
Context & tracing APIs
promising/promising_context.py, promising/__init__.py, README.md
context now built on PromisingDecorator; added get_trace/format_trace/print_trace and collect_remaining_children; default recursively=True for await/collect helpers; top-level exports updated.
Entrypoints & run semantics
promising/promising_function.py, examples/keyword_agent.py, tests/misc/test_decorator_asyncio_run.py
Added PromisingFunction.run() (sync entry using asyncio.run) and protected_run() (async); example updated to use @promising.function + .run(); tests cover threaded/.run() behaviors.
Promise await & event-loop checks
promising/promise.py, promising/promising_context.py
_AwaitablePromiseUnpacker.__await__ enforces running-loop identity via get_running_asyncio_loop and raises EventLoopMismatchError; root context loop selection uses get_running_asyncio_loop(raise_if_none=True); sync-guard errors updated.
Sentinels & boolean-coercion behavior
promising/sentinels.py, promising/__init__.py
Sentinels are named, RECURSIVELY added and exported; Sentinel.__bool__ now raises SentinelUsageError with guidance.
Errors & hierarchy
promising/errors.py, promising/__init__.py
BasePromisingErrorPromisingError; added ContextError and EventLoopError; event-loop and sentinel errors introduced/renamed (EventLoopMismatchError, NoRunningEventLoopError, SentinelUsageError); exports adjusted.
Tests: decorator robustness & asyncio integration
tests/misc/test_decorator_robustness.py, tests/misc/test_decorator_asyncio_run.py, tests/misc/test_traces.py
Large new test suites validating decorator stacking, immediate validation, thread-run/.run() behavior, and tracing utilities.
Tests: await-children & context updates
tests/promise/..., tests/promising_context/...
Expanded promise graphs to include grandchildren, updated await patterns and expected ordering; many sync tests converted to async and decorator invocation forms normalized.
Test helpers & normalization
tests/utils_for_tests.py, tests/promising_context/test_namespaces.py
Added normalize_object_repr and run_in_thread helpers; tests updated to use normalized reprs and adjusted namespace expectations.
Packaging / CI
pyproject.toml, CONTRIBUTORS.md
Commented out orjson from main deps, added pytest-timeout to dev extras, set pytest default timeout to 10s; documentation updated with new APIs/defaults.
Example change
examples/keyword_agent.py
Replaced manual asyncio startup with @promising.function + .run() and added unconditional promising.print_trace() call in keyword extraction.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Caller as CallSite
    participant PF as PromisingFunction.run
    participant AR as asyncio.run
    participant PR as protected_run()
    participant Ctx as PromisingContext
    participant Promise as Promise

    Caller->>PF: call .run(*args, await_children=RECURSIVELY)
    PF->>AR: asyncio.run(self.protected_run(...))
    AR->>PR: start protected_run coroutine
    PR->>Ctx: create/enter PromisingContext (select running loop)
    PR->>Promise: invoke wrapped function -> returns Promise (children may start)
    Promise-->>PR: Promise running
    PR->>Promise: await Promise result
    PR->>Ctx: finally -> collect_remaining_children(recursively=True)
    Ctx->>Promise: collect/await descendant promises
    Ctx-->>PR: children awaited, exit context
    PR-->>AR: coroutine complete
    AR-->>PF: asyncio.run returns result
    PF-->>Caller: return result
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

  • PR #61: Decorator/call-time configuration overlap — touches PromisingFunction/PromisingContext sentinel handling and per-call settings.
  • PR #65: Promise await internals — related changes to Promise.await and awaiter/loop checks.
  • PR #59: Namespace/representation handling — overlaps with namespace resolution and repr/trace expectations in tests.

Poem

🐇 I hopped through wrappers, tidy and spry,

I named the sentinels, so miscasts won't fly,
I traced each parent and child in a row,
Made run() greet loops and checked where they go,
A carrot for tests — now the traces all glow.

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'PromisingContext::get_trace()' is vague and refers to only one of many API additions in this changeset; the PR makes extensive changes across error hierarchies, decorator semantics, recursion defaults, and infrastructure, making the title overly narrow and not representative of the main scope. Consider retitling to capture the primary change, such as 'Add trace/inspection APIs and restructure error hierarchy' or 'Refactor decorator infrastructure with recursive child awaiting defaults' depending on which change the author considers most important.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch promise-cancellations

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@teremterem
Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 18, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 5 additional findings.

Open in Devin Review

@teremterem teremterem marked this pull request as ready for review March 25, 2026 13:13
coderabbitai[bot]

This comment was marked as resolved.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
promising/promising_context.py (1)

753-761: 🧹 Nitpick | 🔵 Trivial

TODO comment notes incorrect error class/message.

The comment at lines 755-757 correctly identifies that SyncUsageError and its message are no longer accurate for all cases where this exception is raised. Consider creating a follow-up issue to address this technical debt.

Do you want me to open a GitHub issue to track updating the error class and message to accurately reflect all scenarios where this exception can be raised?

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@promising/promising_context.py` around lines 753 - 761, The TODO notes that
_call_soon_threadsafe currently raises SyncUsageError with a message mentioning
synchronous operations, which is inaccurate; change the raised exception in
_call_soon_threadsafe from SyncUsageError to a more general/appropriate error
(e.g., EventLoopNotRunningError or ContextUsageError) and update the message to
not reference "synchronous operation" (suggest: "The event loop associated with
this {self.__class__.__name__} is not running"); add or update the custom
exception class (if needed) and keep the TODO comment or open a follow-up issue
to track any broader refactor of error taxonomy so all callers reflect the new
semantics.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@promising/promising_function.py`:
- Around line 155-160: Replace the private asyncio hook usage by using
inspect.markcoroutinefunction() on Python 3.12+ and fall back to the existing
asyncio.coroutines._is_coroutine for older Pythons: update the assignment of
_is_coroutine in promising_function.py so that if sys.version_info >= (3,12) you
call inspect.markcoroutinefunction() to mark the Promising function type,
otherwise keep the current asyncio.coroutines._is_coroutine fallback (keep
reference to the existing symbol _is_coroutine and the current logic that relies
on asyncio.coroutines._is_coroutine for 3.11).

---

Outside diff comments:
In `@promising/promising_context.py`:
- Around line 753-761: The TODO notes that _call_soon_threadsafe currently
raises SyncUsageError with a message mentioning synchronous operations, which is
inaccurate; change the raised exception in _call_soon_threadsafe from
SyncUsageError to a more general/appropriate error (e.g.,
EventLoopNotRunningError or ContextUsageError) and update the message to not
reference "synchronous operation" (suggest: "The event loop associated with this
{self.__class__.__name__} is not running"); add or update the custom exception
class (if needed) and keep the TODO comment or open a follow-up issue to track
any broader refactor of error taxonomy so all callers reflect the new semantics.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: d384e834-a393-4a99-8e82-14cb25c1dc9e

📥 Commits

Reviewing files that changed from the base of the PR and between c198404 and 8c25d64.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (6)
  • CONTRIBUTORS.md
  • README.md
  • promising/__init__.py
  • promising/promise.py
  • promising/promising_context.py
  • promising/promising_function.py

Comment thread promising/promising_function.py
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@promising/__init__.py`:
- Around line 44-49: Add a disableable defensive check where synchronous promise
call depth is tracked (the code path that schedules work into the
ThreadPoolExecutor and the Promise/PromiseExecutor logic) to detect when current
synchronous call depth >= executor.max_workers and raise a clear, specific
exception (e.g., SynchronousCallDepthExceededError or ThreadPoolExhaustionError)
instead of deadlocking; make this behavior configurable via a toggle (e.g., a
module-level flag or env var like PROMISING_DISABLE_DEPTH_CHECK or an optional
parameter on Promise/PromiseExecutor with default enabled) so callers can opt
out, and update/add unit tests to assert the exception is raised when depth
reaches max_workers and that the toggle disables the check.

In `@promising/promise.py`:
- Around line 109-113: Create two tracked GitHub issues for the P1 TODOs
referenced in promising/promise.py: one for "support async generator interface"
and one for "memory leak prevention for long sequences/streams". Each issue
should include a clear problem statement, proposed approaches (e.g., async
iterator protocol support, backpressure/eviction strategies or weak buffering,
explicit APIs for one-pass streams), acceptance criteria, and labels (e.g., P1,
architecture). Then update the TODO comments in promising/promise.py to include
the created issue numbers (e.g., TODO #[issue-number]) and a short note pointing
to the issue for design decisions and progress tracking.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 743bf1a5-8a95-4a5c-a34f-f83ce40533d6

📥 Commits

Reviewing files that changed from the base of the PR and between c198404 and 059392f.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (8)
  • CONTRIBUTORS.md
  • README.md
  • promising/__init__.py
  • promising/errors.py
  • promising/promise.py
  • promising/promising_context.py
  • promising/promising_function.py
  • promising/utils.py

Comment thread promising/__init__.py
Comment thread promising/promise.py
@teremterem teremterem merged commit ef65415 into main Mar 25, 2026
3 checks passed
@teremterem teremterem deleted the promise-cancellations branch March 25, 2026 20:28
@coderabbitai coderabbitai Bot mentioned this pull request Mar 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant