try claude - add a basic config and play around with some quality of life changes i was offputting #584

RonnyPfannschmidt · 2025-06-19T18:37:19Z

Adds CLAUDE.md file with development commands and architecture overview for future Claude Code instances.

🤖 Generated with Claude Code

add the config
experiment with some code moves
plan out async

RonnyPfannschmidt · 2025-06-20T12:34:17Z

@nicoddemus please have a look, this is one of the changes i was dreading to start a while back - we have similar ones in pytest

RonnyPfannschmidt · 2025-06-20T12:38:58Z

src/pluggy/_manager.py

+        except Exception:  # pragma: no cover
+            impl_attr = None
+
+        if impl_attr is not None:


this one is a artifact i still need to resolve, but the followup on those is much lighter than the heavy lifting before

nicoddemus

Overall the changes look OK, however, my review was not through.

For the "ready for review" version of the PR, I believe a clean commit history is a must. There is a good amount of code that is introduced in some commits, only to be removed later.

nicoddemus · 2025-06-22T12:57:42Z

pyproject.toml

@@ -66,7 +67,9 @@ known-local-folder = ["pluggy"]
 lines-after-imports = 2

 [tool.setuptools_scm]
-version_file = "src/pluggy/_version.py"
+
+[tool.uv]


Please update docs/index.rst, Development section, about using uv.

src/pluggy/_hooks.py

RonnyPfannschmidt · 2025-06-23T22:03:53Z

i added greenlet for the asnyc support as complete recoloring was insane

RonnyPfannschmidt · 2025-06-23T22:09:40Z

@nicoddemus this is partially ready for discussion, i'll create a new branc hand cherry pick myself up over the course of the week

Agree with adding Claude's conf, can't review the refactor though.

RonnyPfannschmidt · 2025-06-25T15:43:30Z

i just forced myself to relearn git revlog

RonnyPfannschmidt · 2025-06-25T20:27:33Z

i made https://gist.github.com/RonnyPfannschmidt/e90dec6540d6be45d63793493fe0b811and fixed things

also add uv based tooling to ease things Co-Authored-By: Claude <noreply@anthropic.com> - add ruff/mypy - allow claude to run some commands directly

Introduces a class-based approach to replace the HookimplOpts TypedDict in future versions, with backward compatibility methods. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

…action - Add HookimplMarker.get_hookconfig() method that returns HookimplConfiguration directly - Update test files to use new method instead of accessing dynamic attributes - Fix mypy type errors in test_multicall.py and test_hookcaller.py - Update CLAUDE.md to use 'uv run mypy src/' command 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

- Remove HookImpl.opts deprecated attribute - Remove HookimplConfiguration.to_opts() method (no longer used) - Update benchmark tests to use hookimpl.get_hookconfig() directly - Eliminates internal HookimplOpts conversions while preserving public API 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

- Update HookimplMarker to store HookimplConfiguration objects directly instead of HookimplOpts dicts on decorated functions - Add new internal _parse_hookimpl method that works with HookimplConfiguration and handles both new-style configs and legacy dict-based configs - Convert parse_hookimpl_opts to compatibility shim that returns None, allowing legacy projects like pytest to override while modern code uses structured configuration throughout - Update test to use direct attribute access instead of dict .get() method - Eliminate redundant dict-to-config conversions in plugin registration 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

- Add new ProjectSpec class that manages hook markers and plugin manager creation - ProjectSpec encapsulates HookspecMarker, HookimplMarker, and PluginManager creation - Support for custom PluginManager subclasses via plugin_manager_cls parameter - All components share the same project_name ensuring consistent behavior - Update marker classes and PluginManager to accept str or ProjectSpec instances - Use module references (_project.ProjectSpec) to avoid circular imports - Add comprehensive test suite covering all ProjectSpec functionality 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

- Remove normalize_hookimpl_opts function as modern pluggy uses HookimplConfiguration directly - Remove HookimplConfiguration.from_opts class method (backward compatibility no longer needed) - Simplify _parse_hookimpl method by removing dict-based configuration handling - Use HookimplConfiguration constructor directly instead of factory method - Maintain compatibility with parse_hookimpl_opts override for subclasses like pytest - Remove unnecessary normalize_hookimpl_opts import from _manager.py 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

- Add HookspecConfiguration class mirroring HookimplConfiguration pattern - Replace HookspecOpts TypedDict usage with structured class internally - Add backward compatibility for HookCaller.set_specification method - Support both positional/keyword HookspecOpts and new HookspecConfiguration - Add comprehensive tests for new configuration system - Deprecate parse_hookspec_opts method to return None by default 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

