feat: async def support#180
Merged
Merged
Conversation
b81fe5f to
2d888cb
Compare
#179) - Fix root-file step condition: was `SET_DEFAULT != ''` (stable-release only); now `SKIP != 'true'` so llms.txt/robots.txt/sitemap.xml land on gh-pages root on every push to main (dev/) and every tag deploy — resolves 404s on deployed docs - Add `overwrite_root_files` boolean dispatch input (default false): backfills skip root-file overwrite by default; auto-enabled when alias='stable' - Guard each cp with [ -f ] check: absent files at historical refs emit ::warning:: and are skipped gracefully instead of hard-failing - Update header comment: document all dispatch inputs, root-file deploy rules, and job-tree step --------- Co-authored-by: Claude Code <noreply@anthropic.com>
Contributor
Code Review SummaryStatus: No Issues Found | Recommendation: Merge No new issues found in the incremental diff ( Previous Issues Fixed Since Last Review
Other Observations (not in incremental diff)Unresolved issue from the previous review (not in current incremental diff scope, carried forward for maintainer awareness):
Files Reviewed
Reviewed by nemotron-3-super-120b-a12b-20230311:free · 221,951 tokens |
- Add `async def async_wrapped_fn` branch in `packing()` — mirrors sync wrapper exactly; `await` gates all four callable-invocation points; sync target under async source dispatched without `await` so callers can migrate from sync to async API in one step
- Extract `_build_call_plan` shared dispatch helper and `_CallPlan` dataclass into `_types.py` so sync and async wrappers share identical side-effect logic with no duplication
- Add async fixtures: `_async_source_{notify,remap,callable}` in `collection_deprecate.py`; `async_{notify,args_remap,callable}` wrappers covering all three TargetModes; `async_target` import (fixture absent from `collection_targets.py` — follow-up required)
- Add integration tests: round-trip × 3 TargetModes, eager-warning check, stacklevel check, `iscoroutinefunction(wrapper)` invariant; `_reset_async_state` autouse fixture mirrors generator pattern
- Add `pytest-asyncio >=0.21` to `tests/requirements.txt`
- Add `docs/guide/use-cases.md` async section with runnable `asyncio.run(...)` examples per TargetMode
- Add `docs/troubleshooting.md` concurrency note (`_WrapperState` racy under concurrent coroutines; `num_warns=-1` workaround)
- Update `docs/llms.txt` decision flowchart and Known Limitations for async support
Known gaps (follow-up commits): `async_target` missing from `collection_targets.py` (ImportError blocks suite); warning-timing contract (fires on await, not on call — docs/test alignment pending); B2 async-gen guard docs vs code mismatch; README async section; CHANGELOG entry.
---
Co-authored-by: Claude Code <noreply@anthropic.com>
- Add missing `async_target` to `tests/collection_targets.py` — BLOCKER-1; ImportError crashed entire suite on import of `collection_deprecate` - Populate `tests/unittests/test_call_plan.py` with 3 unit tests for `_build_call_plan` (callable-target, ARGS_REMAP, NOTIFY) — was 0 bytes - Rename `test_async_warning_fires_eagerly` → `test_async_warning_fires_on_await_not_on_call`; add separate pre/post-await `catch_warnings` blocks that actually assert warning silence before await — BLOCKER-2 - Fix warning-timing claim in `docs/guide/use-cases.md:1058` and `docs/llms.txt`: "fires at call time" → "fires when coroutine is awaited" — BLOCKER-2 - Remove false B2 async-gen guard claims from `docs/llms.txt` (lines 169, 213–214, 224, 230) and `docs/guide/use-cases.md:1124`; UserWarning-at-decoration-time is not emitted — BLOCKER-3 - Add `functools.partial(async_fn)` limitation (Python 3.9–3.11 drops coroutine flag) to `docs/troubleshooting.md` and `docs/llms.txt` Known Limitations — W4 - Add sync-decorator-over-async-def limitation to `docs/troubleshooting.md` and `docs/llms.txt` — W5 - Add async section to `README.md` with one example per TargetMode + TOC entry — W6 - Regenerate `tests/integration/test_readme.py` via phmdoctest (35 tests pass) — W6 - Add async coroutine wrapper entry to `CHANGELOG.md [Unreleased] ### Added` — W7 - Add clarifying stacklevel comment in `src/deprecate/deprecation.py` async path — M9 All 840 tests pass. --- Co-authored-by: Claude Code <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
isasyncgenfunction(source) returns False for async generators, so they silently fall through to the sync wrapper and break await wrapper(...). Add decoration-time guard between the coroutine-function branch and the sync wrapped_fn definition. --- Co-authored-by: Claude Code <noreply@anthropic.com>
The previous wording implied the short-circuit path always calls source(**resolved_kwargs); document that sources with *args receive source(*args, **original_kwargs) to preserve extra positional args. --- Co-authored-by: Claude Code <noreply@anthropic.com>
Without this guard the sync wrapper returns a bare coroutine object, producing a silent RuntimeWarning at GC time. Raise a TypeError at call time so the misconfiguration is caught immediately with a message pointing to the fix (declare source as async def, or use a sync target). --- Co-authored-by: Claude Code <noreply@anthropic.com>
Add inline comment block in packing() listing the three source types that inspect.iscoroutinefunction misses (plain-decorator-wrapped async, async __call__ objects, functools.partial on <=3.11) with the shared workaround. Add callable-with-async-__call__ entry to llms.txt Known Limitations to match the other already-documented false-negative cases. --- Co-authored-by: Claude Code <noreply@anthropic.com>
_update_kwargs_with_args stops at *args parameters and cannot convert extra positional values to keyword form. On the short-circuit path both wrappers previously called source(**plan.resolved_kwargs), silently dropping any positional args beyond the named parameters. Fix: when source_has_var_positional, call source(*args, **plan.original_kwargs) so that the full original positional tuple is forwarded. Applies to both the sync wrapped_fn and async async_wrapped_fn short-circuit paths. --- Co-authored-by: Claude Code <noreply@anthropic.com>
Lychee strips ${{ ... }} template syntax before applying exclude regexes,
emitting https://github.com/$ (not the original template form). The
previous update required ${{...}} literals that no longer appear in the
extracted URL so the exclude never fired and the 404 was reported.
Revert to the literal-$-at-end form: ^[hH]ttps://github\.com/\$(\.git)?$
---
Co-authored-by: Claude Code <noreply@anthropic.com>
_double and _identity were defined inline in test_call_plan.py, violating CONTRIBUTING's three-layer rule (target callables belong in collection_targets.py). Replace with the equivalent double_value and identity_value already present in collection_targets. --- Co-authored-by: Claude Code <noreply@anthropic.com>
Line 724 had a second consecutive horizontal-rule immediately after the one on line 722, with only a blank line between them. --- Co-authored-by: Claude Code <noreply@anthropic.com>
- Remove local unpacking of dep_cfg fields; use dep_cfg.X directly throughout _build_call_plan body (type-narrowing alias _am added for args_mapping inside nested closure) - Rename _target → normalized_target and _source_is_stacked → source_is_stacked in _build_call_plan signature and all call sites - Remove stale N1-step2 plan-reference comments from deprecation.py, test_callable_kinds.py, collection_deprecate.py, and tests/requirements.txt --- Co-authored-by: Claude Code <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.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.
Before submitting
What does this PR do?
async def async_wrapped_fnbranch inpacking()foriscoroutinefunction(source)— mirrors sync wrapper exactly, awaits target; sync targets under async source invoked directlyisasyncgenfunction(source)emitsUserWarningat decoration time (async gen support N4 still pending)pytest-asyncio >=0.21to test deps; 8 new async tests + 1 B2 test intest_callable_kinds.py## Asyncsection inuse-cases.md, concurrency FAQ introubleshooting.md, JSON-LD FAQ entry,llms.txtasync notesPR review
Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.
Did you have fun?
Make sure you had fun coding 🙃