Move hook configuration extraction methods from HookspecMarker and HookimplMarker to ProjectSpec to provide a unified API for accessing hook configurations. Changes: - Add ProjectSpec.get_hookspec_config() and get_hookimpl_config() methods - Remove HookspecMarker._get_hookconfig() and HookimplMarker.get_hookconfig() - Update PluginManager to use ProjectSpec methods for configuration extraction - Use getattr with default None instead of exception handling for better performance - Update tests to use the new ProjectSpec-based API 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

Update the benchmark file to use the new ProjectSpec.get_hookimpl_config() method instead of the removed HookimplMarker.get_hookconfig() method. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

- Create dedicated HistoricHookCaller class for historic hooks - Remove historic functionality from regular HookCaller class - Implement handover mechanism in PluginManager to convert HookCaller to HistoricHookCaller when historic specs are detected - Update _SubsetHookCaller to properly delegate historic operations - Add HistoricHookCaller to public API exports - Historic hooks now have simplified implementation ordering (no wrapper support) - Improves separation of concerns and performance for non-historic hooks 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

- Create HookCaller protocol defining public interface for all hook callers - Rename HookCaller to NormalHookCaller for non-historic hook handling - Make HistoricHookCaller standalone (no inheritance) - Rename _SubsetHookCaller to SubsetHookCaller and make standalone - Delegate argument verification to HookSpec.verify_all_args_are_provided() - SubsetHookCaller now maintains same invariants as proxied hook caller: - Historic: only allows call_historic(), blocks __call__() and call_extra() - Normal: allows __call__() and call_extra(), blocks call_historic() - Both: block set_specification() as read-only proxies - Update type hints throughout codebase to use protocol interface - Maintain backward compatibility with _HookCaller and _SubsetHookCaller aliases All tests pass (142/142) and code quality checks pass. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

Split the large _hooks.py file (1132 lines) into three focused modules: - _hook_config.py: Configuration classes and type definitions (HookspecOpts, HookimplOpts, HookspecConfiguration, HookimplConfiguration) - _hook_markers.py: Hook decorators and specifications (HookspecMarker, HookimplMarker, HookSpec, varnames utility) - _hook_callers.py: Hook caller implementations and HookImpl (HookCaller protocol, NormalHookCaller, HistoricHookCaller, SubsetHookCaller, HookRelay, HookImpl) The original _hooks.py now serves as a backward compatibility module that re-exports all symbols, maintaining 100% API compatibility. Updated all internal imports across the codebase to use the new module structure while preserving existing public interfaces. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

…lots__ classes This change improves type safety and code organization by: - Moving all Final annotations from constructors to class level for consistency - Adding proper type annotations to all __slots__ classes - Simplifying constructor assignments by removing redundant type annotations Classes updated: - HistoricHookCaller: 5 Final attributes moved to class level - NormalHookCaller: 4 Final attributes moved to class level - HookImpl: 11 Final attributes moved to class level - HookspecMarker: _project_spec moved to class level with Final - HookimplMarker: _project_spec moved to class level with Final - HookspecConfiguration: 4 Final attributes moved to class level - HookimplConfiguration: 6 Final attributes moved to class level - HookRelay: Added __dict__ type annotation - SubsetHookCaller: Added type annotations for _orig and _remove_plugins - HookSpec: Added type annotations for all 8 attributes - Result: Added type annotations for all 3 attributes 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

- Split single _hookimpls list into _normal_hookimpls and _wrapper_hookimpls - Add _insert_hookimpl_into_list() utility to eliminate code duplication - Use unpacking syntax [*first, *second] for cleaner list combinations - Simplifies hook management by removing complex splitpoint calculations - Maintains all existing ordering semantics and test compatibility 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

This change introduces type-safe separation between normal hook implementations and wrapper implementations by: - Creating WrapperImpl subclass for wrapper/hookwrapper implementations - Modifying _multicall to handle normal and wrapper implementations separately - Updating hook callers to maintain separate lists and enforce type constraints - Adding factory method in HookimplConfiguration for proper type instantiation - Ensuring historic hooks reject wrapper implementations at registration time The separation improves type safety, clarifies execution flow, and maintains backward compatibility while providing better error messages for misuse. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

Move the duplicated argument extraction and error handling logic from _multicall into a private _get_call_args() method on HookImpl. This eliminates code duplication between wrapper and normal hook implementation processing paths and centralizes the argument verification logic. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

…hook pattern The _multicall function had intricate generator-based wrapper teardown logic that was difficult to understand and maintain. This refactoring introduces a completion hook pattern that simplifies the wrapper invocation process. Key changes: - Add CompletionHook type alias with (result, exception) -> (result, exception) signature - Implement setup_and_get_completion_hook() method in WrapperImpl class - Unify old-style and new-style wrapper handling using run_old_style_hookwrapper adapter - Replace complex teardown loop with simple completion hook calls - Adjust warning stacklevel from 6 to 7 for correct source attribution The completion hook pattern makes wrapper teardown explicit and easier to debug while maintaining full backward compatibility with both hookwrapper and wrapper implementations. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

Split the large _hooks.py file (1132 lines) into three focused modules: - _hook_config.py: Configuration classes and type definitions (HookspecOpts, HookimplOpts, HookspecConfiguration, HookimplConfiguration) - _hook_markers.py: Hook decorators and specifications (HookspecMarker, HookimplMarker, HookSpec, varnames utility) - _hook_callers.py: Hook caller implementations and HookImpl (HookCaller protocol, NormalHookCaller, HistoricHookCaller, SubsetHookCaller, HookRelay, HookImpl) The original _hooks.py now serves as a backward compatibility module that re-exports all symbols, maintaining 100% API compatibility. Updated all internal imports across the codebase to use the new module structure while preserving existing public interfaces. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

…lots__ classes This change improves type safety and code organization by: - Moving all Final annotations from constructors to class level for consistency - Adding proper type annotations to all __slots__ classes - Simplifying constructor assignments by removing redundant type annotations Classes updated: - HistoricHookCaller: 5 Final attributes moved to class level - NormalHookCaller: 4 Final attributes moved to class level - HookImpl: 11 Final attributes moved to class level - HookspecMarker: _project_spec moved to class level with Final - HookimplMarker: _project_spec moved to class level with Final - HookspecConfiguration: 4 Final attributes moved to class level - HookimplConfiguration: 6 Final attributes moved to class level - HookRelay: Added __dict__ type annotation - SubsetHookCaller: Added type annotations for _orig and _remove_plugins - HookSpec: Added type annotations for all 8 attributes - Result: Added type annotations for all 3 attributes 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

- Split single _hookimpls list into _normal_hookimpls and _wrapper_hookimpls - Add _insert_hookimpl_into_list() utility to eliminate code duplication - Use unpacking syntax [*first, *second] for cleaner list combinations - Simplifies hook management by removing complex splitpoint calculations - Maintains all existing ordering semantics and test compatibility 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

This change introduces type-safe separation between normal hook implementations and wrapper implementations by: - Creating WrapperImpl subclass for wrapper/hookwrapper implementations - Modifying _multicall to handle normal and wrapper implementations separately - Updating hook callers to maintain separate lists and enforce type constraints - Adding factory method in HookimplConfiguration for proper type instantiation - Ensuring historic hooks reject wrapper implementations at registration time The separation improves type safety, clarifies execution flow, and maintains backward compatibility while providing better error messages for misuse. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

…hook pattern The _multicall function had intricate generator-based wrapper teardown logic that was difficult to understand and maintain. This refactoring introduces a completion hook pattern that simplifies the wrapper invocation process. Key changes: - Add CompletionHook type alias with (result, exception) -> (result, exception) signature - Implement setup_and_get_completion_hook() method in WrapperImpl class - Unify old-style and new-style wrapper handling using run_old_style_hookwrapper adapter - Replace complex teardown loop with simple completion hook calls - Adjust warning stacklevel from 6 to 7 for correct source attribution The completion hook pattern makes wrapper teardown explicit and easier to debug while maintaining full backward compatibility with both hookwrapper and wrapper implementations. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

RonnyPfannschmidt · 2025-06-27T07:06:24Z

src/pluggy/_async.py

+to return awaitable objects that are automatically awaited when running in an
+async context.
+"""
+


@simonw i'd appreciate early feedback on this wrt utility for datasette

its not the deep integration like I wanted/hoped to implement at first

however its a directly working integration that doesn’t break on color mixing - but it does have the caveat of breaking never officieally supported threading patterns some people use

i currently intend to clean up history and then have this land and release

nicoddemus · 2025-06-28T12:31:58Z

src/pluggy/_manager.py

+            result = await pm.run_async(lambda: pm.hook.some_hook())
+        """
+
+        def wrapper() -> _T:


What is the purpose of this wrapper?

I changed the bottom of the function to return await self._async_submitter.run(func) (ignoring the wrapper), and all tests still pass.

This can be removed as i made the submitter mandatory after adding that api

Im not yet that far with the history trim down

I figured as much, but given I noticed it, figured it better to leave a comment.

nicoddemus · 2025-06-28T13:10:34Z

src/pluggy/_async.py

+        else:
+            raise RuntimeError("require_await called outside of async context")
+
+    async def run(self, sync_func: Callable[[], _T]) -> _T:


Sorry if this question is obvious—I don’t have much experience with greenlets.

Isn’t the purpose of using greenlet to have multiple worker greenlets and switch between them as they execute? In other words, if one worker is busy and can’t proceed, we can switch to another worker in the meantime.

If that’s correct, what does greenlet actually bring to the table here? As I understand it, we only have two greenlets: the main one and a worker. The main greenlet runs a while loop, switching to the worker greenlet to obtain a new awaitable.

From my (admittedly limited) understanding, to support async hooks in PluggyManager, it seems we only need to explicitly handle the awaitable objects returned by the hooks. For example:

async def run_async2(self, func: Callable[[], _T]) -> _T: results = func() for index, result in enumerate(results): if inspect.isawaitable(result): result = await result results[index] = result return results

If we replace pm.run_async with pm.run_async2 in test_async.py, all the tests still pass. What am I missing here?

Greenlet allows to switch stacks

Which allows to switch from 10 sync calls in back to a async function that can await a awaitable

The run function will pass into asyncio trio or anyio. Then it will switch back into the sync function

And now any sync call below can send a sync call to the outer async function

Thanks, @RonnyPfannschmidt, but it’s still not clear to me how greenlet fits into this.

In other words, what does run_async do that run_async2 does not? From my understanding, they seem functionally equivalent—is that correct?

If not, we should create a test case that only works with run_async but fails with run_async2. That would help clarify why greenlet is necessary here.

RonnyPfannschmidt changed the title ~~Add CLAUDE.md with development guidance~~ try claude - add a basic config and play around with some quality of life changes i was offputting Jun 19, 2025

RonnyPfannschmidt marked this pull request as draft June 19, 2025 19:25

Pierre-Sassoulas previously approved these changes Jun 19, 2025

View reviewed changes

RonnyPfannschmidt force-pushed the try-claude branch 2 times, most recently from 9f4ce17 to d1815d2 Compare June 20, 2025 12:32

RonnyPfannschmidt commented Jun 20, 2025

View reviewed changes

nicoddemus reviewed Jun 22, 2025

View reviewed changes

RonnyPfannschmidt force-pushed the try-claude branch 4 times, most recently from 99be2a2 to f93156a Compare June 24, 2025 16:52

RonnyPfannschmidt mentioned this pull request Jun 25, 2025

evolve hook tracing by providing the concept of hook exec/call tracers #588

Open

RonnyPfannschmidt force-pushed the try-claude branch 4 times, most recently from b9bec1d to 978a61b Compare June 25, 2025 15:38

RonnyPfannschmidt force-pushed the try-claude branch from 844dffb to e8ce9f7 Compare June 25, 2025 20:28

RonnyPfannschmidt and others added 7 commits June 25, 2025 22:30

Add CLAUDE.md with development guidance

5a292b3

also add uv based tooling to ease things Co-Authored-By: Claude <noreply@anthropic.com> - add ruff/mypy - allow claude to run some commands directly

RonnyPfannschmidt and others added 19 commits June 25, 2025 22:30

add utilities to deal with async in pluggy

a4772bb

FIXUP: rebase artifacts

cb748f6

fixup: older python support

6570d43

RonnyPfannschmidt force-pushed the try-claude branch from e8ce9f7 to 6570d43 Compare June 25, 2025 20:31

RonnyPfannschmidt commented Jun 27, 2025

View reviewed changes

nicoddemus reviewed Jun 28, 2025

View reviewed changes

try claude - add a basic config and play around with some quality of life changes i was offputting #584

Are you sure you want to change the base?

try claude - add a basic config and play around with some quality of life changes i was offputting #584

Conversation

RonnyPfannschmidt commented Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RonnyPfannschmidt commented Jun 20, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nicoddemus left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

RonnyPfannschmidt commented Jun 23, 2025

Uh oh!

RonnyPfannschmidt commented Jun 23, 2025

Uh oh!

RonnyPfannschmidt commented Jun 25, 2025

Uh oh!

RonnyPfannschmidt commented Jun 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nicoddemus Jun 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

RonnyPfannschmidt commented Jun 19, 2025 •

edited

Loading

nicoddemus Jun 28, 2025 •

edited

Loading