From 74c0649bf6b812c9ca350dbf57e2474148904517 Mon Sep 17 00:00:00 2001 From: Pavel Kutsenko Date: Sun, 23 Aug 2026 02:47:02 +0300 Subject: [PATCH 01/12] perf: stop packing a dict and re-looking-up the registry on the hot path provider() checked its three flags by looping over a **kwargs dict, so every call packed a dict to catch a mistake almost no call makes. Spelling the three tests out takes about 135 ns off entering a block, which was a seventh of what a block cost. use() and _Provider.__enter__ looked _registry.get up on the module every time, which the @inject wrapper already stopped doing by binding it at decoration, and binding it once here too takes about five percent off a read. Measured interleaved against the parent revision, alternating rounds, with the value-passed-in row and bare ContextVar.get() flat as controls. use() 60.5 to 57.9 ns, use(Config) alone 44.7 to 42.3, enter and exit 1002 to 830, the same with providers already open 1025 to 871, extend=True 1656 to 1491, sealed=True 2524 to 2378. Both spellings look arbitrary from the code, so the argument for each is on the design page rather than only in a comment. The benchmark table is left for the change that follows, so the numbers are regenerated once rather than twice. --- docs/content/misc/design.rst | 10 ++++++++++ src/nodrill/_core.py | 35 +++++++++++++++++++++++------------ 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/docs/content/misc/design.rst b/docs/content/misc/design.rst index e4dd4ee..e22cfb7 100644 --- a/docs/content/misc/design.rst +++ b/docs/content/misc/design.rst @@ -62,6 +62,16 @@ The :func:`~nodrill.declare` catalogue is a second table of exactly that kind, w Both are configuration rather than flowing state, and state lives only in ContextVars, with the further exceptions argued for below. The suspicious-fallback counter beside the catalogue is instrumentation under the ledger's rules, written on the fallback path and reported by :func:`~nodrill.explain`. +Two spellings on the hot path that look arbitrary +------------------------------------------------- + +``provider()`` checks its three flags one at a time, spelled out, rather than looping over ``**flags``. +The loop packed a dictionary on every call to catch a mistake almost no call makes, and it was about a seventh of the cost of entering a block. +The message lives once, in a helper that builds the error rather than raising it, so each of the three tests is still one line. + +``use()`` and a provider's ``__enter__`` read the registry through a module-level name bound once at import rather than through the :class:`~contextvars.ContextVar`'s attribute. +The compiled :func:`~nodrill.inject` wrapper already did exactly this at decoration, and doing it here too is worth about five percent of a read. + The ambient context object -------------------------- diff --git a/src/nodrill/_core.py b/src/nodrill/_core.py index 3ff33cf..66f7a16 100644 --- a/src/nodrill/_core.py +++ b/src/nodrill/_core.py @@ -35,6 +35,8 @@ _registry: ContextVar[dict[str | type[Any], Any]] = ContextVar( "nodrill_registry", default=_EMPTY_REGISTRY ) +# Bound once for the two hot readers, the way the @inject wrapper already binds it at decoration. +_registry_get = _registry.get # Configuration rather than per-context state, so deliberately not a ContextVar. _defaults: dict[type[Any], Callable[[], Any]] = {} @@ -173,7 +175,7 @@ def __enter__(self) -> T: # A fresh scope per entry, which is what stops a re-entry reviving the last one. self._scope = scope = _Scope(self._key, _user_site()[0]) value, public = _sealed_views(value, public, scope) - enclosing = _registry.get() + enclosing = _registry_get() updated = dict(enclosing) updated[self._key] = public if _debug_state.recording: @@ -415,20 +417,29 @@ class _SealedExtendingProvider(_Sealing, _ExtendingProvider): __slots__ = () -def _refuse_data_flags(**flags: Any) -> None: +def _data_flag_error(name: str, value: Any) -> TypeError: + """Build the error for a flag handed data, naming the namespace spelling that wanted it.""" + return TypeError( + f"provider({name}=...) is a flag and cannot carry data, and " + f"{value!r} would turn it on as well as vanish. For a namespace " + f"attribute of that name write " + f"provider(Namespace({name}={value!r}, ...), key=)" + ) + + +def _refuse_data_flags(frozen: Any, extend: Any, sealed: Any) -> None: """Refuse a flag carrying data, which would otherwise eat a namespace attribute. provider("plan", extend="v1") reads as an attribute and binds the parameter, so the value disappears and the feature turns itself on. """ - for name, value in flags.items(): - if value is not True and value is not False: - raise TypeError( - f"provider({name}=...) is a flag and cannot carry data, and " - f"{value!r} would turn it on as well as vanish. For a namespace " - f"attribute of that name write " - f"provider(Namespace({name}={value!r}, ...), key=)" - ) + # Spelled out rather than looped over **flags, which packed a dict on every provider() call. + if frozen is not True and frozen is not False: + raise _data_flag_error("frozen", frozen) + if extend is not True and extend is not False: + raise _data_flag_error("extend", extend) + if sealed is not True and sealed is not False: + raise _data_flag_error("sealed", sealed) @overload @@ -490,7 +501,7 @@ def provider( once the block has exited, so a value captured by a closure or a background task reports the escape where it happens. """ - _refuse_data_flags(frozen=frozen, extend=extend, sealed=sealed) + _refuse_data_flags(frozen, extend, sealed) target = _target_of(args, values) if isinstance(target, str): if key is not None: @@ -604,7 +615,7 @@ def use(key: Any, *, default: Any = _MISSING) -> Any: instance typed as that class. A miss tries a set_default() factory, then the default argument, then raises NoProviderError. """ - registry = _registry.get() + registry = _registry_get() try: return registry[key] except KeyError: From 6e877e2bdf689b58e2e880d9d7591c653f02f895 Mon Sep 17 00:00:00 2001 From: Pavel Kutsenko Date: Sun, 23 Aug 2026 02:48:06 +0300 Subject: [PATCH 02/12] feat: record what each entry point reads, with python -m nodrill contract A parameter is visible in a signature and a context lookup is not, so whether a handler can miss in production is a question answered today by deploying. Run a suite with NODRILL_CONTRACT set to a directory and every read is recorded against the entry point it happened under, then python -m nodrill contract renders the record into a file a pull request reviews. A key a handler starts reading is one added line in that diff, and a set_default quietly taking over a boundary is one changed line, which is the case the whole thing exists for. The instrumentation is the ledger and not a second mechanism. Read counting already installs a dict subclass as the registry rather than branching in use(), and the recorder rides the same subclass, so the hot path gains no line and an off run pays nothing, measured interleaved at under two percent on every reference row with the controls moving the same amount. It is also the only mechanism that sees a compiled @inject wrapper, which binds its registry accessor at decoration where nothing patched later reaches it. That wrapper reads with a subscript in a try rather than get() with a sentinel, which is both faster and, since a read through get is deliberately not recorded, the reason injected reads appear in a contract at all. The change belongs here rather than with the speed work it also happens to be. An entry point is the key of a block with nothing open above it, or of one NODRILL_CONTRACT_ENTRY names. The first rule alone does not survive an ordinary application, where anything opened above the boundaries becomes the entry point for all of them, so naming the boundaries is what makes the first column mean anything. It is a variable rather than a sixth provider() keyword because a keyword on a released function cannot be taken back and a variable can. Outermost is read off the chain of open blocks rather than off the kind of mapping a block inherited, since a block closing out of order leaves a repaired mapping that outlives its chain and would hand its dead label to whatever opened next. The recorder sits above the defaults probe in _resolve_miss rather than on the raise, since a set_default factory and a use(key, default=...) both return before anything reports a miss, and that miss is the one worth surfacing. _resolve_miss now raises from None, because the wrapper calls it inside its own except KeyError and a caller must not be able to tell. The file is three tab-separated fields, sorted, and carries no file names and no line numbers. A tab rather than padding, since one long key would rewrite every line, and rather than two spaces, since repr escapes a tab and a newline but not a space and a key may hold two in a row. The verb carries the whole answer, requires or set_default or default, so every line is one shape. Not TOML, since tomllib is 3.11 and the floor here is 3.10, and not JSON, since a nested object re-indents and this file exists to be read in a diff. The switch is an environment variable read once at import, because a child interpreter inherits one. A pool worker needs one thing more, since multiprocessing exits through os._exit, which runs finalizers and never atexit, so the dump is registered both ways and made idempotent. Every process of a run shares a run id, so a directory reused later yields the newer contract rather than the union of both. No new public name and no console script. python -m nodrill is what pip and json.tool are spelled as, pyproject.toml still declares no scripts, and coverage stays at 100 with no omit and no pragma. Recording this repository's own suite gives 85 facts under 63 entry points, byte for byte identical across three pytest-randomly seeds. --- README.md | 3 + docs/content/howto/index.rst | 1 + .../howto/record-what-a-handler-reads.rst | 178 ++++++ docs/content/misc/design.rst | 43 ++ docs/content/misc/faq.rst | 2 + docs/content/misc/performance.rst | 28 +- docs/content/misc/scope.rst | 3 +- docs/content/ref/debugging.rst | 24 + src/nodrill/__main__.py | 11 + src/nodrill/_audit.py | 204 +++++++ src/nodrill/_core.py | 35 +- src/nodrill/_debug.py | 134 +++- src/nodrill/_errors.py | 12 + src/nodrill/_inject.py | 6 +- tests/__init__.py | 1 + tests/audit_app/__init__.py | 7 + tests/audit_app/app.py | 74 +++ tests/test_audit.py | 575 ++++++++++++++++++ tests/test_debug.py | 10 + tests/test_inject_codegen.py | 25 +- 20 files changed, 1311 insertions(+), 65 deletions(-) create mode 100644 docs/content/howto/record-what-a-handler-reads.rst create mode 100644 src/nodrill/__main__.py create mode 100644 src/nodrill/_audit.py create mode 100644 tests/__init__.py create mode 100644 tests/audit_app/__init__.py create mode 100644 tests/audit_app/app.py create mode 100644 tests/test_audit.py diff --git a/README.md b/README.md index 3e5862c..d2818ba 100644 --- a/README.md +++ b/README.md @@ -62,6 +62,9 @@ When it goes wrong, `debug()` turns a miss into a diagnosis naming the thread, t `isolate()` gives a test fresh context state and rolls it back afterwards. Everything above is importable from the top-level package, and there is nothing else to import. +Before it goes wrong, running a suite under `NODRILL_CONTRACT` records which keys each entry point actually read and which of those a fallback answered rather than a provider, and `python -m nodrill contract` renders that into a file a pull request reviews. +It is a command rather than an import, so it adds no name to the package and no script to your PATH. + ## Overhead A lookup is one dict read on a single `ContextVar`, and nothing is constructed, resolved or cached along the way. diff --git a/docs/content/howto/index.rst b/docs/content/howto/index.rst index 282259a..ac129ec 100644 --- a/docs/content/howto/index.rst +++ b/docs/content/howto/index.rst @@ -20,6 +20,7 @@ Each one is a complete program you can paste into a file and run, with the reaso test-injected-code refer-to-a-key-you-cannot-import find-out-why-the-context-is-missing + record-what-a-handler-reads see-the-context-in-a-traceback add-context-to-every-log-record replace-a-contextvar diff --git a/docs/content/howto/record-what-a-handler-reads.rst b/docs/content/howto/record-what-a-handler-reads.rst new file mode 100644 index 0000000..0ba19b0 --- /dev/null +++ b/docs/content/howto/record-what-a-handler-reads.rst @@ -0,0 +1,178 @@ +.. _howto-record-what-a-handler-reads: + +Record what a handler reads +=========================== + +A parameter is visible in a signature and a context lookup is not, so the question a reviewer actually asks, which is whether this handler can miss in production, is answered today by deploying. + +Recording answers it from a test run instead. +Run the suite with ``NODRILL_CONTRACT`` pointing at a directory, render what it recorded into a file, and commit the file. +From then on every pull request that changes what a handler reads changes one line of that file, in the diff, where somebody can see it. + +.. code-block:: python + :caption: app.py + + from collections.abc import Iterator + from contextlib import contextmanager + + from nodrill import provider, set_default, use + + + class Settings: + def __init__(self, dsn: str = "sqlite://") -> None: + self.dsn = dsn + + + class User: + def __init__(self, name: str = "anonymous") -> None: + self.name = name + + + class Origin: + def __init__(self, label: str = "system") -> None: + self.label = label + + + set_default(Origin, Origin) + + + def record_write() -> str: + return f"{use(User).name} from {use(Origin).label} on {use(Settings).dsn}" + + + @contextmanager + def running() -> Iterator[None]: + """The process-wide layer, opened once the way a main function does.""" + with provider(Settings()): + yield + + + def serve_http(name: str) -> str: + """The web entry point, which opens both keys the handler reads.""" + with provider("http request"), provider(User(name)), provider(Origin("http")): + return record_write() + + + def run_job(name: str) -> str: + """The queue entry point, which opens the user and leaves the origin to fall back.""" + with provider("celery worker"), provider(User(name)): + return record_write() + +Record a run, then render it. +Anything that exercises the entry points will do, and a test suite is the usual one. + +.. code-block:: console + + $ NODRILL_CONTRACT=.nodrill python -c " + import app + with app.running(): + app.serve_http('ada'); app.run_job('grace')" + $ python -m nodrill contract --from .nodrill + nodrill: 4 facts under 1 entry point, recorded from 1 process. A contract is only as complete as the run that recorded it. + +.. code-block:: text + + # nodrill contract 1 + app:Settings requires app:Origin + app:Settings requires app:Settings + app:Settings requires app:User + app:Settings set_default app:Origin + +Four facts under one entry point, and the entry point is the configuration this process opened in ``running()``. +That is correct and it is useless. + +Name the boundaries +------------------- + +An entry point is the outermost provider block open above a read. +An application that opens configuration, a database handle or a settings object above its server loop makes that block the entry point for everything underneath, and the first column stops distinguishing anything. + +``NODRILL_CONTRACT_ENTRY`` names the blocks that are boundaries, as rendered keys separated by commas, so a block whose key is in that list mints its own entry point even when something is open above it. + +.. code-block:: console + + $ NODRILL_CONTRACT=.nodrill NODRILL_CONTRACT_ENTRY="'http request','celery worker'" python -c " + import app + with app.running(): + app.serve_http('ada'); app.run_job('grace')" + $ python -m nodrill contract --from .nodrill + nodrill: 6 facts under 2 entry points, recorded from 1 process. A contract is only as complete as the run that recorded it. + +.. code-block:: text + + # nodrill contract 1 + 'celery worker' requires app:Settings + 'celery worker' requires app:User + 'celery worker' set_default app:Origin + 'http request' requires app:Origin + 'http request' requires app:Settings + 'http request' requires app:User + +Now the file says something. +The two boundaries read the same three keys, except that the queue never opens `Origin`, so a :func:`~nodrill.set_default` factory answers for it and every row it writes is labelled `system`. +That is a bug the code cannot show you and no test fails on, and it is one line of a diff. + +Pass the same value to the command, so it can tell you about a boundary you named that no block opened, which is what a renamed key looks like. + +Reading the file +---------------- + +Three tab-separated fields, sorted, one fact per line. +The first is the entry point, the second is how the read was answered, the third is the key, rendered the way :func:`~nodrill.ref` spells one so two same-named classes in different modules stay apart. +A string key keeps the quotes Python puts on it, which is also what keeps a key holding a tab or a newline from becoming two lines. + +The second field is the one to read. + +`requires` + A provider answered, which is the ordinary case. + +`set_default` + No provider was open and a :func:`~nodrill.set_default` factory answered instead. + Every one of these is a boundary that does not open a key somebody registered a fallback for. + +`default` + No provider was open and the ``use(key, default=...)`` at the call site answered. + +An entry point of `(none)` means no provider block was open at all, which a read can only survive by falling back. +It is what an unwrapped worker thread looks like, and what a read at import time looks like. + +Wiring it into CI +----------------- + +Two steps, recording and reviewing. + +.. code-block:: yaml + :caption: .github/workflows/ci.yml + + - run: NODRILL_CONTRACT=.nodrill pytest + env: + NODRILL_CONTRACT_ENTRY: "'http request','celery worker'" + - run: python -m nodrill contract --from .nodrill --write nodrill.contract + - run: git diff --exit-code nodrill.contract + +Recording is off unless ``NODRILL_CONTRACT`` is set, and the variable is read once when `nodrill` is imported, which is also why a subprocess your suite spawns records too. +Each process writes its own file into the directory and the command merges them, so a suite under `xdist`, one that shells out, or one using a :class:`~concurrent.futures.ProcessPoolExecutor` needs nothing extra. +A directory reused by a later run is not a problem either, since every process of one run shares a run id and the command reads the newest run and says how many older files it left out. + +What the contract is worth +-------------------------- + +Exactly as much as the run that recorded it. + +A contract lists what the run observed and nothing else, so a key only one untested branch reads is a key the file does not mention. +The summary line says how many facts under how many entry points the conclusion rests on, and it says it every time rather than only when the number is small, because a guarantee that overstates itself is worse than no guarantee. + +Three limits are worth knowing before you rely on it. + +The entry point is a key, so two boundaries that open the same one are one row set, and a boundary you have not named is whatever is open above it. + +A read that raises :exc:`~nodrill.NoProviderError` is not recorded, because it is already loud. +The file is about what an entry point needs and gets, and a miss that reaches a traceback needs no file to be noticed. + +A value read through :func:`~nodrill.inject` is recorded exactly like one read through :func:`~nodrill.use`, but a value a handler receives as an ordinary argument is not context and never appears. +The file describes the context a boundary depends on, which is the part of its input that no signature shows. + +.. rubric:: See also + +- :doc:`find-out-why-the-context-is-missing` for a miss that is happening now rather than one that might. +- :doc:`/content/topics/declaring` for naming, in the code, which boundary was meant to provide a key. diff --git a/docs/content/misc/design.rst b/docs/content/misc/design.rst index e22cfb7..67aaa98 100644 --- a/docs/content/misc/design.rst +++ b/docs/content/misc/design.rst @@ -463,8 +463,51 @@ That base is also how ``_lazy`` sees through a view to its target when it checks ``_debug`` is instrumentation rather than registry, and sits beside ``_core`` because nothing on a successful lookup reads it. ``_report`` is reporting rather than registry, sharing with ``_core`` only the value a provider is holding, and nothing in it runs until an exception is already leaving a block. ``_inject``, ``_concurrency`` and ``_errors`` are the remaining features. +``_audit`` is the contract tool, which reads the table the ledger fills and is imported by nothing in the core, so a process that never audits never loads it. +``__main__`` is two lines of dispatch under it, which is the whole command line surface. Nothing under ``nodrill._*`` is public. + +The contract recorder +--------------------- + +Recording what each entry point reads is the second reader of a trick the ledger already uses. +Read counting installs a ``dict`` subclass as the registry rather than branching in ``use()``, and the recorder rides the same subclass, so a lookup pays nothing for either feature when neither is on. + +That choice was forced rather than preferred. +A compiled :func:`~nodrill.inject` wrapper binds its registry accessor at decoration, so patching a module afterwards does not reach it, and swapping ``use`` or installing a pytest plugin would record every plain lookup and none of the injected ones. +Emitting a recording branch from the codegen instead would make an enabled and a disabled wrapper two different compiled artefacts, which is the one thing the wrapper's design does not allow. +The wrapper's registry read is a subscript inside a ``try`` for the same reason it is fast, and that spelling is now load-bearing twice over, since a read through ``get`` is deliberately not recorded. + +An entry point is the key of a provider block with no block open above it, or of one that ``NODRILL_CONTRACT_ENTRY`` names. +The first rule alone was the original design and it does not survive contact with an ordinary application. +Anything opened above the boundaries becomes the entry point for everything beneath it, so a service that opens configuration or a database handle in its main function gets one entry point and a file that distinguishes nothing. +Naming the boundaries is therefore not a refinement, it is what makes the first column mean anything, and it is a variable rather than a ``provider()`` keyword because a sixth reserved name on a released function cannot be taken back and a variable can. + +Outermost is read off the chain of open blocks, not off the kind of mapping a block inherited. +The difference matters because a block closing out of order leaves a repaired mapping that outlives its own chain, and asking "was the mapping I inherited an instrumented one" would hand that dead mapping's label to the next boundary that opened. +The chain is already computed one line further down in the same method, so the correct rule is also the cheaper one. + +The label travels on the registry, which is what makes it survive a task, a wrapped thread and a repair, and the repair carries it for the same reason it carries the counting table. + +A consumer read is a subscript. +:func:`~nodrill.use` and the compiled wrapper both read the registry with ``[]``, and everything the library does to a registry for its own reasons, the open chain, an ``extend=True`` merge and the out-of-order repair, reads it with ``get``, so the recorder can tell a read that a user wrote from a read that the library did without being told which is which. +The instrumented registry also sees what a caller passed rather than what the registry stores, so a :func:`~nodrill.ref` arrives unresolved and is resolved before it is recorded. + +The recorder sits above the defaults probe in the miss path rather than on the raise. +A :func:`~nodrill.set_default` factory and a ``use(key, default=...)`` both return before anything reports a miss, so a lookup that a registration is quietly answering is invisible to anything watching for the error, and that lookup is the one the whole feature exists to surface. +It is also why a miss the registrations do not answer is left out of the file entirely, since an exception reaching a traceback needs no artefact to be noticed. + +The file is three tab-separated fields, sorted, and carries no file names and no line numbers. +A site moves whenever anything above it moves, so a contract carrying sites churns on every pull request and stops being read, and sites belong in a failure message where the audience is different. +A tab rather than aligned columns, because padding means one long key rewrites every line, and rather than two spaces, because ``repr`` escapes a tab and a newline but not a space, so a key holding two spaces in a row would otherwise split into more fields than the format has. +The verb carries the whole answer, ``requires`` or ``set_default`` or ``default``, rather than a fourth column, so every line is the same shape and a reviewer greps for what is not ``requires``. + +The switch is an environment variable read once at import, because a child interpreter inherits one. +That is what makes a suite that spawns subprocesses, runs under ``xdist`` or uses a process pool record without a special case for any of them. +A pool worker needs one more thing, since :mod:`multiprocessing` exits a worker through :func:`os._exit`, which runs finalizers and never :mod:`atexit`, so the dump is registered both ways and made idempotent rather than registered once and lost. +Each process of a run shares a run id minted at arming and written back into the environment, so a directory reused by a later run yields the newer contract rather than the union of both. + isolate() --------- diff --git a/docs/content/misc/faq.rst b/docs/content/misc/faq.rst index 507ca08..6a3a1ee 100644 --- a/docs/content/misc/faq.rst +++ b/docs/content/misc/faq.rst @@ -13,6 +13,8 @@ The criticism the anti-pattern label points at is real, though, because a depend That is why ``@inject`` exists, and why explicit arguments always win. Where visibility matters, put the dependency in the signature and let the decorator fill it. +For what a whole boundary reads rather than what one function does, :doc:`/content/howto/record-what-a-handler-reads` records it from a test run into a file a pull request reviews, which answers the same objection at the scale a reviewer asks it. + Does it work with FastAPI, Django, Flask, Celery? ------------------------------------------------- diff --git a/docs/content/misc/performance.rst b/docs/content/misc/performance.rst index 5a749c7..7853e8d 100644 --- a/docs/content/misc/performance.rst +++ b/docs/content/misc/performance.rst @@ -19,23 +19,23 @@ The first rows are one function doing one read, reached six ways, so they can be operation ns × ================================================================ ==== === one read in a function, value passed in as a parameter 23 1.0 -the same read through `use()` 61 2.7 -the same read through `@inject` 71 3.1 -the same read through a `frozen=True` provider 117 5.1 -the same read through a `sealed=True` provider 124 5.5 -the same read through a resolved `lazy` provider 138 6.0 -`use(Config)` on its own, without the call frame 44 2.0 -the same lookup through a `ref()` key 149 6.6 +the same read through `use()` 60 2.6 +the same read through `@inject` 61 2.6 +the same read through a `frozen=True` provider 114 4.9 +the same read through a `sealed=True` provider 121 5.2 +the same read through a resolved `lazy` provider 137 5.8 +`use(Config)` on its own, without the call frame 42 1.8 +the same lookup through a `ref()` key 144 6.2 bare `ContextVar.get()`, for reference 16 0.7 -`with provider(...)`, enter and exit 1033 45 -the same with 8 providers already open 1070 47 -`with provider(..., sealed=True)`, entered and exited 2599 114 -`with provider(lazy(...))`, entered and exited unread 2003 88 -`with provider(..., extend=True)`, over an 8-attribute namespace 2144 94 -`wrap(fn)()`, per call into a thread 546 24 +`with provider(...)`, enter and exit 868 37 +the same with 8 providers already open 904 39 +`with provider(..., sealed=True)`, entered and exited 2386 102 +`with provider(lazy(...))`, entered and exited unread 1799 77 +`with provider(..., extend=True)`, over an 8-attribute namespace 1984 85 +`wrap(fn)()`, per call into a thread 539 23 ================================================================ ==== === -CPython 3.14.5 on macOS 26.6, arm64, measured 2026-08-20. +CPython 3.14.5 on macOS 26.6, arm64, measured 2026-08-23. .. end benchmarks diff --git a/docs/content/misc/scope.rst b/docs/content/misc/scope.rst index 61b1808..b3e3a51 100644 --- a/docs/content/misc/scope.rst +++ b/docs/content/misc/scope.rst @@ -43,10 +43,11 @@ What is not in this release No framework integrations. No middleware, no plugins, no ASGI helpers. +No pytest plugin either, which is why the contract recorder in :doc:`/content/howto/record-what-a-handler-reads` is a command armed by an environment variable, so a test runner this library has never heard of records the same way. No global singletons. All state that flows with a call lives in :mod:`contextvars`. -The exceptions are configuration and instrumentation, the ``set_default`` and :func:`~nodrill.declare` tables written at import time, the ledger :func:`~nodrill.debug` keeps while it is on with the fallback counter beside the declare catalogue, and the two switches :func:`~nodrill.annotate_exceptions` and :func:`~nodrill.set_codec` set at startup. +The exceptions are configuration and instrumentation, the ``set_default`` and :func:`~nodrill.declare` tables written at import time, the ledger :func:`~nodrill.debug` keeps while it is on with the fallback counter beside the declare catalogue, the contract table the recorder fills while ``NODRILL_CONTRACT`` is set, and the two switches :func:`~nodrill.annotate_exceptions` and :func:`~nodrill.set_codec` set at startup. No lifecycle management. Nothing is constructed, cached, pooled, or closed on your behalf. diff --git a/docs/content/ref/debugging.rst b/docs/content/ref/debugging.rst index ce7e9c1..4ead23c 100644 --- a/docs/content/ref/debugging.rst +++ b/docs/content/ref/debugging.rst @@ -53,6 +53,30 @@ debug NODRILL_DEBUG=1 python -m myapp +.. _ref-contract: + +Recording a contract +-------------------- + +``NODRILL_CONTRACT`` names a directory and turns on recording of what each entry point reads. +It is read once, at import, like ``NODRILL_DEBUG``, and any non-empty value is a directory rather than a switch, so ``0`` names a directory called ``0``. +Every process of a run writes its own file there, including one a suite spawns, and the files are merged when the contract is rendered. + +``NODRILL_CONTRACT_ENTRY`` names the provider keys that are boundaries, as rendered keys separated by commas, so ``"'http request',myapp.web:Request"``. +A block whose key is named mints its own entry point even when a block is open above it. +Without it the entry point is whatever block is outermost, which in an application that opens configuration above its server loop is that configuration. + +.. code-block:: bash + + NODRILL_CONTRACT=.nodrill NODRILL_CONTRACT_ENTRY="'http request'" pytest + python -m nodrill contract --from .nodrill --write nodrill.contract + +``python -m nodrill contract`` renders what a run recorded. +``--from`` names the directory and is required, ``--write`` names the file and defaults to standard output, and the summary of what the contract rests on always goes to standard error so the artefact can be piped. +It returns ``0`` when it rendered a contract and ``1`` when it could not, leaving ``2`` to mean the command line itself was wrong. + +:doc:`/content/howto/record-what-a-handler-reads` is the task-shaped version, with the file format and what it is worth. + With ``unused=True``, a provider nothing read warns as its block exits, pointing at the ``with`` statement that opened it. .. code-block:: text diff --git a/src/nodrill/__main__.py b/src/nodrill/__main__.py new file mode 100644 index 0000000..d650ac1 --- /dev/null +++ b/src/nodrill/__main__.py @@ -0,0 +1,11 @@ +"""Dispatch for python -m nodrill, which is the whole command line surface. + +Not a console script, so nothing lands on a PATH and the package still +declares none, which leaves adding one later possible and removing one never +necessary. No __name__ guard, since a __main__ module is only ever run as +one and a guard would be a branch nothing can take the other way. +""" + +from ._audit import main + +raise SystemExit(main()) diff --git a/src/nodrill/_audit.py b/src/nodrill/_audit.py new file mode 100644 index 0000000..cf490e1 --- /dev/null +++ b/src/nodrill/_audit.py @@ -0,0 +1,204 @@ +"""The contract a run records, and the file a pull request reviews. + +A parameter is visible in a signature and a context lookup is not, so +whether a handler can miss in production is a question answered today by +deploying. Recording what each entry point actually read, and reviewing the +diff of that record, is how snapshot testing already answers the same +question about the same class of problem. + +Recording is armed by NODRILL_CONTRACT and is off otherwise, and the ledger +does the observing, so nothing here is reachable from a lookup. What a +contract says is only ever as true as the run that recorded it, which is a +limit the output states rather than one the reader has to infer. +""" + +from __future__ import annotations + +import argparse +import os +import sys +import time +import uuid +from pathlib import Path + +_Reads = set[tuple[str, str, str]] + +# The one format, carried by a shard and by a contract alike, so there is one reader. +_HEADER = "# nodrill contract 1" +_SUFFIX = ".shard" +# A tab, because repr escapes one and a key may legally hold two spaces in a row. +_GAP = "\t" +# Written and diffed on machines nobody here chose, so the encoding is named rather than guessed. +_ENCODING = "utf-8" +# The verbs, in the order a reader cares about them, since anything but requires is worth a look. +_VERBS = ("requires", "set_default", "default") +# Named here rather than in _debug, so one module owns the spelling of both variables. +_ENTRY_VAR = "NODRILL_CONTRACT_ENTRY" + + +def _declared(value: str) -> frozenset[str]: + """Read NODRILL_CONTRACT_ENTRY, whose value is rendered keys separated by commas. + + A key holding a comma cannot be named this way, which is the price of a + spelling somebody types into a CI file by hand. + """ + return frozenset(entry for entry in (part.strip() for part in value.split(",")) if entry) + + +def _new_run() -> str: + """Mint an id for this run, so a directory reused tomorrow does not merge into today.""" + return f"{time.time_ns()}-{uuid.uuid4().hex}" + + +def _fact(read: tuple[str, str, str]) -> str: + """Render one recorded fact as the file's one line shape.""" + return _GAP.join(read) + + +def _render(reads: _Reads) -> str: + """Render a contract, sorted so the file is a property of the run and not of its order.""" + lines = [_HEADER, *(_fact(read) for read in sorted(reads))] + return "".join(f"{line}\n" for line in lines) + + +def _parse(text: str, source: str) -> _Reads: + """Read a contract or a shard back, refusing a version this reader does not know.""" + lines = text.splitlines() + if not lines or lines[0] != _HEADER: + opening = lines[0] if lines else "an empty file" + raise ValueError( + f"{source} is not a nodrill contract this version reads. " + f"Expected {_HEADER!r} on the first line and found {opening!r}" + ) + found: _Reads = set() + for line in lines[1:]: + entry, verb, key = line.split(_GAP) + found.add((entry, verb, key)) + return found + + +def _dump(directory: str, run: str, reads: _Reads) -> None: + """Write this process's records into its own shard of the run, then forget them. + + Forgetting is what makes a second call a no-op, which matters because a + multiprocessing worker is finalized as well as registered at exit. + """ + if not reads: + return + target = Path(directory) + target.mkdir(parents=True, exist_ok=True) + # The run first so a merge can group by it, then pid and a token, since a pid is reused. + shard = target / f"{run}-{uuid.uuid4().hex}{_SUFFIX}" + shard.write_text(_render(reads), encoding=_ENCODING) + reads.clear() + + +def _merge(directory: str) -> tuple[_Reads, int, int]: + """Read the newest run in a directory, and say how many shards it left behind. + + A directory reused across runs holds both, and a contract built from + yesterday's reads describes a program that no longer exists. + """ + shards = sorted(Path(directory).glob(f"*{_SUFFIX}")) + if not shards: + return set(), 0, 0 + newest = max(shard.name.rpartition("-")[0] for shard in shards) + current = [shard for shard in shards if shard.name.startswith(f"{newest}-")] + found: _Reads = set() + for shard in current: + found |= _parse(shard.read_text(encoding=_ENCODING), str(shard)) + return found, len(current), len(shards) - len(current) + + +def _counted(count: int, singular: str, plural: str) -> str: + """Render a count and its noun, since every figure below reads as a sentence.""" + return f"{count} {singular if count == 1 else plural}" + + +def _summary(reads: _Reads, shards: int, stale: int) -> str: + """Say what the contract rests on, since a guarantee that overstates itself is worse than none. + + The figures are what this stage can honestly own, which is what a run + observed rather than what a tree contains. + """ + entries = len({entry for entry, _, _ in reads}) + said = ( + f"nodrill: {_counted(len(reads), 'fact', 'facts')} under " + f"{_counted(entries, 'entry point', 'entry points')}, " + f"recorded from {_counted(shards, 'process', 'processes')}. " + f"A contract is only as complete as the run that recorded it." + ) + if stale: + said += f" {_counted(stale, 'shard', 'shards')} from an earlier run were left out." + return said + + +def _unseen(reads: _Reads, declared: frozenset[str]) -> str | None: + """Report a declared entry point no block opened, since a renamed key would go quiet.""" + missing = sorted(declared - {entry for entry, _, _ in reads}) + if not missing: + return None + return f"nodrill: no block opened {', '.join(missing)}, named by NODRILL_CONTRACT_ENTRY" + + +def _contract(source: str, target: str | None, declared: frozenset[str]) -> int: + """Render the contract a recorded run left, to a file or to stdout.""" + directory = Path(source) + if not directory.is_dir(): + sys.stderr.write(f"nodrill: nothing recorded at {source}, so there is no contract\n") + return 1 + reads, shards, stale = _merge(source) + if not shards: + sys.stderr.write( + f"nodrill: {source} holds no shards, so nothing armed the recorder. " + f"Run the suite with NODRILL_CONTRACT={source} first\n" + ) + return 1 + text = _render(reads) + if target is None: + sys.stdout.write(text) + else: + try: + Path(target).write_text(text, encoding=_ENCODING) + except OSError as error: + sys.stderr.write(f"nodrill: cannot write {target}, {error.strerror}\n") + return 1 + sys.stderr.write(f"{_summary(reads, shards, stale)}\n") + unseen = _unseen(reads, declared) + if unseen is not None: + sys.stderr.write(f"{unseen}\n") + return 0 + + +def main(argv: list[str] | None = None) -> int: + """Run one subcommand and return the code the interpreter should exit with.""" + parser = argparse.ArgumentParser( + prog="python -m nodrill", + description="Record and review what each entry point reads out of the context.", + allow_abbrev=False, + ) + commands = parser.add_subparsers(dest="command", required=True) + contract = commands.add_parser( + "contract", + help="render the contract a run recorded under NODRILL_CONTRACT", + description=( + "Run the suite with NODRILL_CONTRACT set to a directory, then render what it " + "recorded into a file a pull request can review." + ), + allow_abbrev=False, + ) + contract.add_argument( + "--from", + dest="source", + required=True, + metavar="DIR", + help="the directory NODRILL_CONTRACT named during the run", + ) + contract.add_argument( + "--write", + dest="target", + metavar="FILE", + help="the contract file to write, where the default is stdout", + ) + args = parser.parse_args(argv) + return _contract(args.source, args.target, _declared(os.environ.get(_ENTRY_VAR, ""))) diff --git a/src/nodrill/_core.py b/src/nodrill/_core.py index 66f7a16..96df460 100644 --- a/src/nodrill/_core.py +++ b/src/nodrill/_core.py @@ -15,7 +15,14 @@ from typing import TYPE_CHECKING, Any, ClassVar, Generic, TypeVar, overload from ._ambient import _ambient -from ._debug import _diagnose, _record_enter, _record_exit, _recount, _user_site +from ._debug import ( + _diagnose, + _record_enter, + _record_exit, + _record_fallback, + _reinstrument, + _user_site, +) from ._debug import _state as _debug_state from ._declare import _expected_at, _fired, _note_fallback, _pending from ._declare import _restore as _restore_declared @@ -116,12 +123,13 @@ def _repaired( for open_block in reversed(chain): entered = open_block._entered # noqa: SLF001 if open_block is not leaving and entered is not None and open_block._key == key: # noqa: SLF001 - repaired[key] = entered[key] + # Read with get, since a subscript is what the audit counts as a consumer read. + repaired[key] = entered.get(key) break else: repaired.pop(key, None) repaired[_Open] = tuple(block for block in chain if block is not leaving) - return _recount(repaired, current) + return _reinstrument(repaired, current) class _Provider(Generic[T]): @@ -178,10 +186,11 @@ def __enter__(self) -> T: enclosing = _registry_get() updated = dict(enclosing) updated[self._key] = public - if _debug_state.recording: - self._block, updated = _record_enter(self._key, enclosing, updated) + chain = enclosing.get(_Open, ()) + if _debug_state.watching: + self._block, updated = _record_enter(self._key, enclosing, updated, outermost=not chain) # After the ledger, so the chain lands on the mapping actually installed. - updated[_Open] = (*enclosing.get(_Open, ()), self) + updated[_Open] = (*chain, self) self._entered = updated self._token = _registry.set(updated) return value @@ -418,7 +427,7 @@ class _SealedExtendingProvider(_Sealing, _ExtendingProvider): def _data_flag_error(name: str, value: Any) -> TypeError: - """Build the error for a flag handed data, naming the namespace spelling that wanted it.""" + """Report a flag handed data, naming the namespace spelling that wanted it.""" return TypeError( f"provider({name}=...) is a flag and cannot carry data, and " f"{value!r} would turn it on as well as vanish. For a namespace " @@ -643,26 +652,32 @@ def _resolve_miss(key: Any, default: Any = _MISSING) -> Any: raise TypeError( f"use() received what lazy() returned, which is a target rather than a key. " f"Open it with provider(lazy({name}, factory)) and read it with use({name})" - ) + ) from None raise TypeError( f"use() expects a string name or a class, got {type(target).__name__}: {target!r}" - ) + ) from None if isinstance(target, type): factory = _defaults.get(target) if factory is not None: # A suspicious class pays the count, and a pending declaration one resolution check. if _pending or target in _fired: _note_fallback(target) + if _debug_state.auditing: + _record_fallback(_registry_get(), target, "set_default") return factory() if default is not _MISSING: + if _debug_state.auditing: + _record_fallback(_registry_get(), target, "default") return default # The resolved target, since that is what a provider registered under. recording = _debug_state.recording diagnosis = _diagnose(target) if recording else None available = [k for k in _registry.get() if k is not _Open] + # from None because the @inject wrapper calls this inside its own except KeyError, + # where use() calls it outside one, and a caller must not see that difference. raise NoProviderError( key, available, diagnosis, provided_by=_expected_at(target), offer_debug=not recording - ) + ) from None def active() -> Mapping[str | type[Any], Any]: diff --git a/src/nodrill/_debug.py b/src/nodrill/_debug.py index 8ad647f..0987dea 100644 --- a/src/nodrill/_debug.py +++ b/src/nodrill/_debug.py @@ -13,17 +13,20 @@ from __future__ import annotations +import atexit import inspect import itertools import os import threading import warnings +from collections.abc import MutableMapping from types import TracebackType from typing import Any, NamedTuple from weakref import WeakKeyDictionary from ._declare import _report_lines -from ._errors import UnusedProviderWarning, _describe_key, _Key +from ._errors import UnusedProviderWarning, _describe_key, _Key, _key_path +from ._refs import _key_target _Registry = dict[_Key, Any] @@ -83,15 +86,18 @@ class _State: recording and counting mirror the two depths rather than being read off them, since the provider path tests one of them on every block entered. + watching is recording or auditing, so that path still tests one thing. """ - __slots__ = ("counting", "depth", "recording", "seq", "unused_depth") + __slots__ = ("auditing", "counting", "depth", "recording", "seq", "unused_depth", "watching") def __init__(self) -> None: self.depth = 0 self.unused_depth = 0 self.recording = False self.counting = False + self.auditing = False + self.watching = False self.seq = 0 @@ -105,6 +111,13 @@ def __init__(self) -> None: # Keys the cap above dropped, which a miss reports as gone rather than as absent. _forgotten: dict[_Key, None] = {} +# What the audit accumulates, uncapped and never rolled back, since a run is the unit. +_reads: set[tuple[str, str, str]] = set() +# The entry point of a read no provider block encloses, which only a fallback can be. +_NO_ENTRY = "(none)" +# Keys NODRILL_CONTRACT_ENTRY names as boundaries, which mint a label even when nested. +_declared_entries: set[str] = set() + # Serials rather than id(), which the interpreter hands on as soon as a task dies. _task_serials: WeakKeyDictionary[Any, int] = WeakKeyDictionary() _next_task_serial = itertools.count(1).__next__ @@ -113,21 +126,54 @@ def __init__(self) -> None: _from_env = os.environ.get("NODRILL_DEBUG", "") not in {"", "0"} _state.depth = 1 if _from_env else 0 _state.recording = _from_env +_state.watching = _from_env + + +def _arm(environ: MutableMapping[str, str]) -> None: + """Turn the audit on from the environment, and arrange for this process to write its shard. + + A variable rather than a call, because a child interpreter inherits one + and a call would have to be made again in every process a suite spawns. + Takes the mapping rather than reading os.environ, so what it sets can be + tested without a child interpreter. + """ + directory = environ.get("NODRILL_CONTRACT", "") + if not directory: + return + _state.auditing = True + _state.watching = True + # Deferred, so a process that never audits pays for none of the tool's imports. + from multiprocessing.util import Finalize # noqa: PLC0415 + + from ._audit import _ENTRY_VAR, _declared, _dump, _new_run # noqa: PLC0415 + + _declared_entries.update(_declared(environ.get(_ENTRY_VAR, ""))) + run = environ.get("NODRILL_CONTRACT_RUN") or _new_run() + # Written back so every child joins this run rather than starting one of its own. + environ["NODRILL_CONTRACT_RUN"] = run + atexit.register(_dump, directory, run, _reads) + # A multiprocessing worker exits through os._exit, which runs finalizers and not atexit. + Finalize(None, _dump, args=(directory, run, _reads), exitpriority=0) -class _CountingRegistry(dict[_Key, Any]): - """Registry that marks which block's value a lookup read. +_arm(os.environ) - Installed only while debug(unused=True) is on, which is what keeps read - counting out of use() itself. owners maps a key to the block providing - it, so a read credits that block and not every block sharing the key. + +class _InstrumentedRegistry(dict[_Key, Any]): + """Registry that watches lookups, for read counting and for the audit. + + Installed instead of branching in use(), which is what keeps both + features out of the hot path when neither is on. owners maps a key to + the block providing it, so a read credits that block and not every block + sharing the key, and entry names the outermost block open above it. """ - __slots__ = ("owners",) + __slots__ = ("entry", "owners") - def __init__(self, registry: _Registry, owners: dict[_Key, _Reads]) -> None: + def __init__(self, registry: _Registry, owners: dict[_Key, _Reads], entry: str) -> None: super().__init__(registry) self.owners = owners + self.entry = entry def _mark(self, key: _Key) -> None: """Note that something read the block providing key.""" @@ -135,13 +181,17 @@ def _mark(self, key: _Key) -> None: if reads is not None: reads.hit = True - def __getitem__(self, key: _Key) -> Any: + def __getitem__(self, key: Any) -> Any: + # Typed loosely because this sees what a caller passed, not what the registry stores. value = super().__getitem__(key) self._mark(key) + # A consumer read is a subscript, which is what leaves the chain key and a merge out. + if _state.auditing: + _reads.add((self.entry, "requires", _key_path(_key_target(key)))) return value - def get(self, key: _Key, default: Any = None) -> Any: - """Return the value for key, marking the read, the way @inject reads it.""" + def get(self, key: Any, default: Any = None) -> Any: + """Return the value for key, marking the read, the way an extending layer reads it.""" value = super().get(key, _MISS) if value is _MISS: return default @@ -149,13 +199,24 @@ def get(self, key: _Key, default: Any = None) -> Any: return value -def _recount(registry: _Registry, replaced: _Registry) -> _Registry: - """Return registry as a counting one when the mapping it replaces was counting.""" - if isinstance(replaced, _CountingRegistry): - return _CountingRegistry(registry, replaced.owners) +def _reinstrument(registry: _Registry, replaced: _Registry) -> _Registry: + """Return registry instrumented the way the mapping it replaces was.""" + if isinstance(replaced, _InstrumentedRegistry): + return _InstrumentedRegistry(registry, replaced.owners, replaced.entry) return registry +def _record_fallback(registry: _Registry, key: _Key, source: str) -> None: + """Note a miss a registration answered, which is the read a raise would never report. + + A set_default factory and a use(key, default=...) both return before + anything reports a miss, so a NoProviderError a registration is hiding + would otherwise never appear in a contract. + """ + entry = registry.entry if isinstance(registry, _InstrumentedRegistry) else _NO_ENTRY + _reads.add((entry, source, _key_path(key))) + + def _user_site() -> tuple[_Site, int]: """Return the innermost site outside this package, and how far up it is. @@ -197,26 +258,41 @@ def _where() -> _Where: return _Where(ident, name, serial, task.get_name()) -def _record_enter(key: _Key, enclosing: _Registry, registry: _Registry) -> tuple[int, _Registry]: +def _record_enter( + key: _Key, enclosing: _Registry, registry: _Registry, *, outermost: bool +) -> tuple[int | None, _Registry]: """Note an entered provider block, and return its handle with the registry to install. - The handle is the block's serial, which the provider holds until it exits. - id() would be reused by the next provider at that address. + The handle is the block's serial, which the provider holds until it + exits, and it is None when only the audit is watching, since then the + ledger has nothing to forget. id() would be reused by the next provider + at that address. """ - site, _ = _user_site() - where = _where() - reads = _Reads() if _state.counting else None - with _lock: - _state.seq += 1 - handle = _state.seq - _open[handle] = _Block(key, site, where, handle, reads) + handle: int | None = None + reads: _Reads | None = None + if _state.recording: + site, _ = _user_site() + where = _where() + reads = _Reads() if _state.counting else None + with _lock: + _state.seq += 1 + handle = _state.seq + _open[handle] = _Block(key, site, where, handle, reads) owners: dict[_Key, _Reads] = {} - if isinstance(enclosing, _CountingRegistry): + minted = _key_path(key) + entry = minted + if isinstance(enclosing, _InstrumentedRegistry): # Inherited whether or not counting is still on, since it is process-wide. owners = dict(enclosing.owners) + # Outermost comes from the open chain rather than from the kind of mapping inherited, + # since a repaired mapping outlives its chain and would hand on a label nothing owns. + if not outermost and minted not in _declared_entries: + entry = enclosing.entry if reads is not None: owners[key] = reads - return handle, _CountingRegistry(registry, owners) if owners else registry + if not owners and not _state.auditing: + return handle, registry + return handle, _InstrumentedRegistry(registry, owners, entry) def _remember_closed(entry: _Block) -> None: @@ -371,6 +447,7 @@ def __enter__(self) -> None: with _lock: _state.depth += 1 _state.recording = True + _state.watching = True if self._unused: _state.unused_depth += 1 _state.counting = True @@ -384,6 +461,7 @@ def __exit__( with _lock: _state.depth -= 1 _state.recording = _state.depth > 0 + _state.watching = _state.recording or _state.auditing if self._unused: _state.unused_depth -= 1 _state.counting = _state.unused_depth > 0 diff --git a/src/nodrill/_errors.py b/src/nodrill/_errors.py index 73f48df..0383cc4 100644 --- a/src/nodrill/_errors.py +++ b/src/nodrill/_errors.py @@ -18,6 +18,18 @@ def _describe_key(key: Any) -> str: return repr(key) if isinstance(key, str) else getattr(key, "__qualname__", repr(key)) +def _key_path(key: _Key) -> str: + """Render a key the way ref() spells one, so two same-named classes stay apart. + + _describe_key renders a bare qualname, which reads well in a message and + is ambiguous in a file that is diffed, since two Config classes in two + modules render identically. + """ + if isinstance(key, str): + return repr(key) + return f"{key.__module__}:{key.__qualname__}" + + def _rebuilt( cls: type[BaseException], args: tuple[Any, ...], state: dict[str, Any] ) -> BaseException: diff --git a/src/nodrill/_inject.py b/src/nodrill/_inject.py index f2d350d..5fe0c26 100644 --- a/src/nodrill/_inject.py +++ b/src/nodrill/_inject.py @@ -417,8 +417,10 @@ def _missing_guard_lines(label: str, missing: list[str], ns: _WrapperSpace) -> l def _resolve_lines(target: str, key: str, ns: _WrapperSpace, indent: str) -> list[str]: """Render the one lookup template, an inlined registry hit with the miss path in _core.""" return [ - f"{indent}{target} = {ns.registry}().get({key}, {ns.omitted})", - f"{indent}if {target} is {ns.omitted}:", + # A subscript in a try beats get() plus an identity test, since a hit skips the handler. + f"{indent}try:", + f"{indent} {target} = {ns.registry}()[{key}]", + f"{indent}except KeyError:", f"{indent} {target} = {ns.miss}({key})", ] diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 0000000..35d9bc0 --- /dev/null +++ b/tests/__init__.py @@ -0,0 +1 @@ +"""The suite, a package so both checkers agree on what a test module is called.""" diff --git a/tests/audit_app/__init__.py b/tests/audit_app/__init__.py new file mode 100644 index 0000000..fb034f9 --- /dev/null +++ b/tests/audit_app/__init__.py @@ -0,0 +1,7 @@ +"""A small application with three entry points, one of which cannot see what it reads. + +Written to be representative rather than flattering. The broken entry point +fails the way a real one does, by reading a key a boundary above it never +opened, and the two working ones read the same key through different shapes +so a contract has something to say about each. +""" diff --git a/tests/audit_app/app.py b/tests/audit_app/app.py new file mode 100644 index 0000000..9af9578 --- /dev/null +++ b/tests/audit_app/app.py @@ -0,0 +1,74 @@ +"""The application itself, a process-wide layer with three entry points under it. + +Shaped after a real service rather than after what flatters the tool. Config +is opened once in main and every boundary nests under it, which is the shape +that collapses to one entry point unless the boundaries are declared. +""" + +from __future__ import annotations + +from collections.abc import Iterator +from contextlib import contextmanager + +from nodrill import FromCtx, inject, injected, provider, set_default, use + + +class Settings: + """What the process is configured with, opened once above everything.""" + + def __init__(self, dsn: str = "sqlite://") -> None: + self.dsn = dsn + + +class User: + """Who the request is for.""" + + def __init__(self, name: str = "anonymous") -> None: + self.name = name + + +class Origin: + """Where a write came from, which an audit table records.""" + + def __init__(self, label: str = "system") -> None: + self.label = label + + +set_default(Origin, Origin) + + +def record_write() -> str: + """Write a row, naming the user and where the write came from.""" + return f"{use(User).name} from {use(Origin).label}" + + +@inject +def open_connection(settings: FromCtx[Settings] = injected) -> str: + """Read through a compiled wrapper, which is a different code path from use().""" + return settings.dsn + + +@contextmanager +def running() -> Iterator[None]: + """Open the process-wide layer the way a main function does.""" + with provider(Settings()): + yield + + +def serve_http(name: str) -> str: + """The web entry point, which opens both keys the handler reads.""" + with provider("http request", route="/writes"), provider(User(name)): + with provider(Origin("http")): + return f"{record_write()} {open_connection()} {use('http request').route}" + + +def run_job(name: str) -> str: + """The queue entry point, which opens the user and leaves the origin to fall back.""" + with provider("celery worker"), provider(User(name)): + return record_write() + + +def run_report() -> str: + """The reporting entry point, which opens a boundary and forgets the user.""" + with provider("nightly report"): + return record_write() diff --git a/tests/test_audit.py b/tests/test_audit.py new file mode 100644 index 0000000..0460c7c --- /dev/null +++ b/tests/test_audit.py @@ -0,0 +1,575 @@ +"""What a run records, what the contract file says, and what the tool admits it cannot know.""" + +from __future__ import annotations + +import asyncio +import atexit +import multiprocessing.util +import os +import runpy +import subprocess +import sys +from collections.abc import Iterator +from pathlib import Path +from typing import Any + +import pytest + +from nodrill import NoProviderError, provider, ref, set_default, use, wrap +from nodrill._audit import ( + _contract, + _counted, + _declared, + _dump, + _merge, + _new_run, + _parse, + _render, + _summary, + _unseen, + main, +) +from nodrill._debug import _arm, _declared_entries, _reads, _state +from tests.audit_app.app import User, open_connection, run_job, run_report, running, serve_http + +_ROOT = Path(__file__).parent.parent +HEADER = "# nodrill contract 1" +APP = "tests.audit_app.app" +TAB = "\t" + + +@pytest.fixture +def recording() -> Iterator[set[tuple[str, str, str]]]: + """Turn the audit on for one test, which no public name does on purpose.""" + # Saved and restored rather than switched off, since a process may be recording for real. + saved = (_state.auditing, _state.watching, set(_reads), set(_declared_entries)) + _reads.clear() + _state.auditing = True + _state.watching = True + try: + yield _reads + finally: + _state.auditing, _state.watching = saved[0], saved[1] + _reads.clear() + _reads.update(saved[2]) + _declared_entries.clear() + _declared_entries.update(saved[3]) + + +@pytest.fixture +def declaring(recording: set[tuple[str, str, str]]) -> set[tuple[str, str, str]]: + """Name the app's two boundaries the way NODRILL_CONTRACT_ENTRY does.""" + # The recording fixture restores the declared set, so this one only has to fill it. + _declared_entries.update({"'http request'", "'celery worker'"}) + return recording + + +@pytest.fixture +def armed(recording: set[tuple[str, str, str]]) -> Iterator[list[tuple[Any, ...]]]: + """Collect what _arm registers, so a test never leaves a real hook on this process.""" + calls: list[tuple[Any, ...]] = [] + with pytest.MonkeyPatch.context() as patch: + patch.setattr(atexit, "register", lambda *call: calls.append(call)) + patch.setattr( + multiprocessing.util, "Finalize", lambda *call, **kw: calls.append((call, kw)) + ) + yield calls + + +def _facts(reads: set[tuple[str, str, str]]) -> set[str]: + """Render what was recorded the way the contract file does, minus the header.""" + return {line for line in _render(reads).splitlines() if line != HEADER} + + +def _entries(reads: set[tuple[str, str, str]]) -> set[str]: + """The first column, which is the whole question the entry point rule answers.""" + return {entry for entry, _, _ in reads} + + +class TestWhatARunRecords: + """A read is credited to the outermost block open above it.""" + + def test_a_read_is_credited_to_the_entry_point(self, declaring: Any) -> None: + with running(): + serve_http("ada") + assert f"'http request'{TAB}requires{TAB}{APP}:User" in _facts(declaring) + + def test_a_nested_block_does_not_become_an_entry_point(self, declaring: Any) -> None: + with running(): + serve_http("ada") + assert _entries(declaring) == {"'http request'"} + + def test_a_class_keyed_block_is_an_entry_point_like_any_other(self, recording: Any) -> None: + with running(): + use(User, default=None) + assert _entries(recording) == {f"{APP}:Settings"} + + def test_a_string_key_keeps_its_quotes_and_a_class_key_is_a_path(self, declaring: Any) -> None: + with running(): + serve_http("ada") + assert f"'http request'{TAB}requires{TAB}'http request'" in _facts(declaring) + assert f"'http request'{TAB}requires{TAB}{APP}:Origin" in _facts(declaring) + + def test_a_read_through_inject_is_recorded_like_any_other(self, declaring: Any) -> None: + with running(), provider("http request"): + open_connection() + assert _facts(declaring) == {f"'http request'{TAB}requires{TAB}{APP}:Settings"} + + def test_a_ref_key_records_what_it_resolves_to(self, recording: Any) -> None: + with provider("http request"), provider(User("ada")): + assert use(ref(f"{APP}:User")).name == "ada" + assert _facts(recording) == {f"'http request'{TAB}requires{TAB}{APP}:User"} + + def test_a_read_outside_every_block_has_no_entry_point(self, recording: Any) -> None: + class Loose: + pass + + set_default(Loose, Loose) + use(Loose) + assert _entries(recording) == {"(none)"} + + def test_two_classes_of_the_same_name_stay_apart(self, recording: Any) -> None: + class User: # the point is that this collides with the app's User + pass + + with provider("boundary"), provider(User()): + use(User) + recorded = {line for line in _facts(recording) if "User" in line} + assert len(recorded) == 1 + assert f"{APP}:User" not in next(iter(recorded)) + + +class TestTheCollapseAndTheDeclaration: + """A layer above the boundaries swallows them, which is why a boundary can be named.""" + + def test_a_process_wide_layer_swallows_every_boundary(self, recording: Any) -> None: + with running(): + serve_http("ada") + run_job("grace") + assert _entries(recording) == {f"{APP}:Settings"} + + def test_a_declared_key_mints_its_own_entry_point_under_that_layer( + self, declaring: Any + ) -> None: + with running(): + serve_http("ada") + run_job("grace") + assert _entries(declaring) == {"'http request'", "'celery worker'"} + + def test_the_fallback_lands_on_the_boundary_that_let_it_happen(self, declaring: Any) -> None: + with running(): + run_job("grace") + assert f"'celery worker'{TAB}set_default{TAB}{APP}:Origin" in _facts(declaring) + + def test_a_declared_key_nothing_opened_is_reported(self) -> None: + reads = {("'http request'", "requires", "x")} + assert _unseen(reads, frozenset({"'http request'"})) is None + message = _unseen(reads, frozenset({"'http request'", "'celery worker'"})) + assert message is not None + assert "no block opened 'celery worker'" in message + + @pytest.mark.parametrize( + ("value", "expected"), + [ + ("", frozenset()), + ("'a'", frozenset({"'a'"})), + ("'a', pkg:B ", frozenset({"'a'", "pkg:B"})), + ], + ids=["empty", "one", "several"], + ) + def test_the_variable_is_rendered_keys_separated_by_commas( + self, value: str, expected: frozenset[str] + ) -> None: + assert _declared(value) == expected + + +class TestWhatARaiseWouldNeverReport: + """A registration answering a miss is the case the audit exists for.""" + + def test_a_set_default_fallback_is_recorded_under_its_entry_point(self, recording: Any) -> None: + run_job("grace") + assert f"'celery worker'{TAB}set_default{TAB}{APP}:Origin" in _facts(recording) + + def test_a_default_argument_is_recorded_too(self, recording: Any) -> None: + with provider("celery worker"): + assert use(User, default=None) is None + assert f"'celery worker'{TAB}default{TAB}{APP}:User" in _facts(recording) + + def test_a_miss_that_actually_raises_records_nothing(self, recording: Any) -> None: + with pytest.raises(NoProviderError): + run_report() + assert not any("User" in line for line in _facts(recording)) + + @pytest.mark.parametrize("call", [lambda: use(User), open_connection], ids=["use", "inject"]) + def test_a_miss_carries_no_internal_exception(self, call: Any) -> None: + """The wrapper resolves inside its own except KeyError and must not show it.""" + with pytest.raises(NoProviderError) as raised: + call() + assert raised.value.__suppress_context__ + + +class TestTheLabelSurvivesTheAwkwardPaths: + """The entry point rides the registry, so it goes wherever the registry goes.""" + + def test_a_block_closing_out_of_order_keeps_the_entry_point(self, recording: Any) -> None: + def tenant(slug: str) -> Iterator[None]: + with provider("tenant", slug=slug): + yield + yield + + with provider("http request", route="/"): + first, second = tenant("acme"), tenant("globex") + list(zip(first, second, strict=False)) + list(first) + list(second) + use("http request") + assert _facts(recording) == {f"'http request'{TAB}requires{TAB}'http request'"} + + def test_a_repair_does_not_hand_its_label_to_the_next_boundary(self, recording: Any) -> None: + """The mapping a repair leaves outlives its chain, and must not name what follows.""" + + def tenant(slug: str) -> Iterator[None]: + with provider("tenant", slug=slug): + yield + yield + + with provider("http request", route="/"): + first, second = tenant("acme"), tenant("globex") + list(zip(first, second, strict=False)) + list(first) + list(second) + with provider("celery worker"), provider(User("grace")): + use(User) + facts = _facts(recording) + assert f"'celery worker'{TAB}requires{TAB}{APP}:User" in facts + assert f"'http request'{TAB}requires{TAB}{APP}:User" not in facts + + def test_a_thread_carries_the_entry_point_it_was_wrapped_under( + self, recording: Any, in_thread: Any + ) -> None: + with provider("http request", route="/"), provider(User("ada")): + in_thread(wrap(lambda: use(User))) + assert f"'http request'{TAB}requires{TAB}{APP}:User" in _facts(recording) + + def test_a_thread_nobody_wrapped_reads_under_no_entry_point( + self, recording: Any, in_thread: Any + ) -> None: + """The bug the tool exists to surface, recorded as the fallback it becomes.""" + + class Loose: + pass + + set_default(Loose, Loose) + with provider("http request", route="/"): + in_thread(lambda: use(Loose)) + assert _entries(recording) == {"(none)"} + + async def test_a_sibling_task_reads_under_its_own_entry_point(self, recording: Any) -> None: + async def worker(label: str) -> None: + with provider(label), provider(User(label)): + await asyncio.sleep(0) + use(User) + + await asyncio.gather(worker("http request"), worker("celery worker")) + assert _facts(recording) == { + f"'http request'{TAB}requires{TAB}{APP}:User", + f"'celery worker'{TAB}requires{TAB}{APP}:User", + } + + +class TestTheSwitch: + """Off is the default, and arming is a function so it can be tested without a child.""" + + def test_an_unset_variable_arms_nothing(self) -> None: + environ: dict[str, str] = {} + before = (_state.auditing, _state.watching) + _arm(environ) + assert (_state.auditing, _state.watching) == before + assert environ == {} + + def test_arming_sets_the_switches_and_joins_a_run( + self, tmp_path: Path, armed: list[tuple[Any, ...]] + ) -> None: + environ = {"NODRILL_CONTRACT": str(tmp_path), "NODRILL_CONTRACT_ENTRY": "'a'"} + _arm(environ) + assert _state.auditing + assert _state.watching + assert "'a'" in _declared_entries + # Written back so a child interpreter joins this run rather than starting one. + assert environ["NODRILL_CONTRACT_RUN"] + # Both, since a pool worker exits through os._exit and never runs atexit. + assert len(armed) == 2 + + def test_an_inherited_run_is_kept(self, tmp_path: Path, armed: list[tuple[Any, ...]]) -> None: + environ = {"NODRILL_CONTRACT": str(tmp_path), "NODRILL_CONTRACT_RUN": "given"} + _arm(environ) + assert environ["NODRILL_CONTRACT_RUN"] == "given" + assert all("given" in repr(call) for call in armed) + + def test_a_run_with_the_switch_off_records_nothing( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Off is the state a lookup pays nothing for, so it is worth pinning as behaviour.""" + monkeypatch.setattr(_state, "auditing", False) + before = set(_reads) + with running(): + serve_http("ada") + assert set(_reads) == before + + +class TestTheContractFile: + """The file is the delivery mechanism, so its shape is the deliverable.""" + + def test_the_header_names_the_format(self) -> None: + assert _render(set()) == f"{HEADER}\n" + + def test_lines_are_sorted_so_the_file_is_not_a_property_of_the_run(self) -> None: + reads = {("'b'", "requires", "x"), ("'a'", "requires", "y"), ("'a'", "default", "z")} + assert _render(reads).splitlines()[1:] == [ + f"'a'{TAB}default{TAB}z", + f"'a'{TAB}requires{TAB}y", + f"'b'{TAB}requires{TAB}x", + ] + + def test_one_new_key_is_one_added_line(self) -> None: + before = _render({("'a'", "requires", "x"), ("'a'", "requires", "z")}) + after = _render({("'a'", "requires", spelling) for spelling in "xyz"}) + assert set(after.splitlines()) - set(before.splitlines()) == {f"'a'{TAB}requires{TAB}y"} + + def test_a_key_holding_two_spaces_is_still_one_line_of_three_fields( + self, recording: Any + ) -> None: + with provider("http request"), provider("a b", tag=1): + use("a b") + [line] = _facts(recording) + assert line.split(TAB) == ["'http request'", "requires", "'a b'"] + + def test_a_key_holding_a_newline_is_still_one_line(self, recording: Any) -> None: + with provider("http request"), provider("two\nlines", tag=1): + use("two\nlines") + [line] = _facts(recording) + assert line.split(TAB) == ["'http request'", "requires", "'two\\nlines'"] + + def test_a_contract_round_trips(self) -> None: + reads = {("'a'", "requires", "x"), ("'b'", "set_default", "y")} + assert _parse(_render(reads), "test") == reads + + @pytest.mark.parametrize( + ("text", "shown"), + [("", "an empty file"), ("# nodrill contract 2\n", "# nodrill contract 2")], + ids=["empty", "another version"], + ) + def test_a_version_this_reader_does_not_know_is_refused(self, text: str, shown: str) -> None: + with pytest.raises(ValueError, match="not a nodrill contract this version reads") as raised: + _parse(text, "somewhere") + assert shown in str(raised.value) + + +class TestShards: + """One run is many processes, so the record is written per process and merged.""" + + def test_a_process_that_recorded_nothing_writes_no_shard(self, tmp_path: Path) -> None: + _dump(str(tmp_path / "missing"), "run", set()) + assert not (tmp_path / "missing").exists() + + def test_a_shard_round_trips(self, tmp_path: Path) -> None: + reads = {("'a'", "requires", "x"), ("'b'", "default", "y")} + _dump(str(tmp_path), "run", set(reads)) + assert _merge(str(tmp_path)) == (reads, 1, 0) + + def test_dumping_twice_writes_one_shard(self, tmp_path: Path) -> None: + """A pool worker is finalized as well as registered, so a second dump is a no-op.""" + reads = {("'a'", "requires", "x")} + _dump(str(tmp_path), "run", reads) + _dump(str(tmp_path), "run", reads) + assert len(list(tmp_path.glob("*.shard"))) == 1 + + def test_shards_from_several_processes_merge(self, tmp_path: Path) -> None: + _dump(str(tmp_path), "run", {("'a'", "requires", "x")}) + _dump(str(tmp_path), "run", {("'b'", "requires", "y")}) + found, shards, stale = _merge(str(tmp_path)) + assert found == {("'a'", "requires", "x"), ("'b'", "requires", "y")} + assert (shards, stale) == (2, 0) + + def test_an_earlier_run_in_the_same_directory_is_left_out(self, tmp_path: Path) -> None: + first, second = _new_run(), _new_run() + _dump(str(tmp_path), first, {("'a'", "requires", "gone")}) + _dump(str(tmp_path), second, {("'a'", "requires", "here")}) + assert _merge(str(tmp_path)) == ({("'a'", "requires", "here")}, 1, 1) + + def test_an_empty_directory_merges_to_nothing(self, tmp_path: Path) -> None: + assert _merge(str(tmp_path)) == (set(), 0, 0) + + def test_a_run_id_is_unique(self) -> None: + assert _new_run() != _new_run() + + +class TestWhatTheToolAdmits: + """A guarantee that overstates itself is worse than no guarantee.""" + + def test_the_summary_counts_what_it_rests_on(self) -> None: + reads = {("'a'", "requires", "x"), ("'a'", "requires", "y"), ("'b'", "requires", "z")} + assert _summary(reads, 2, 0) == ( + "nodrill: 3 facts under 2 entry points, recorded from 2 processes. " + "A contract is only as complete as the run that recorded it." + ) + + def test_one_of_each_reads_as_a_sentence(self) -> None: + assert _summary({("'a'", "requires", "x")}, 1, 0).startswith( + "nodrill: 1 fact under 1 entry point, recorded from 1 process." + ) + + def test_shards_left_out_are_said_rather_than_dropped_quietly(self) -> None: + assert _summary(set(), 1, 3).endswith("3 shards from an earlier run were left out.") + + @pytest.mark.parametrize( + ("count", "rendered"), [(0, "0 processes"), (1, "1 process"), (2, "2 processes")] + ) + def test_a_count_carries_its_noun(self, count: int, rendered: str) -> None: + assert _counted(count, "process", "processes") == rendered + + +class TestTheCommandLine: + """python -m nodrill is the whole surface, and it stays out of __all__.""" + + def test_a_directory_nothing_recorded_is_an_error(self, tmp_path: Path, capsys: Any) -> None: + assert _contract(str(tmp_path / "missing"), None, frozenset()) == 1 + assert "nothing recorded" in capsys.readouterr().err + + def test_a_directory_with_no_shards_says_the_recorder_never_armed( + self, tmp_path: Path, capsys: Any + ) -> None: + assert _contract(str(tmp_path), None, frozenset()) == 1 + assert "nothing armed the recorder" in capsys.readouterr().err + + def test_the_contract_goes_to_stdout_by_default(self, tmp_path: Path, capsys: Any) -> None: + _dump(str(tmp_path), "run", {("'a'", "requires", "x")}) + assert _contract(str(tmp_path), None, frozenset()) == 0 + captured = capsys.readouterr() + assert captured.out == f"{HEADER}\n'a'{TAB}requires{TAB}x\n" + assert "1 fact under 1 entry point" in captured.err + + def test_a_declared_key_nothing_opened_reaches_the_output( + self, tmp_path: Path, capsys: Any + ) -> None: + _dump(str(tmp_path), "run", {("'a'", "requires", "x")}) + assert _contract(str(tmp_path), None, frozenset({"'b'"})) == 0 + assert "no block opened 'b'" in capsys.readouterr().err + + def test_write_names_the_file(self, tmp_path: Path, capsys: Any) -> None: + _dump(str(tmp_path), "run", {("'a'", "requires", "x")}) + target = tmp_path / "nodrill.contract" + assert main(["contract", "--from", str(tmp_path), "--write", str(target)]) == 0 + assert target.read_text(encoding="utf-8") == f"{HEADER}\n'a'{TAB}requires{TAB}x\n" + assert capsys.readouterr().out == "" + + def test_a_file_it_cannot_write_is_a_message_and_not_a_traceback( + self, tmp_path: Path, capsys: Any + ) -> None: + _dump(str(tmp_path), "run", {("'a'", "requires", "x")}) + target = tmp_path / "no" / "such" / "dir" / "out" + assert _contract(str(tmp_path), str(target), frozenset()) == 1 + assert "cannot write" in capsys.readouterr().err + + def test_a_subcommand_is_required(self) -> None: + with pytest.raises(SystemExit) as raised: + main([]) + # argparse owns 2, which is why nothing recorded is 1. + assert raised.value.code == 2 + + def test_a_flag_cannot_be_abbreviated(self, tmp_path: Path) -> None: + with pytest.raises(SystemExit): + main(["contract", "--fro", str(tmp_path)]) + + +def _child(program: str, directory: Path, entries: str = "") -> subprocess.CompletedProcess[str]: + """Run a program in a child interpreter with the recorder armed.""" + return subprocess.run( # the interpreter running this suite, with a program written above + [sys.executable, "-c", program], + check=True, + capture_output=True, + text=True, + cwd=str(_ROOT), + env={ + **os.environ, + "NODRILL_CONTRACT": str(directory), + "NODRILL_CONTRACT_ENTRY": entries, + "PYTHONPATH": str(_ROOT), + }, + ) + + +class TestARecordedRun: + """The end to end path, in a child interpreter, since the switch is read once at import.""" + + def test_the_environment_variable_arms_a_whole_process(self, tmp_path: Path) -> None: + program = f"from {APP} import running, serve_http\nwith running(): serve_http('ada')" + _child(program, tmp_path) + reads, _, _ = _merge(str(tmp_path)) + assert f"{APP}:Settings{TAB}requires{TAB}{APP}:User" in _render(reads) + + def test_declaring_the_boundaries_splits_the_entry_points(self, tmp_path: Path) -> None: + _child( + f"from {APP} import running, serve_http, run_job\n" + "with running():\n serve_http('ada')\n run_job('grace')", + tmp_path, + entries="'http request','celery worker'", + ) + reads, _, _ = _merge(str(tmp_path)) + assert _entries(reads) == {"'http request'", "'celery worker'"} + + def test_a_subprocess_the_run_spawns_joins_the_same_run(self, tmp_path: Path) -> None: + program = ( + "import subprocess, sys\n" + f"from {APP} import running, serve_http\n" + "with running(): serve_http('ada')\n" + "subprocess.run([sys.executable, '-c'," + f" 'from {APP} import run_job; run_job(\"grace\")'], check=True)\n" + ) + _child(program, tmp_path) + reads, shards, stale = _merge(str(tmp_path)) + assert (shards, stale) == (2, 0) + assert f"'celery worker'{TAB}set_default{TAB}{APP}:Origin" in _render(reads) + + def test_a_process_pool_worker_records_its_own_shard(self, tmp_path: Path) -> None: + """A worker exits through os._exit, which runs finalizers and never atexit.""" + program = ( + "from concurrent.futures import ProcessPoolExecutor\n" + f"from {APP} import run_job\n" + "if __name__ == '__main__':\n" + " with ProcessPoolExecutor(max_workers=1) as pool:\n" + " pool.submit(run_job, 'grace').result()\n" + ) + _child(program, tmp_path) + reads, _, _ = _merge(str(tmp_path)) + assert f"'celery worker'{TAB}requires{TAB}{APP}:User" in _render(reads) + + def test_two_runs_of_the_same_program_agree_byte_for_byte(self, tmp_path: Path) -> None: + program = f"from {APP} import running, serve_http\nwith running(): serve_http('ada')" + first, second = tmp_path / "first", tmp_path / "second" + _child(program, first) + _child(program, second) + assert _render(_merge(str(first))[0]) == _render(_merge(str(second))[0]) + + def test_the_module_runs_as_a_command(self, tmp_path: Path) -> None: + program = f"from {APP} import running, serve_http\nwith running(): serve_http('ada')" + _child(program, tmp_path) + result = subprocess.run( # the interpreter running this suite + [sys.executable, "-m", "nodrill", "contract", "--from", str(tmp_path)], + check=True, + capture_output=True, + text=True, + ) + assert result.stdout.startswith(f"{HEADER}\n") + assert "A contract is only as complete as the run that recorded it." in result.stderr + + def test_the_dispatch_exits_with_what_the_command_returned( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: Any + ) -> None: + """__main__ is the two lines that turn a return code into an exit code.""" + argv = ["nodrill", "contract", "--from", str(tmp_path / "missing")] + monkeypatch.setattr(sys, "argv", argv) + with pytest.raises(SystemExit) as raised: + runpy.run_module("nodrill", run_name="__main__") + assert raised.value.code == 1 + assert "nothing recorded" in capsys.readouterr().err diff --git a/tests/test_debug.py b/tests/test_debug.py index cb852e8..668fd94 100644 --- a/tests/test_debug.py +++ b/tests/test_debug.py @@ -527,6 +527,16 @@ def test_an_extending_layer_counts_as_a_read_of_what_it_extends(self) -> None: [record] = records assert record.lineno == layered + def test_a_layer_over_nothing_credits_no_one(self) -> None: + """extend=True over a name nothing provided has no outer namespace to read.""" + with pytest.warns(UnusedProviderWarning, match="never read") as records, debug(unused=True): + with provider("db", dsn="x"): + use("db") + with provider("app", extend=True, tag="x"): + opened = line_above() + [record] = records + assert record.lineno == opened + def test_a_block_that_raised_is_not_warned_about(self) -> None: """A body that blew up never had the chance to read, so it is not blamed for it.""" with debug(unused=True): diff --git a/tests/test_inject_codegen.py b/tests/test_inject_codegen.py index a670815..5468d26 100644 --- a/tests/test_inject_codegen.py +++ b/tests/test_inject_codegen.py @@ -160,8 +160,9 @@ def handler(request: str, db: FromCtx[Db] = injected) -> str: assert generated_source(handler) == ( "def handler(request, db=_nd_injected):\n" " if db is _nd_injected:\n" - " db = _nd_registry().get(_nd_key_db, _nd_omitted)\n" - " if db is _nd_omitted:\n" + " try:\n" + " db = _nd_registry()[_nd_key_db]\n" + " except KeyError:\n" " db = _nd_miss(_nd_key_db)\n" " return _nd_func(request, db)" ) @@ -175,8 +176,9 @@ def handler(dsn: Annotated[str, from_ctx(ref(f"{__name__}:Db"))] = injected) -> assert generated_source(handler) == ( "def handler(dsn=_nd_injected):\n" " if dsn is _nd_injected:\n" - " _nd_value = _nd_registry().get(_nd_key_dsn, _nd_omitted)\n" - " if _nd_value is _nd_omitted:\n" + " try:\n" + " _nd_value = _nd_registry()[_nd_key_dsn]\n" + " except KeyError:\n" " _nd_value = _nd_miss(_nd_key_dsn)\n" " dsn = _nd_ref_attr(_nd_key_dsn, _nd_value, 'dsn')\n" " return _nd_func(dsn)" @@ -193,8 +195,9 @@ def handler(db: FromCtx[Db], tag: str) -> str: " if tag is _nd_injected:\n" " raise _nd_missing_error(_nd_label, (('tag', tag),))\n" " if db is _nd_injected:\n" - " db = _nd_registry().get(_nd_key_db, _nd_omitted)\n" - " if db is _nd_omitted:\n" + " try:\n" + " db = _nd_registry()[_nd_key_db]\n" + " except KeyError:\n" " db = _nd_miss(_nd_key_db)\n" " return _nd_func(db, tag)" ) @@ -211,16 +214,18 @@ def render(user: str, lang: str = "en") -> str: " _nd_unmet = []\n" " if user is _nd_injected:\n" " if _nd_source is _nd_omitted:\n" - " _nd_source = _nd_registry().get(_nd_from_key, _nd_omitted)\n" - " if _nd_source is _nd_omitted:\n" + " try:\n" + " _nd_source = _nd_registry()[_nd_from_key]\n" + " except KeyError:\n" " _nd_source = _nd_miss(_nd_from_key)\n" " user = _nd_getattr(_nd_source, 'user', _nd_omitted)\n" " if user is _nd_omitted:\n" " _nd_unmet.append('user')\n" " if lang is _nd_injected:\n" " if _nd_source is _nd_omitted:\n" - " _nd_source = _nd_registry().get(_nd_from_key, _nd_omitted)\n" - " if _nd_source is _nd_omitted:\n" + " try:\n" + " _nd_source = _nd_registry()[_nd_from_key]\n" + " except KeyError:\n" " _nd_source = _nd_miss(_nd_from_key)\n" " lang = _nd_getattr(_nd_source, 'lang', _nd_omitted)\n" " if lang is _nd_omitted:\n" From 1738080cd56f5c9fc0e7b620ab2e5771aa0d690b Mon Sep 17 00:00:00 2001 From: Pavel Kutsenko Date: Mon, 24 Aug 2026 01:59:56 +0300 Subject: [PATCH 03/12] fix: close the paths the review found in the contract recorder A forked worker recorded nothing, because multiprocessing clears the finalizer registry before the worker body runs, so the child now registers the dump again from an after-fork hook. Only spawn worked before, which is the macOS default and not the Linux one, so the pool test was green here and red on every CI job. The directory NODRILL_CONTRACT names is resolved when the variable is read rather than at exit, since the hooks run after a program may have moved. The reader is defensive now. A shard truncated by a killed worker, one from a version this reader does not know, and one carrying a verb nothing writes are each a message and the exit code the reference page promises. _VERBS is what _parse refuses a line outside of, which is what gives the constant a reader. The write side is guarded the same way, since it runs in an exit hook where a raise is a traceback the process still exits zero after, and the newline and the encoding are named rather than left to the platform. The @inject wrapper binds a sentinel in its except KeyError and takes the miss path after the handler, so nothing a set_default factory raises arrives chained to a lookup the caller never wrote. That is what from None was reaching for, and from None was wrong in both directions, since it also hid the caller's own in-flight exception on the use() path. A repair derives the entry label again from the chain that survived, and moves the restored key's read counter to the block it was restored from, so neither the label nor a later read belongs to the block that just left. The repair reads through dict.__getitem__, which is invisible to the recorder and to the read counter alike and still raises if the key is missing. What the recorder keeps is capped, because an entry point is a provider key and a key built per request grew the set for the life of the process. Recording a contract is scoped by _recording() rather than by a fixture flipping four module globals, and the fixture clearing the declared set is what lets the suite run under the environment its own how-to documents. --- .../howto/record-what-a-handler-reads.rst | 28 +- docs/content/misc/design.rst | 25 +- docs/content/misc/performance.rst | 3 +- docs/content/ref/debugging.rst | 41 +-- src/nodrill/__main__.py | 7 +- src/nodrill/_audit.py | 131 +++++---- src/nodrill/_core.py | 33 ++- src/nodrill/_debug.py | 170 +++++++++--- src/nodrill/_declare.py | 4 +- src/nodrill/_errors.py | 19 +- src/nodrill/_inject.py | 9 +- tests/test_audit.py | 258 +++++++++++++++--- tests/test_inject_codegen.py | 10 + 13 files changed, 569 insertions(+), 169 deletions(-) diff --git a/docs/content/howto/record-what-a-handler-reads.rst b/docs/content/howto/record-what-a-handler-reads.rst index 0ba19b0..ac0f0e8 100644 --- a/docs/content/howto/record-what-a-handler-reads.rst +++ b/docs/content/howto/record-what-a-handler-reads.rst @@ -144,16 +144,24 @@ Two steps, recording and reviewing. .. code-block:: yaml :caption: .github/workflows/ci.yml - - run: NODRILL_CONTRACT=.nodrill pytest - env: - NODRILL_CONTRACT_ENTRY: "'http request','celery worker'" - - run: python -m nodrill contract --from .nodrill --write nodrill.contract - - run: git diff --exit-code nodrill.contract + env: + NODRILL_CONTRACT_ENTRY: "'http request','celery worker'" + steps: + - run: NODRILL_CONTRACT=.nodrill pytest + - run: python -m nodrill contract --from .nodrill --write nodrill.contract + - run: git diff --exit-code nodrill.contract + +The variable is on the job rather than on the recording step, because the command reads it too, and that is what lets it report a boundary you named that no block opened. +The contract file has to be committed for the third step to compare anything, since ``git diff`` says nothing about a path git does not track. Recording is off unless ``NODRILL_CONTRACT`` is set, and the variable is read once when `nodrill` is imported, which is also why a subprocess your suite spawns records too. -Each process writes its own file into the directory and the command merges them, so a suite under `xdist`, one that shells out, or one using a :class:`~concurrent.futures.ProcessPoolExecutor` needs nothing extra. +Each process writes its own file into the directory and the command merges them, so a suite that shells out or one using a :class:`~concurrent.futures.ProcessPoolExecutor` needs nothing extra. A directory reused by a later run is not a problem either, since every process of one run shares a run id and the command reads the newest run and says how many older files it left out. +A run id is inherited through the environment, so processes of one run share it only when the process that started them imported `nodrill` itself. +A runner that starts its workers directly is the case where that does not hold, and `pytest -n` from a controller whose `conftest.py` never imports the library is the one you are most likely to meet. +Either import `nodrill` in `conftest.py`, or set ``NODRILL_CONTRACT_RUN`` yourself alongside ``NODRILL_CONTRACT``, and the summary will then report one run rather than shards left out. + What the contract is worth -------------------------- @@ -162,7 +170,7 @@ Exactly as much as the run that recorded it. A contract lists what the run observed and nothing else, so a key only one untested branch reads is a key the file does not mention. The summary line says how many facts under how many entry points the conclusion rests on, and it says it every time rather than only when the number is small, because a guarantee that overstates itself is worse than no guarantee. -Three limits are worth knowing before you rely on it. +Five limits are worth knowing before you rely on it. The entry point is a key, so two boundaries that open the same one are one row set, and a boundary you have not named is whatever is open above it. @@ -172,6 +180,12 @@ The file is about what an entry point needs and gets, and a miss that reaches a A value read through :func:`~nodrill.inject` is recorded exactly like one read through :func:`~nodrill.use`, but a value a handler receives as an ordinary argument is not context and never appears. The file describes the context a boundary depends on, which is the part of its input that no signature shows. +An ambient read through `nodrill.context` is not recorded, because the ambient namespace is unscoped and has no entry point to be credited to. +A handler that reaches for `context.request_id` shows nothing in the file, and a provider block is what makes a dependency reviewable. + +A :func:`~nodrill.lazy` factory runs under the context its own block was opened in, so the keys it reads are credited to whatever entry point was current then rather than to the boundary whose request forced the build. +Read the factory's dependencies under the boundary as well if the row matters, or open the lazy block inside the boundary. + .. rubric:: See also - :doc:`find-out-why-the-context-is-missing` for a miss that is happening now rather than one that might. diff --git a/docs/content/misc/design.rst b/docs/content/misc/design.rst index 67aaa98..91ba984 100644 --- a/docs/content/misc/design.rst +++ b/docs/content/misc/design.rst @@ -478,6 +478,8 @@ That choice was forced rather than preferred. A compiled :func:`~nodrill.inject` wrapper binds its registry accessor at decoration, so patching a module afterwards does not reach it, and swapping ``use`` or installing a pytest plugin would record every plain lookup and none of the injected ones. Emitting a recording branch from the codegen instead would make an enabled and a disabled wrapper two different compiled artefacts, which is the one thing the wrapper's design does not allow. The wrapper's registry read is a subscript inside a ``try`` for the same reason it is fast, and that spelling is now load-bearing twice over, since a read through ``get`` is deliberately not recorded. +The miss it falls through to runs after the handler and not inside it, so nothing a :func:`~nodrill.set_default` factory raises arrives chained to a ``KeyError`` the caller never wrote, and a hit still skips the handler entirely. +A subscript in a ``try`` wins on the hit and loses on the miss, by about as much again, which is the right trade only because a parameter answered by a provider is the common case and one answered by a fallback takes the miss path on every call. An entry point is the key of a provider block with no block open above it, or of one that ``NODRILL_CONTRACT_ENTRY`` names. The first rule alone was the original design and it does not survive contact with an ordinary application. @@ -488,11 +490,18 @@ Outermost is read off the chain of open blocks, not off the kind of mapping a bl The difference matters because a block closing out of order leaves a repaired mapping that outlives its own chain, and asking "was the mapping I inherited an instrumented one" would hand that dead mapping's label to the next boundary that opened. The chain is already computed one line further down in the same method, so the correct rule is also the cheaper one. -The label travels on the registry, which is what makes it survive a task, a wrapped thread and a repair, and the repair carries it for the same reason it carries the counting table. +The label travels on the registry, which is what makes it survive a task, a wrapped thread and a repair. +A repair derives it again from the chain that survived rather than copying the one the replaced mapping held, since the block that minted that label may be the one that just left. +The key a repair restores is credited to the block it was restored from, for the same reason and by the same rule, so the next read of it counts for a block that is still open. + +What the recorder keeps is capped. +An entry point is a provider key, and a key built per request mints one entry point per request, so the set would otherwise grow for as long as the process lives and the file would hold one line per request. +The cap says on standard error that the run stopped being one a contract can rest on, and names the variable that turns a per-request key back into one boundary. A consumer read is a subscript. -:func:`~nodrill.use` and the compiled wrapper both read the registry with ``[]``, and everything the library does to a registry for its own reasons, the open chain, an ``extend=True`` merge and the out-of-order repair, reads it with ``get``, so the recorder can tell a read that a user wrote from a read that the library did without being told which is which. -The instrumented registry also sees what a caller passed rather than what the registry stores, so a :func:`~nodrill.ref` arrives unresolved and is resolved before it is recorded. +:func:`~nodrill.use` and the compiled wrapper both read the registry with ``[]``, and everything the library does to a registry for its own reasons, the open chain, an ``extend=True`` merge and the out-of-order repair, reads it unhooked through ``dict`` itself or through ``get``, so the recorder can tell a read that a user wrote from a read that the library did without being told which is which. +The instrumented registry also sees what a caller passed rather than what the registry stores, so a :func:`~nodrill.ref` arrives unresolved and is resolved before it is recorded, and anything else is rendered by its ``repr`` rather than raising out of instrumentation that is supposed to be passive. +The repair reads through ``dict.__getitem__`` rather than through ``get``, which keeps it invisible to both the recorder and the read counter and still raises if the key it is restoring is ever missing. The recorder sits above the defaults probe in the miss path rather than on the raise. A :func:`~nodrill.set_default` factory and a ``use(key, default=...)`` both return before anything reports a miss, so a lookup that a registration is quietly answering is invisible to anything watching for the error, and that lookup is the one the whole feature exists to surface. @@ -505,8 +514,16 @@ The verb carries the whole answer, ``requires`` or ``set_default`` or ``default` The switch is an environment variable read once at import, because a child interpreter inherits one. That is what makes a suite that spawns subprocesses, runs under ``xdist`` or uses a process pool record without a special case for any of them. -A pool worker needs one more thing, since :mod:`multiprocessing` exits a worker through :func:`os._exit`, which runs finalizers and never :mod:`atexit`, so the dump is registered both ways and made idempotent rather than registered once and lost. +A pool worker needs two more things. +:mod:`multiprocessing` exits a worker through :func:`os._exit`, which runs finalizers and never :mod:`atexit`, so the dump is registered both ways and made idempotent rather than registered once and lost. +A fork then clears the finalizer registry before the worker body runs, so the child registers the finalizer again from an after-fork hook, which is the one callback :mod:`multiprocessing` runs after that clear. +The directory is resolved to an absolute path when the variable is read, since the hooks run at exit and a program that changed directory would otherwise write somewhere nobody looks. Each process of a run shares a run id minted at arming and written back into the environment, so a directory reused by a later run yields the newer contract rather than the union of both. +That inheritance works through the environment, so it reaches a child and not a sibling started by a runner that never imported the library, which is why the variable can also be set from outside. + +Everything the reader can be handed is a file somebody else wrote. +A shard truncated by a killed worker, one from a version this reader does not know, one carrying a verb nothing writes, are each a message and the exit code the reference page promises, never a traceback out of a command line. +The recorder's own write is held to the same rule from the other side, since it runs in an exit hook where a raise is a traceback the process still exits zero after. isolate() --------- diff --git a/docs/content/misc/performance.rst b/docs/content/misc/performance.rst index 7853e8d..463628f 100644 --- a/docs/content/misc/performance.rst +++ b/docs/content/misc/performance.rst @@ -62,7 +62,8 @@ What has no row Debug mode has none, because it is not for the hot path. :func:`~nodrill.debug` makes entering a provider read the stack and write to a ledger, and leaves a lookup that hits costing what it always cost. -``debug(unused=True)`` also routes every read through a counting registry, which puts a hit at roughly three times its usual price. +``debug(unused=True)`` also routes every read through an instrumented registry, which puts a hit at roughly three times its usual price. +``NODRILL_CONTRACT`` installs the same registry and pays the same, which is why recording a contract belongs in a suite and not in a running service. Exception notes have none either, because nothing in that path runs until an exception is already leaving a block. A block that exits cleanly costs one pointer comparison more than it did before :func:`~nodrill.annotate_exceptions` existed. diff --git a/docs/content/ref/debugging.rst b/docs/content/ref/debugging.rst index 4ead23c..a384f08 100644 --- a/docs/content/ref/debugging.rst +++ b/docs/content/ref/debugging.rst @@ -53,6 +53,23 @@ debug NODRILL_DEBUG=1 python -m myapp + With ``unused=True``, a provider nothing read warns as its block exits, pointing at the ``with`` statement that opened it. + + .. code-block:: text + + UnusedProviderWarning: nodrill: the provider for Session at scope.py:31 was never read, + since no use(Session) ran inside the block. + + It is an :exc:`UnusedProviderWarning`, so it can be silenced by category rather than by matching its message. + Counting is off by default even inside debug mode, because a warning changes what a program prints. + A block whose body raised is never warned about, since nothing had the chance to read it. + + Debug mode is not for production. + Every provider entered reads the stack and writes to the ledger, while a lookup that hits costs what it costs with debug mode off. + ``unused=True`` puts a counting registry in front of every read on top of that, which is roughly three times a plain hit. + + :ref:`howto-find-out-why-the-context-is-missing` runs all of it on a live program. + .. _ref-contract: Recording a contract @@ -61,6 +78,8 @@ Recording a contract ``NODRILL_CONTRACT`` names a directory and turns on recording of what each entry point reads. It is read once, at import, like ``NODRILL_DEBUG``, and any non-empty value is a directory rather than a switch, so ``0`` names a directory called ``0``. Every process of a run writes its own file there, including one a suite spawns, and the files are merged when the contract is rendered. +A relative directory is resolved when the variable is read, so a program that changes directory still writes where it was armed. +Recording puts an instrumented registry in front of every read, at the same cost ``unused=True`` pays, so it belongs in a suite rather than in production. ``NODRILL_CONTRACT_ENTRY`` names the provider keys that are boundaries, as rendered keys separated by commas, so ``"'http request',myapp.web:Request"``. A block whose key is named mints its own entry point even when a block is open above it. @@ -71,29 +90,17 @@ Without it the entry point is whatever block is outermost, which in an applicati NODRILL_CONTRACT=.nodrill NODRILL_CONTRACT_ENTRY="'http request'" pytest python -m nodrill contract --from .nodrill --write nodrill.contract +``NODRILL_CONTRACT_RUN`` groups the processes of one run, and is written into the environment by the first process that reads ``NODRILL_CONTRACT``. +A child inherits it and joins the run, while a worker whose parent never imported nodrill would start a run of its own, which the merge then reports as shards left out. +Setting it yourself is how a runner that starts its workers directly, such as ``pytest -n`` from a controller that never imports the library, keeps one run. + ``python -m nodrill contract`` renders what a run recorded. ``--from`` names the directory and is required, ``--write`` names the file and defaults to standard output, and the summary of what the contract rests on always goes to standard error so the artefact can be piped. It returns ``0`` when it rendered a contract and ``1`` when it could not, leaving ``2`` to mean the command line itself was wrong. +A shard it cannot read is a message and the exit code, never a traceback, and ``python -m nodrill --version`` says which nodrill is reading. :doc:`/content/howto/record-what-a-handler-reads` is the task-shaped version, with the file format and what it is worth. - With ``unused=True``, a provider nothing read warns as its block exits, pointing at the ``with`` statement that opened it. - - .. code-block:: text - - UnusedProviderWarning: nodrill: the provider for Session at scope.py:31 was never read, - since no use(Session) ran inside the block. - - It is an :exc:`UnusedProviderWarning`, so it can be silenced by category rather than by matching its message. - Counting is off by default even inside debug mode, because a warning changes what a program prints. - A block whose body raised is never warned about, since nothing had the chance to read it. - - Debug mode is not for production. - Every provider entered reads the stack and writes to the ledger, while a lookup that hits costs what it costs with debug mode off. - ``unused=True`` puts a counting registry in front of every read on top of that, which is roughly three times a plain hit. - - :ref:`howto-find-out-why-the-context-is-missing` runs all of it on a live program. - explain ------- diff --git a/src/nodrill/__main__.py b/src/nodrill/__main__.py index d650ac1..81e3010 100644 --- a/src/nodrill/__main__.py +++ b/src/nodrill/__main__.py @@ -2,10 +2,11 @@ Not a console script, so nothing lands on a PATH and the package still declares none, which leaves adding one later possible and removing one never -necessary. No __name__ guard, since a __main__ module is only ever run as -one and a guard would be a branch nothing can take the other way. +necessary. Guarded, since importing a module must not exit the process that +imported it, and a package walker imports this one like any other. """ from ._audit import main -raise SystemExit(main()) +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/src/nodrill/_audit.py b/src/nodrill/_audit.py index cf490e1..a5280b1 100644 --- a/src/nodrill/_audit.py +++ b/src/nodrill/_audit.py @@ -14,13 +14,14 @@ from __future__ import annotations -import argparse import os import sys import time import uuid from pathlib import Path +from ._errors import _NO_ENTRY, _counted + _Reads = set[tuple[str, str, str]] # The one format, carried by a shard and by a contract alike, so there is one reader. @@ -28,12 +29,16 @@ _SUFFIX = ".shard" # A tab, because repr escapes one and a key may legally hold two spaces in a row. _GAP = "\t" -# Written and diffed on machines nobody here chose, so the encoding is named rather than guessed. +# Written and diffed on machines nobody here chose, so neither the encoding nor the line ending +# is left to the platform. _ENCODING = "utf-8" -# The verbs, in the order a reader cares about them, since anything but requires is worth a look. -_VERBS = ("requires", "set_default", "default") -# Named here rather than in _debug, so one module owns the spelling of both variables. +_NEWLINE = "\n" +# The vocabulary a fact is written in, which _parse refuses a line outside of. +_VERBS = frozenset({"requires", "set_default", "default"}) +# Named here because _audit owns the file, where NODRILL_CONTRACT is named in _debug because +# it decides whether this module is imported at all. _ENTRY_VAR = "NODRILL_CONTRACT_ENTRY" +_RUN_VAR = "NODRILL_CONTRACT_RUN" def _declared(value: str) -> frozenset[str]: @@ -50,86 +55,100 @@ def _new_run() -> str: return f"{time.time_ns()}-{uuid.uuid4().hex}" -def _fact(read: tuple[str, str, str]) -> str: - """Render one recorded fact as the file's one line shape.""" - return _GAP.join(read) - - def _render(reads: _Reads) -> str: """Render a contract, sorted so the file is a property of the run and not of its order.""" - lines = [_HEADER, *(_fact(read) for read in sorted(reads))] - return "".join(f"{line}\n" for line in lines) + lines = [_HEADER, *(_GAP.join(read) for read in sorted(reads))] + return "".join(f"{line}{_NEWLINE}" for line in lines) + + +def _refuse(source: str, saw: str, expected: str) -> ValueError: + """Build the one refusal, so a caller can say which file and what it expected.""" + return ValueError( + f"{source} is not a nodrill contract this version reads. {expected}, found {saw!r}" + ) def _parse(text: str, source: str) -> _Reads: - """Read a contract or a shard back, refusing a version this reader does not know.""" + """Read a contract or a shard back, refusing anything this reader does not know.""" lines = text.splitlines() if not lines or lines[0] != _HEADER: opening = lines[0] if lines else "an empty file" - raise ValueError( - f"{source} is not a nodrill contract this version reads. " - f"Expected {_HEADER!r} on the first line and found {opening!r}" - ) + raise _refuse(source, opening, f"Expected {_HEADER!r} on the first line") found: _Reads = set() - for line in lines[1:]: - entry, verb, key = line.split(_GAP) + for number, line in enumerate(lines[1:], start=2): + fields = line.split(_GAP) + if len(fields) != 3: # noqa: PLR2004 + raise _refuse(source, line, f"Expected three fields on line {number}") + entry, verb, key = fields + if verb not in _VERBS: + raise _refuse(source, verb, f"Expected one of {sorted(_VERBS)} on line {number}") found.add((entry, verb, key)) return found +def _write(target: Path, text: str) -> None: + """Write one file of the format, with nothing about it left to the platform.""" + target.write_text(text, encoding=_ENCODING, newline=_NEWLINE) + + def _dump(directory: str, run: str, reads: _Reads) -> None: """Write this process's records into its own shard of the run, then forget them. Forgetting is what makes a second call a no-op, which matters because a - multiprocessing worker is finalized as well as registered at exit. + multiprocessing worker is finalized as well as registered at exit. Taken + before the write, so a thread still recording during shutdown cannot + change the set the render is walking. """ if not reads: return + facts = set(reads) + reads.clear() target = Path(directory) - target.mkdir(parents=True, exist_ok=True) - # The run first so a merge can group by it, then pid and a token, since a pid is reused. + # The run first so a merge can group by it, then one token, since rpartition recovers the run. shard = target / f"{run}-{uuid.uuid4().hex}{_SUFFIX}" - shard.write_text(_render(reads), encoding=_ENCODING) - reads.clear() + try: + target.mkdir(parents=True, exist_ok=True) + _write(shard, _render(facts)) + except OSError as error: + # A message rather than a traceback out of an exit hook, which exits 0 either way. + sys.stderr.write(f"nodrill: cannot record to {directory}, {error.strerror}\n") -def _merge(directory: str) -> tuple[_Reads, int, int]: +def _merge(directory: Path) -> tuple[_Reads, int, int]: """Read the newest run in a directory, and say how many shards it left behind. A directory reused across runs holds both, and a contract built from yesterday's reads describes a program that no longer exists. """ - shards = sorted(Path(directory).glob(f"*{_SUFFIX}")) + shards = sorted(directory.glob(f"*{_SUFFIX}")) if not shards: return set(), 0, 0 - newest = max(shard.name.rpartition("-")[0] for shard in shards) - current = [shard for shard in shards if shard.name.startswith(f"{newest}-")] + runs: dict[str, list[Path]] = {} + for shard in shards: + runs.setdefault(shard.name.rpartition("-")[0], []).append(shard) + # By when a run last wrote rather than by its id, since a run id may be one a CI system chose. + current = max(runs.values(), key=lambda group: max(shard.stat().st_mtime for shard in group)) found: _Reads = set() for shard in current: found |= _parse(shard.read_text(encoding=_ENCODING), str(shard)) return found, len(current), len(shards) - len(current) -def _counted(count: int, singular: str, plural: str) -> str: - """Render a count and its noun, since every figure below reads as a sentence.""" - return f"{count} {singular if count == 1 else plural}" - - def _summary(reads: _Reads, shards: int, stale: int) -> str: """Say what the contract rests on, since a guarantee that overstates itself is worse than none. The figures are what this stage can honestly own, which is what a run observed rather than what a tree contains. """ - entries = len({entry for entry, _, _ in reads}) + entries = len({entry for entry, _, _ in reads} - {_NO_ENTRY}) said = ( - f"nodrill: {_counted(len(reads), 'fact', 'facts')} under " - f"{_counted(entries, 'entry point', 'entry points')}, " + f"nodrill: {_counted(len(reads), 'fact')} under " + f"{_counted(entries, 'entry point')}, " f"recorded from {_counted(shards, 'process', 'processes')}. " f"A contract is only as complete as the run that recorded it." ) if stale: - said += f" {_counted(stale, 'shard', 'shards')} from an earlier run were left out." + said += f" {_counted(stale, 'shard')} from an earlier run were left out." return said @@ -138,45 +157,63 @@ def _unseen(reads: _Reads, declared: frozenset[str]) -> str | None: missing = sorted(declared - {entry for entry, _, _ in reads}) if not missing: return None - return f"nodrill: no block opened {', '.join(missing)}, named by NODRILL_CONTRACT_ENTRY" + return f"nodrill: no block opened {', '.join(missing)}, named by {_ENTRY_VAR}" + + +def _say(message: str) -> None: + """Put one diagnostic on standard error, so the artefact on standard output stays the file.""" + sys.stderr.write(f"{message}\n") def _contract(source: str, target: str | None, declared: frozenset[str]) -> int: """Render the contract a recorded run left, to a file or to stdout.""" directory = Path(source) if not directory.is_dir(): - sys.stderr.write(f"nodrill: nothing recorded at {source}, so there is no contract\n") + _say(f"nodrill: nothing recorded at {source}, so there is no contract") + return 1 + try: + reads, shards, stale = _merge(directory) + except (OSError, ValueError) as error: + _say(f"nodrill: cannot read the run at {source}, {error}") return 1 - reads, shards, stale = _merge(source) if not shards: - sys.stderr.write( + _say( f"nodrill: {source} holds no shards, so nothing armed the recorder. " - f"Run the suite with NODRILL_CONTRACT={source} first\n" + f"Run the suite with NODRILL_CONTRACT={source} first" ) return 1 text = _render(reads) if target is None: - sys.stdout.write(text) + # Through the buffer, so neither the locale nor the platform edits the artefact. + sys.stdout.flush() + sys.stdout.buffer.write(text.encode(_ENCODING)) + sys.stdout.buffer.flush() else: try: - Path(target).write_text(text, encoding=_ENCODING) + _write(Path(target), text) except OSError as error: - sys.stderr.write(f"nodrill: cannot write {target}, {error.strerror}\n") + _say(f"nodrill: cannot write {target}, {error.strerror}") return 1 - sys.stderr.write(f"{_summary(reads, shards, stale)}\n") + _say(_summary(reads, shards, stale)) unseen = _unseen(reads, declared) if unseen is not None: - sys.stderr.write(f"{unseen}\n") + _say(unseen) return 0 def main(argv: list[str] | None = None) -> int: """Run one subcommand and return the code the interpreter should exit with.""" + # Deferred, so arming a process does not pay for the command line it will never run. + import argparse # noqa: PLC0415 + + from . import __version__ # noqa: PLC0415 + parser = argparse.ArgumentParser( prog="python -m nodrill", description="Record and review what each entry point reads out of the context.", allow_abbrev=False, ) + parser.add_argument("--version", action="version", version=f"nodrill {__version__}") commands = parser.add_subparsers(dest="command", required=True) contract = commands.add_parser( "contract", diff --git a/src/nodrill/_core.py b/src/nodrill/_core.py index 96df460..90ed1fe 100644 --- a/src/nodrill/_core.py +++ b/src/nodrill/_core.py @@ -119,17 +119,20 @@ def _repaired( """ repaired = dict(current) key = leaving._key # noqa: SLF001 + restored: tuple[str | type[Any], dict[str | type[Any], Any]] | None = None # Innermost first, so the first block still open under the key is the one that owns it now. for open_block in reversed(chain): entered = open_block._entered # noqa: SLF001 if open_block is not leaving and entered is not None and open_block._key == key: # noqa: SLF001 - # Read with get, since a subscript is what the audit counts as a consumer read. - repaired[key] = entered.get(key) + # Unhooked, since the instrumentation counts a subscript as a read a consumer made. + repaired[key] = dict.__getitem__(entered, key) + restored = (key, entered) break else: repaired.pop(key, None) - repaired[_Open] = tuple(block for block in chain if block is not leaving) - return _reinstrument(repaired, current) + surviving = tuple(block for block in chain if block is not leaving) + repaired[_Open] = surviving + return _reinstrument(repaired, current, surviving, restored=restored) class _Provider(Generic[T]): @@ -635,6 +638,12 @@ def use(key: Any, *, default: Any = _MISSING) -> Any: return _resolve_miss(key, default) +def _open_chain() -> tuple[_Provider[Any], ...]: + """Return the blocks open right now, unhooked so asking is not itself a read.""" + chain: tuple[_Provider[Any], ...] = dict.get(_registry_get(), _Open, ()) + return chain + + def _resolve_miss(key: Any, default: Any = _MISSING) -> Any: """Finish a lookup that missed the registry. @@ -652,10 +661,10 @@ def _resolve_miss(key: Any, default: Any = _MISSING) -> Any: raise TypeError( f"use() received what lazy() returned, which is a target rather than a key. " f"Open it with provider(lazy({name}, factory)) and read it with use({name})" - ) from None + ) raise TypeError( f"use() expects a string name or a class, got {type(target).__name__}: {target!r}" - ) from None + ) if isinstance(target, type): factory = _defaults.get(target) if factory is not None: @@ -663,21 +672,19 @@ def _resolve_miss(key: Any, default: Any = _MISSING) -> Any: if _pending or target in _fired: _note_fallback(target) if _debug_state.auditing: - _record_fallback(_registry_get(), target, "set_default") + _record_fallback(target, "set_default", _open_chain()) return factory() if default is not _MISSING: if _debug_state.auditing: - _record_fallback(_registry_get(), target, "default") + _record_fallback(target, "default", _open_chain()) return default # The resolved target, since that is what a provider registered under. recording = _debug_state.recording diagnosis = _diagnose(target) if recording else None - available = [k for k in _registry.get() if k is not _Open] - # from None because the @inject wrapper calls this inside its own except KeyError, - # where use() calls it outside one, and a caller must not see that difference. + available = [k for k in _registry_get() if k is not _Open] raise NoProviderError( key, available, diagnosis, provided_by=_expected_at(target), offer_debug=not recording - ) from None + ) def active() -> Mapping[str | type[Any], Any]: @@ -689,7 +696,7 @@ def active() -> Mapping[str | type[Any], Any]: """ registry = _registry.get() if _Open in registry: - # A counting registry always carries _Open, so the filter is also the uncounting copy. + # An instrumented registry always carries _Open, so the filter is also the plain copy. registry = {key: value for key, value in registry.items() if key is not _Open} return MappingProxyType(registry) diff --git a/src/nodrill/_debug.py b/src/nodrill/_debug.py index 0987dea..167574c 100644 --- a/src/nodrill/_debug.py +++ b/src/nodrill/_debug.py @@ -9,6 +9,10 @@ a module-level ledger, and a miss reads the ledger to report a cause. A ContextVar could not hold it, since one would only ever show the scopes this frame already sees. + +The contract recorder rides the same instrumentation. NODRILL_CONTRACT arms +it at import, every read under an entry point becomes a fact, and the process +writes its shard at exit through _audit. """ from __future__ import annotations @@ -17,15 +21,17 @@ import inspect import itertools import os +import sys import threading import warnings -from collections.abc import MutableMapping +from collections.abc import Iterable, Iterator, MutableMapping +from contextlib import contextmanager from types import TracebackType from typing import Any, NamedTuple from weakref import WeakKeyDictionary from ._declare import _report_lines -from ._errors import UnusedProviderWarning, _describe_key, _Key, _key_path +from ._errors import _NO_ENTRY, UnusedProviderWarning, _counted, _describe_key, _Key, _key_path from ._refs import _key_target _Registry = dict[_Key, Any] @@ -89,7 +95,16 @@ class _State: watching is recording or auditing, so that path still tests one thing. """ - __slots__ = ("auditing", "counting", "depth", "recording", "seq", "unused_depth", "watching") + __slots__ = ( + "auditing", + "counting", + "depth", + "reads_full", + "recording", + "seq", + "unused_depth", + "watching", + ) def __init__(self) -> None: self.depth = 0 @@ -98,8 +113,13 @@ def __init__(self) -> None: self.counting = False self.auditing = False self.watching = False + self.reads_full = False self.seq = 0 + def watch(self) -> None: + """Restate what the provider path tests, so the two flags behind it live in one place.""" + self.watching = self.recording or self.auditing + _state = _State() @@ -111,13 +131,43 @@ def __init__(self) -> None: # Keys the cap above dropped, which a miss reports as gone rather than as absent. _forgotten: dict[_Key, None] = {} -# What the audit accumulates, uncapped and never rolled back, since a run is the unit. +# Never rolled back, since a run is the unit, and capped since an entry point may carry data. _reads: set[tuple[str, str, str]] = set() -# The entry point of a read no provider block encloses, which only a fallback can be. -_NO_ENTRY = "(none)" +# High enough that no honest run reaches it, low enough to stay an answer rather than a heap. +_READS_LIMIT = 100_000 # Keys NODRILL_CONTRACT_ENTRY names as boundaries, which mint a label even when nested. _declared_entries: set[str] = set() + +def _note(fact: tuple[str, str, str]) -> None: + """Record one fact, and say once when a run stopped being one the contract can rest on.""" + if fact in _reads: + return + if len(_reads) >= _READS_LIMIT: + if not _state.reads_full: + _state.reads_full = True + sys.stderr.write( + f"nodrill: {_READS_LIMIT} facts recorded, so this run stopped recording. " + f"An entry point carrying a request id mints one per request, and " + f"NODRILL_CONTRACT_ENTRY names the block that is the boundary\n" + ) + return + _reads.add(fact) + + +def _entry_for(chain: tuple[Any, ...]) -> str: + """Return the entry point a chain of open blocks answers to. + + A declared boundary wins over the block above it, innermost first, and an + empty chain is no entry point at all rather than the last one to close. + """ + labels = [_key_path(_key_target(block._key)) for block in chain] # noqa: SLF001 + for label in reversed(labels): + if label in _declared_entries: + return label + return labels[0] if labels else _NO_ENTRY + + # Serials rather than id(), which the interpreter hands on as soon as a task dies. _task_serials: WeakKeyDictionary[Any, int] = WeakKeyDictionary() _next_task_serial = itertools.count(1).__next__ @@ -126,7 +176,7 @@ def __init__(self) -> None: _from_env = os.environ.get("NODRILL_DEBUG", "") not in {"", "0"} _state.depth = 1 if _from_env else 0 _state.recording = _from_env -_state.watching = _from_env +_state.watch() def _arm(environ: MutableMapping[str, str]) -> None: @@ -137,28 +187,66 @@ def _arm(environ: MutableMapping[str, str]) -> None: Takes the mapping rather than reading os.environ, so what it sets can be tested without a child interpreter. """ + # Spelled here rather than imported, since it decides whether _audit is loaded at all. directory = environ.get("NODRILL_CONTRACT", "") if not directory: return _state.auditing = True - _state.watching = True + _state.watch() # Deferred, so a process that never audits pays for none of the tool's imports. - from multiprocessing.util import Finalize # noqa: PLC0415 + from multiprocessing.util import Finalize, register_after_fork # noqa: PLC0415 + from pathlib import Path # noqa: PLC0415 - from ._audit import _ENTRY_VAR, _declared, _dump, _new_run # noqa: PLC0415 + from ._audit import _ENTRY_VAR, _RUN_VAR, _declared, _dump, _new_run # noqa: PLC0415 + # Resolved now, since the hooks below run at exit and a program may have moved by then. + directory = str(Path(directory).resolve()) _declared_entries.update(_declared(environ.get(_ENTRY_VAR, ""))) - run = environ.get("NODRILL_CONTRACT_RUN") or _new_run() + run = environ.get(_RUN_VAR) or _new_run() # Written back so every child joins this run rather than starting one of its own. - environ["NODRILL_CONTRACT_RUN"] = run + environ[_RUN_VAR] = run + + def _finalize(_: object = None) -> None: + """Arrange the exit a worker takes when it never runs atexit.""" + Finalize(None, _dump, args=(directory, run, _reads), exitpriority=0) + atexit.register(_dump, directory, run, _reads) # A multiprocessing worker exits through os._exit, which runs finalizers and not atexit. - Finalize(None, _dump, args=(directory, run, _reads), exitpriority=0) + _finalize() + # A fork clears that registry before the worker body runs, so the child registers again. + register_after_fork(sys.modules[__name__], _finalize) _arm(os.environ) +@contextmanager +def _recording(entries: Iterable[str] = ()) -> Iterator[set[tuple[str, str, str]]]: + """Record a contract for the extent of a block, which nothing public does on purpose. + + Saved and restored rather than switched off at the end, since the process + may be recording for real. Meant for a test that wants to assert what a + handler read without spawning a child interpreter. + """ + saved = (_state.auditing, set(_reads), set(_declared_entries), _state.reads_full) + _reads.clear() + _declared_entries.clear() + _declared_entries.update(entries) + _state.auditing = True + _state.reads_full = False + _state.watch() + try: + yield _reads + finally: + _state.auditing = saved[0] + _state.reads_full = saved[3] + _state.watch() + _reads.clear() + _reads.update(saved[1]) + _declared_entries.clear() + _declared_entries.update(saved[2]) + + class _InstrumentedRegistry(dict[_Key, Any]): """Registry that watches lookups, for read counting and for the audit. @@ -184,10 +272,11 @@ def _mark(self, key: _Key) -> None: def __getitem__(self, key: Any) -> Any: # Typed loosely because this sees what a caller passed, not what the registry stores. value = super().__getitem__(key) - self._mark(key) + if self.owners: + self._mark(key) # A consumer read is a subscript, which is what leaves the chain key and a merge out. if _state.auditing: - _reads.add((self.entry, "requires", _key_path(_key_target(key)))) + _note((self.entry, "requires", _key_path(_key_target(key)))) return value def get(self, key: Any, default: Any = None) -> Any: @@ -195,26 +284,44 @@ def get(self, key: Any, default: Any = None) -> Any: value = super().get(key, _MISS) if value is _MISS: return default - self._mark(key) + if self.owners: + self._mark(key) return value -def _reinstrument(registry: _Registry, replaced: _Registry) -> _Registry: - """Return registry instrumented the way the mapping it replaces was.""" - if isinstance(replaced, _InstrumentedRegistry): - return _InstrumentedRegistry(registry, replaced.owners, replaced.entry) - return registry +def _reinstrument( + registry: _Registry, + replaced: _Registry, + chain: tuple[Any, ...], + *, + restored: tuple[_Key, _Registry] | None = None, +) -> _Registry: + """Return registry instrumented the way the mapping it replaces was. + + The label is derived from the chain rather than carried over, since a + repaired mapping outlives the block that minted it. A key restored from + a block still open is credited to that block, or the next read of it + would count for the block that just left. + """ + if not isinstance(replaced, _InstrumentedRegistry): + return registry + owners = replaced.owners + if restored is not None: + key, entered = restored + reads = entered.owners.get(key) if isinstance(entered, _InstrumentedRegistry) else None + if reads is not None: + owners = {**owners, key: reads} + return _InstrumentedRegistry(registry, owners, _entry_for(chain)) -def _record_fallback(registry: _Registry, key: _Key, source: str) -> None: +def _record_fallback(key: _Key, source: str, chain: tuple[Any, ...]) -> None: """Note a miss a registration answered, which is the read a raise would never report. A set_default factory and a use(key, default=...) both return before anything reports a miss, so a NoProviderError a registration is hiding would otherwise never appear in a contract. """ - entry = registry.entry if isinstance(registry, _InstrumentedRegistry) else _NO_ENTRY - _reads.add((entry, source, _key_path(key))) + _note((_entry_for(chain), source, _key_path(_key_target(key)))) def _user_site() -> tuple[_Site, int]: @@ -279,14 +386,13 @@ def _record_enter( handle = _state.seq _open[handle] = _Block(key, site, where, handle, reads) owners: dict[_Key, _Reads] = {} - minted = _key_path(key) - entry = minted + # Only the audit reads a label, and rendering a key is not free on the block path. + entry = _key_path(key) if _state.auditing else _NO_ENTRY if isinstance(enclosing, _InstrumentedRegistry): # Inherited whether or not counting is still on, since it is process-wide. owners = dict(enclosing.owners) - # Outermost comes from the open chain rather than from the kind of mapping inherited, - # since a repaired mapping outlives its chain and would hand on a label nothing owns. - if not outermost and minted not in _declared_entries: + # Outermost is read off the chain, since a repaired mapping outlives the one that made it. + if not outermost and entry not in _declared_entries: entry = enclosing.entry if reads is not None: owners[key] = reads @@ -447,7 +553,7 @@ def __enter__(self) -> None: with _lock: _state.depth += 1 _state.recording = True - _state.watching = True + _state.watch() if self._unused: _state.unused_depth += 1 _state.counting = True @@ -461,7 +567,7 @@ def __exit__( with _lock: _state.depth -= 1 _state.recording = _state.depth > 0 - _state.watching = _state.recording or _state.auditing + _state.watch() if self._unused: _state.unused_depth -= 1 _state.counting = _state.unused_depth > 0 @@ -521,7 +627,7 @@ def explain() -> str: blocks = sorted(_open.copy().values(), key=lambda entry: _listing(entry, here)) if not blocks: return "\n".join([*heading, "nodrill debug: no provider block is open."]) - counted = f"{len(blocks)} provider block{'' if len(blocks) == 1 else 's'}" + counted = _counted(len(blocks), "provider block") lines = [*heading, f"nodrill debug: {counted} open, innermost first within each thread."] lines += [ f" {_describe_key(entry.key)} opened at {entry.site.file}:{entry.site.line}, " diff --git a/src/nodrill/_declare.py b/src/nodrill/_declare.py index 057ba12..9789596 100644 --- a/src/nodrill/_declare.py +++ b/src/nodrill/_declare.py @@ -22,7 +22,7 @@ from types import MappingProxyType from typing import Any, Literal, TypeVar, overload -from ._errors import _KEY_TYPES, _describe_key, _Key +from ._errors import _KEY_TYPES, _counted, _describe_key, _Key from ._refs import _PENDING, _during_import, _is_ref, _Ref, _resolutions T = TypeVar("T") @@ -300,7 +300,7 @@ def _report_lines() -> list[str]: counts = sorted(dict(_fired).items(), key=lambda item: _describe_key(item[0])) lines = [ f"nodrill declare: the 'suspicious' fallback for {_describe_key(target)} has fired " - f"{count} time{'' if count == 1 else 's'}." + f"{_counted(count, 'time')}." for target, count in counts if count ] diff --git a/src/nodrill/_errors.py b/src/nodrill/_errors.py index 0383cc4..e2626d6 100644 --- a/src/nodrill/_errors.py +++ b/src/nodrill/_errors.py @@ -1,4 +1,4 @@ -"""Exceptions raised by nodrill.""" +"""Exceptions raised by nodrill, and the key vocabulary every message renders with.""" from __future__ import annotations @@ -13,6 +13,9 @@ # The same union for isinstance, as a tuple since `str | type` allocates a UnionType per evaluation. _KEY_TYPES = (str, type) +# What _key_path renders for no key at all, which only a read outside every block can be. +_NO_ENTRY = "(none)" + def _describe_key(key: Any) -> str: return repr(key) if isinstance(key, str) else getattr(key, "__qualname__", repr(key)) @@ -23,11 +26,21 @@ def _key_path(key: _Key) -> str: _describe_key renders a bare qualname, which reads well in a message and is ambiguous in a file that is diffed, since two Config classes in two - modules render identically. + modules render identically. Anything else falls back to its repr, since + instrumentation sees what a caller passed rather than what use() accepts. """ if isinstance(key, str): return repr(key) - return f"{key.__module__}:{key.__qualname__}" + module = getattr(key, "__module__", None) + qualname = getattr(key, "__qualname__", None) + if module is None or qualname is None: + return repr(key) + return f"{module}:{qualname}" + + +def _counted(count: int, singular: str, plural: str | None = None) -> str: + """Render a count and its noun, since a figure in a sentence needs to agree with it.""" + return f"{count} {singular if count == 1 else plural or singular + 's'}" def _rebuilt( diff --git a/src/nodrill/_inject.py b/src/nodrill/_inject.py index 5fe0c26..ebb8963 100644 --- a/src/nodrill/_inject.py +++ b/src/nodrill/_inject.py @@ -29,7 +29,7 @@ ) from ._core import _registry, _resolve_miss -from ._errors import _describe_key +from ._errors import _counted, _describe_key from ._refs import _is_ref, _KeyArg _T = TypeVar("_T") @@ -292,8 +292,8 @@ def _missing_error(label: str, values: tuple[tuple[str, Any], ...]) -> TypeError # Two names join with a bare "and". Three or more take the serial comma. separator = " and " if count == 2 else ", and " # noqa: PLR2004 listed = ", ".join(repr(n) for n in names[:-1]) + separator + repr(names[-1]) - plural = "s" if count > 1 else "" - return TypeError(f"{label}() missing {count} required positional argument{plural}: {listed}") + counted = _counted(count, "required positional argument") + return TypeError(f"{label}() missing {counted}: {listed}") def _reserved(name: str) -> bool: @@ -421,6 +421,9 @@ def _resolve_lines(target: str, key: str, ns: _WrapperSpace, indent: str) -> lis f"{indent}try:", f"{indent} {target} = {ns.registry}()[{key}]", f"{indent}except KeyError:", + f"{indent} {target} = {ns.omitted}", + # The miss runs after the handler, so nothing it raises is chained onto the lookup's own. + f"{indent}if {target} is {ns.omitted}:", f"{indent} {target} = {ns.miss}({key})", ] diff --git a/tests/test_audit.py b/tests/test_audit.py index 0460c7c..299c73a 100644 --- a/tests/test_audit.py +++ b/tests/test_audit.py @@ -4,6 +4,7 @@ import asyncio import atexit +import importlib import multiprocessing.util import os import runpy @@ -15,10 +16,10 @@ import pytest +import nodrill from nodrill import NoProviderError, provider, ref, set_default, use, wrap from nodrill._audit import ( _contract, - _counted, _declared, _dump, _merge, @@ -29,8 +30,17 @@ _unseen, main, ) -from nodrill._debug import _arm, _declared_entries, _reads, _state -from tests.audit_app.app import User, open_connection, run_job, run_report, running, serve_http +from nodrill._debug import _arm, _declared_entries, _reads, _recording, _state +from nodrill._errors import _counted +from tests.audit_app.app import ( + Settings, + User, + open_connection, + run_job, + run_report, + running, + serve_http, +) _ROOT = Path(__file__).parent.parent HEADER = "# nodrill contract 1" @@ -40,28 +50,16 @@ @pytest.fixture def recording() -> Iterator[set[tuple[str, str, str]]]: - """Turn the audit on for one test, which no public name does on purpose.""" - # Saved and restored rather than switched off, since a process may be recording for real. - saved = (_state.auditing, _state.watching, set(_reads), set(_declared_entries)) - _reads.clear() - _state.auditing = True - _state.watching = True - try: - yield _reads - finally: - _state.auditing, _state.watching = saved[0], saved[1] - _reads.clear() - _reads.update(saved[2]) - _declared_entries.clear() - _declared_entries.update(saved[3]) + """Turn the audit on for one test, through the seam the module owns.""" + with _recording() as reads: + yield reads @pytest.fixture -def declaring(recording: set[tuple[str, str, str]]) -> set[tuple[str, str, str]]: +def declaring() -> Iterator[set[tuple[str, str, str]]]: """Name the app's two boundaries the way NODRILL_CONTRACT_ENTRY does.""" - # The recording fixture restores the declared set, so this one only has to fill it. - _declared_entries.update({"'http request'", "'celery worker'"}) - return recording + with _recording({"'http request'", "'celery worker'"}) as reads: + yield reads @pytest.fixture @@ -69,9 +67,16 @@ def armed(recording: set[tuple[str, str, str]]) -> Iterator[list[tuple[Any, ...] """Collect what _arm registers, so a test never leaves a real hook on this process.""" calls: list[tuple[Any, ...]] = [] with pytest.MonkeyPatch.context() as patch: - patch.setattr(atexit, "register", lambda *call: calls.append(call)) + patch.setattr(atexit, "register", lambda *call: calls.append(("atexit", call))) + patch.setattr( + multiprocessing.util, + "Finalize", + lambda *call, **kw: calls.append(("finalize", (call, kw))), + ) patch.setattr( - multiprocessing.util, "Finalize", lambda *call, **kw: calls.append((call, kw)) + multiprocessing.util, + "register_after_fork", + lambda obj, func: calls.append(("after fork", (obj, func))), ) yield calls @@ -128,6 +133,27 @@ class Loose: use(Loose) assert _entries(recording) == {"(none)"} + def test_a_fallback_after_every_block_closed_has_no_entry_point(self, recording: Any) -> None: + """A repair leaves its mapping installed, and a dead boundary must not be blamed.""" + + class Loose: + pass + + set_default(Loose, Loose) + + def tenant(slug: str) -> Iterator[None]: + with provider("tenant", slug=slug): + yield + yield + + with provider("http request", route="/"): + first, second = tenant("acme"), tenant("globex") + list(zip(first, second, strict=False)) + list(first) + list(second) + use(Loose) + assert f"(none){TAB}set_default{TAB}" in _facts(recording).pop() + def test_two_classes_of_the_same_name_stay_apart(self, recording: Any) -> None: class User: # the point is that this collides with the app's User pass @@ -139,6 +165,51 @@ class User: # the point is that this collides with the app's User assert f"{APP}:User" not in next(iter(recorded)) +class TestWhatTheRecorderRefusesToCost: + """Instrumentation is passive, so it caps what it keeps and never raises on what it sees.""" + + def test_an_entry_point_carrying_data_stops_rather_than_growing_without_bound( + self, recording: Any, monkeypatch: pytest.MonkeyPatch, capsys: Any + ) -> None: + """A boundary keyed per request mints one entry point per request, which is unbounded.""" + monkeypatch.setattr("nodrill._debug._READS_LIMIT", 2) + for number in range(5): + with provider(f"request-{number}"), provider("db", dsn="x"): + use("db") + assert len(recording) == 2 + said = capsys.readouterr().err + assert "stopped recording" in said + assert "NODRILL_CONTRACT_ENTRY" in said + + def test_the_cap_says_so_once( + self, recording: Any, monkeypatch: pytest.MonkeyPatch, capsys: Any + ) -> None: + monkeypatch.setattr("nodrill._debug._READS_LIMIT", 1) + for number in range(4): + with provider(f"request-{number}"), provider("db", dsn="x"): + use("db") + assert capsys.readouterr().err.count("stopped recording") == 1 + + def test_a_key_the_recorder_did_not_expect_is_rendered_and_not_raised_on( + self, recording: Any + ) -> None: + """Turning the recorder on must not make a lookup that works in production raise.""" + + class Alias: + """Hashes and compares as the string key, which the registry answers on.""" + + def __hash__(self) -> int: + return hash("db") + + def __eq__(self, other: object) -> bool: + return other == "db" + + alias: Any = Alias() + with provider("http request"), provider("db", dsn="x"): + assert use(alias).dsn == "x" + assert any("Alias object at" in key for _, _, key in recording) + + class TestTheCollapseAndTheDeclaration: """A layer above the boundaries swallows them, which is why a boundary can be named.""" @@ -202,10 +273,38 @@ def test_a_miss_that_actually_raises_records_nothing(self, recording: Any) -> No @pytest.mark.parametrize("call", [lambda: use(User), open_connection], ids=["use", "inject"]) def test_a_miss_carries_no_internal_exception(self, call: Any) -> None: - """The wrapper resolves inside its own except KeyError and must not show it.""" + """The wrapper looks up in a try and must leave the handler before the miss runs.""" with pytest.raises(NoProviderError) as raised: call() - assert raised.value.__suppress_context__ + assert raised.value.__context__ is None + + @pytest.mark.parametrize( + "call", [lambda: use(Settings), open_connection], ids=["use", "inject"] + ) + def test_a_factory_that_raises_is_not_chained_onto_the_lookup(self, call: Any) -> None: + """A set_default factory runs on the miss path, and its failure is the whole story.""" + + def boom() -> Settings: + raise ValueError("the real failure") + + set_default(Settings, boom) + with pytest.raises(ValueError, match="the real failure") as raised: + call() + assert raised.value.__context__ is None + + def test_a_miss_keeps_the_exception_its_caller_was_handling(self) -> None: + """Suppressing every context would hide the error a cleanup path is recovering from.""" + + def cleanup() -> None: + try: + raise ValueError("the real failure") # noqa: TRY301 + except ValueError: + use(User) + + with pytest.raises(NoProviderError) as raised: + cleanup() + assert isinstance(raised.value.__context__, ValueError) + assert not raised.value.__suppress_context__ class TestTheLabelSurvivesTheAwkwardPaths: @@ -297,14 +396,34 @@ def test_arming_sets_the_switches_and_joins_a_run( assert "'a'" in _declared_entries # Written back so a child interpreter joins this run rather than starting one. assert environ["NODRILL_CONTRACT_RUN"] - # Both, since a pool worker exits through os._exit and never runs atexit. - assert len(armed) == 2 + # A pool worker exits through os._exit, and a fork clears what the parent registered. + assert [name for name, _ in armed] == ["atexit", "finalize", "after fork"] + + def test_a_forked_child_registers_the_finalizer_the_fork_cleared( + self, tmp_path: Path, armed: list[tuple[Any, ...]] + ) -> None: + """A fork clears the registry before a worker body runs, so the hook is registered again.""" + _arm({"NODRILL_CONTRACT": str(tmp_path)}) + [(_, (_, after_fork))] = [call for call in armed if call[0] == "after fork"] + armed.clear() + after_fork(None) + assert [name for name, _ in armed] == ["finalize"] + + def test_a_relative_directory_is_resolved_while_the_program_is_still_there( + self, tmp_path: Path, armed: list[tuple[Any, ...]], monkeypatch: pytest.MonkeyPatch + ) -> None: + """The hooks run at exit, by which time the program may have moved.""" + monkeypatch.chdir(tmp_path) + _arm({"NODRILL_CONTRACT": ".nodrill"}) + [(_, call)] = [entry for entry in armed if entry[0] == "atexit"] + assert Path(call[1]).is_absolute() def test_an_inherited_run_is_kept(self, tmp_path: Path, armed: list[tuple[Any, ...]]) -> None: environ = {"NODRILL_CONTRACT": str(tmp_path), "NODRILL_CONTRACT_RUN": "given"} _arm(environ) assert environ["NODRILL_CONTRACT_RUN"] == "given" - assert all("given" in repr(call) for call in armed) + registered = [call for name, call in armed if name in {"atexit", "finalize"}] + assert all("given" in repr(call) for call in registered) def test_a_run_with_the_switch_off_records_nothing( self, monkeypatch: pytest.MonkeyPatch @@ -364,6 +483,29 @@ def test_a_version_this_reader_does_not_know_is_refused(self, text: str, shown: _parse(text, "somewhere") assert shown in str(raised.value) + @pytest.mark.parametrize( + ("line", "expected"), + [ + (f"'a'{TAB}requires", "Expected three fields on line 2"), + (f"'a'{TAB}requires{TAB}x{TAB}y", "Expected three fields on line 2"), + (f"'a'{TAB}invented{TAB}x", "Expected one of"), + ], + ids=["too few", "too many", "a verb nothing writes"], + ) + def test_a_line_the_format_does_not_allow_is_refused(self, line: str, expected: str) -> None: + """A shard is a file on disk, so a half-written one has to be an answer and not a crash.""" + with pytest.raises(ValueError, match="not a nodrill contract this version reads") as raised: + _parse(f"{HEADER}\n{line}\n", "somewhere") + assert expected in str(raised.value) + + def test_the_line_ending_belongs_to_the_file_and_not_to_the_platform( + self, tmp_path: Path + ) -> None: + """The whole workflow is a diff, so the bytes cannot depend on who rendered them.""" + _dump(str(tmp_path), "run", {("'a'", "requires", "x")}) + [shard] = tmp_path.glob("*.shard") + assert b"\r" not in shard.read_bytes() + class TestShards: """One run is many processes, so the record is written per process and merged.""" @@ -375,7 +517,7 @@ def test_a_process_that_recorded_nothing_writes_no_shard(self, tmp_path: Path) - def test_a_shard_round_trips(self, tmp_path: Path) -> None: reads = {("'a'", "requires", "x"), ("'b'", "default", "y")} _dump(str(tmp_path), "run", set(reads)) - assert _merge(str(tmp_path)) == (reads, 1, 0) + assert _merge(tmp_path) == (reads, 1, 0) def test_dumping_twice_writes_one_shard(self, tmp_path: Path) -> None: """A pool worker is finalized as well as registered, so a second dump is a no-op.""" @@ -387,22 +529,49 @@ def test_dumping_twice_writes_one_shard(self, tmp_path: Path) -> None: def test_shards_from_several_processes_merge(self, tmp_path: Path) -> None: _dump(str(tmp_path), "run", {("'a'", "requires", "x")}) _dump(str(tmp_path), "run", {("'b'", "requires", "y")}) - found, shards, stale = _merge(str(tmp_path)) + found, shards, stale = _merge(tmp_path) assert found == {("'a'", "requires", "x"), ("'b'", "requires", "y")} assert (shards, stale) == (2, 0) def test_an_earlier_run_in_the_same_directory_is_left_out(self, tmp_path: Path) -> None: first, second = _new_run(), _new_run() _dump(str(tmp_path), first, {("'a'", "requires", "gone")}) + for shard in tmp_path.glob("*.shard"): + os.utime(shard, (0, 0)) _dump(str(tmp_path), second, {("'a'", "requires", "here")}) - assert _merge(str(tmp_path)) == ({("'a'", "requires", "here")}, 1, 1) + assert _merge(tmp_path) == ({("'a'", "requires", "here")}, 1, 1) + + def test_a_run_id_a_ci_system_chose_does_not_outrank_a_later_one(self, tmp_path: Path) -> None: + """A run id is inheritable, so it may be any string and cannot be ordered as a number.""" + _dump(str(tmp_path), "build-42", {("'a'", "requires", "gone")}) + for shard in tmp_path.glob("*.shard"): + os.utime(shard, (0, 0)) + _dump(str(tmp_path), _new_run(), {("'a'", "requires", "here")}) + assert _merge(tmp_path) == ({("'a'", "requires", "here")}, 1, 1) def test_an_empty_directory_merges_to_nothing(self, tmp_path: Path) -> None: - assert _merge(str(tmp_path)) == (set(), 0, 0) + assert _merge(tmp_path) == (set(), 0, 0) def test_a_run_id_is_unique(self) -> None: assert _new_run() != _new_run() + def test_a_directory_it_cannot_write_is_a_message_and_not_two_tracebacks( + self, tmp_path: Path, capsys: Any + ) -> None: + """The dump runs from an exit hook, where a raise is a traceback and never a failure.""" + blocked = tmp_path / "blocked" + blocked.write_text("not a directory", encoding="utf-8") + reads = {("'a'", "requires", "x")} + _dump(str(blocked), "run", reads) + assert "cannot record to" in capsys.readouterr().err + # Forgotten anyway, or the finalizer would reproduce the same failure a second time. + assert not reads + + def test_a_shard_the_reader_refuses_is_named(self, tmp_path: Path, capsys: Any) -> None: + (tmp_path / "1-x.shard").write_text("nonsense\n", encoding="utf-8") + assert _contract(str(tmp_path), None, frozenset()) == 1 + assert "cannot read the run at" in capsys.readouterr().err + class TestWhatTheToolAdmits: """A guarantee that overstates itself is worse than no guarantee.""" @@ -481,6 +650,13 @@ def test_a_flag_cannot_be_abbreviated(self, tmp_path: Path) -> None: with pytest.raises(SystemExit): main(["contract", "--fro", str(tmp_path)]) + def test_the_command_says_which_nodrill_wrote_a_contract(self, capsys: Any) -> None: + """A format the reader refuses is the moment the version is worth asking for.""" + with pytest.raises(SystemExit) as raised: + main(["--version"]) + assert raised.value.code == 0 + assert capsys.readouterr().out.strip() == f"nodrill {nodrill.__version__}" + def _child(program: str, directory: Path, entries: str = "") -> subprocess.CompletedProcess[str]: """Run a program in a child interpreter with the recorder armed.""" @@ -505,7 +681,7 @@ class TestARecordedRun: def test_the_environment_variable_arms_a_whole_process(self, tmp_path: Path) -> None: program = f"from {APP} import running, serve_http\nwith running(): serve_http('ada')" _child(program, tmp_path) - reads, _, _ = _merge(str(tmp_path)) + reads, _, _ = _merge(tmp_path) assert f"{APP}:Settings{TAB}requires{TAB}{APP}:User" in _render(reads) def test_declaring_the_boundaries_splits_the_entry_points(self, tmp_path: Path) -> None: @@ -515,7 +691,7 @@ def test_declaring_the_boundaries_splits_the_entry_points(self, tmp_path: Path) tmp_path, entries="'http request','celery worker'", ) - reads, _, _ = _merge(str(tmp_path)) + reads, _, _ = _merge(tmp_path) assert _entries(reads) == {"'http request'", "'celery worker'"} def test_a_subprocess_the_run_spawns_joins_the_same_run(self, tmp_path: Path) -> None: @@ -527,7 +703,7 @@ def test_a_subprocess_the_run_spawns_joins_the_same_run(self, tmp_path: Path) -> f" 'from {APP} import run_job; run_job(\"grace\")'], check=True)\n" ) _child(program, tmp_path) - reads, shards, stale = _merge(str(tmp_path)) + reads, shards, stale = _merge(tmp_path) assert (shards, stale) == (2, 0) assert f"'celery worker'{TAB}set_default{TAB}{APP}:Origin" in _render(reads) @@ -541,7 +717,7 @@ def test_a_process_pool_worker_records_its_own_shard(self, tmp_path: Path) -> No " pool.submit(run_job, 'grace').result()\n" ) _child(program, tmp_path) - reads, _, _ = _merge(str(tmp_path)) + reads, _, _ = _merge(tmp_path) assert f"'celery worker'{TAB}requires{TAB}{APP}:User" in _render(reads) def test_two_runs_of_the_same_program_agree_byte_for_byte(self, tmp_path: Path) -> None: @@ -549,7 +725,7 @@ def test_two_runs_of_the_same_program_agree_byte_for_byte(self, tmp_path: Path) first, second = tmp_path / "first", tmp_path / "second" _child(program, first) _child(program, second) - assert _render(_merge(str(first))[0]) == _render(_merge(str(second))[0]) + assert _render(_merge(first)[0]) == _render(_merge(second)[0]) def test_the_module_runs_as_a_command(self, tmp_path: Path) -> None: program = f"from {APP} import running, serve_http\nwith running(): serve_http('ada')" @@ -573,3 +749,11 @@ def test_the_dispatch_exits_with_what_the_command_returned( runpy.run_module("nodrill", run_name="__main__") assert raised.value.code == 1 assert "nothing recorded" in capsys.readouterr().err + + def test_importing_the_dispatch_does_not_exit_the_process_that_imported_it(self) -> None: + """A package walker imports every submodule, and __main__ is one of them.""" + # Popped again, so the next runpy of it starts from source the way a command line does. + try: + assert importlib.import_module("nodrill.__main__").main is main + finally: + sys.modules.pop("nodrill.__main__", None) diff --git a/tests/test_inject_codegen.py b/tests/test_inject_codegen.py index 5468d26..2c86c6a 100644 --- a/tests/test_inject_codegen.py +++ b/tests/test_inject_codegen.py @@ -163,6 +163,8 @@ def handler(request: str, db: FromCtx[Db] = injected) -> str: " try:\n" " db = _nd_registry()[_nd_key_db]\n" " except KeyError:\n" + " db = _nd_omitted\n" + " if db is _nd_omitted:\n" " db = _nd_miss(_nd_key_db)\n" " return _nd_func(request, db)" ) @@ -179,6 +181,8 @@ def handler(dsn: Annotated[str, from_ctx(ref(f"{__name__}:Db"))] = injected) -> " try:\n" " _nd_value = _nd_registry()[_nd_key_dsn]\n" " except KeyError:\n" + " _nd_value = _nd_omitted\n" + " if _nd_value is _nd_omitted:\n" " _nd_value = _nd_miss(_nd_key_dsn)\n" " dsn = _nd_ref_attr(_nd_key_dsn, _nd_value, 'dsn')\n" " return _nd_func(dsn)" @@ -198,6 +202,8 @@ def handler(db: FromCtx[Db], tag: str) -> str: " try:\n" " db = _nd_registry()[_nd_key_db]\n" " except KeyError:\n" + " db = _nd_omitted\n" + " if db is _nd_omitted:\n" " db = _nd_miss(_nd_key_db)\n" " return _nd_func(db, tag)" ) @@ -217,6 +223,8 @@ def render(user: str, lang: str = "en") -> str: " try:\n" " _nd_source = _nd_registry()[_nd_from_key]\n" " except KeyError:\n" + " _nd_source = _nd_omitted\n" + " if _nd_source is _nd_omitted:\n" " _nd_source = _nd_miss(_nd_from_key)\n" " user = _nd_getattr(_nd_source, 'user', _nd_omitted)\n" " if user is _nd_omitted:\n" @@ -226,6 +234,8 @@ def render(user: str, lang: str = "en") -> str: " try:\n" " _nd_source = _nd_registry()[_nd_from_key]\n" " except KeyError:\n" + " _nd_source = _nd_omitted\n" + " if _nd_source is _nd_omitted:\n" " _nd_source = _nd_miss(_nd_from_key)\n" " lang = _nd_getattr(_nd_source, 'lang', _nd_omitted)\n" " if lang is _nd_omitted:\n" From f70fb9a85e1d90ed06e7baa77b626682a733d91f Mon Sep 17 00:00:00 2001 From: Pavel Kutsenko Date: Mon, 24 Aug 2026 02:00:28 +0300 Subject: [PATCH 04/12] perf: measure the benchmark table over passes, not one row at a time Three runs of bench.py on an unchanged tree moved all fifteen rows, by up to 72 percent, and moved the ratio column on fourteen of them. Each row was timed to completion before the next one started, so a row was hostage to whatever the machine did during its own second, and the baseline every ratio divides by is one of those rows. The whole table is timed five times over now and every row keeps its best pass, which is the right estimator because noise only ever adds time. The loop count each row settles on in pass one is reused, so the run costs what it did. On the same three back-to-back runs the worst spread is 14 percent and most rows are inside 3. Writing is what the page actually needed. A row is replaced only when it moved further than a rerun moves it, so --write on an unchanged tree says so and writes nothing, and a diff of that page means a real change rather than the weather on the machine that ran it. The ratios derive from the numbers the page carries, so the two columns agree and the table reproduces itself. The @inject row moved for a real reason, since taking the miss path outside the handler costs the identity test back. It is 65 ns against 61, still well under the 37 the get-plus-sentinel spelling cost. --- benchmarks/bench.py | 125 ++++++++++++++++++++++++------ docs/content/misc/performance.rst | 32 ++++---- 2 files changed, 120 insertions(+), 37 deletions(-) diff --git a/benchmarks/bench.py b/benchmarks/bench.py index 10327e8..f7f7856 100644 --- a/benchmarks/bench.py +++ b/benchmarks/bench.py @@ -10,15 +10,23 @@ prose claims: entering a scope, entering it with a stack already open, and crossing into a thread. -Absolute nanoseconds move with the machine and a rerun lands within a few -percent; the ratios are the part worth reading. Nothing here runs in CI: -timing on a shared runner measures the runner. +Absolute nanoseconds move with the machine, so the whole table is timed +several times over and every row keeps its own best pass. Timing one row to +completion before starting the next made each row hostage to whatever the +machine did during its own second, which moved the ratios as well, since the +baseline every ratio divides by is one of the rows. + +A published number is only replaced when it moved further than a rerun +moves it, so running this on an unchanged tree writes nothing and a diff +means a real change. Nothing here runs in CI, because timing on a shared +runner measures the runner. """ from __future__ import annotations import argparse import platform +import re import sys import timeit from collections.abc import Mapping, Sequence @@ -40,6 +48,12 @@ # What a request scope carries by the time the layers are done accumulating. NAMESPACE_WIDTH = 8 +# Enough passes that a row unlucky in one of them is measured fairly in another. +PASSES = 5 + +# What a rerun moves a row by, below which the published number is left alone. +NOISE = 0.08 + @dataclass class Config: @@ -131,17 +145,36 @@ def noop() -> None: ) -def measure(statement: str, namespace: dict[str, object]) -> float: - """Return nanoseconds per loop for statement, letting timeit pick the count.""" +def measure( + label: str, statement: str, namespace: dict[str, object], loops: dict[str, int] +) -> float: + """Return nanoseconds per loop, reusing the loop count this row settled on in pass one.""" timer = timeit.Timer(statement, globals=namespace) - loops, total = timer.autorange() - # Best of five rather than one mean, since a single run trails whatever the machine did. - best = min(timer.repeat(repeat=5, number=loops)) - return min(total, best) / loops * 1e9 - - -def run() -> dict[str, float]: - """Time every case, each under the context its row describes.""" + count = loops.get(label) + if count is None: + count, total = timer.autorange() + loops[label] = count + return total / count * 1e9 + return timer.timeit(count) / count * 1e9 + + +def best_of(passes: int) -> dict[str, float]: + """Time the whole table repeatedly and keep each row's best pass. + + Noise only ever adds time, so the minimum is the estimate, and taking it + across passes rather than within one row is what stops a row that was + timed during a bad second from being the published number. + """ + loops: dict[str, int] = {} + best: dict[str, float] = {} + for _ in range(passes): + for label, timing in run(loops).items(): + best[label] = min(best.get(label, timing), timing) + return best + + +def run(loops: dict[str, int]) -> dict[str, float]: + """Time every case once, each under the context its row describes.""" config = Config() reference: ContextVar[Config] = ContextVar("reference") reference.set(config) @@ -149,29 +182,29 @@ def run() -> dict[str, float]: with provider(config): bound = wrap(noop) namespace = {**globals(), **locals()} - timings = {label: measure(statement, namespace) for label, statement in PROVIDED} + timings = {label: measure(label, stmt, namespace, loops) for label, stmt in PROVIDED} # The frozen row reuses read_used, so its delta is the proxy and nothing else. with provider(config, frozen=True): - timings[FROZEN] = measure("read_used('r')", {**globals(), **locals()}) + timings[FROZEN] = measure(FROZEN, "read_used('r')", {**globals(), **locals()}, loops) # The sealed row is the same read again, so its delta is the liveness check and nothing else. with provider(config, sealed=True): - timings[SEALED] = measure("read_used('r')", {**globals(), **locals()}) + timings[SEALED] = measure(SEALED, "read_used('r')", {**globals(), **locals()}, loops) # And the lazy row prices the cell after the first read has already resolved it. with provider(lazy(Config, Config)): - timings[LAZY] = measure("read_used('r')", {**globals(), **locals()}) + timings[LAZY] = measure(LAZY, "read_used('r')", {**globals(), **locals()}, loops) # An extending layer copies the enclosing namespace too, so it is priced over a full one. with provider("scope", **{f"field{i}": i for i in range(NAMESPACE_WIDTH)}): - timings[EXTEND] = measure(EXTEND_STATEMENT, {**globals(), **locals()}) + timings[EXTEND] = measure(EXTEND, EXTEND_STATEMENT, {**globals(), **locals()}, loops) # Entering copies the registry, so the claim that the copy scales with depth is priced here. with ExitStack() as stack: for layer in range(STACK_DEPTH): stack.enter_context(provider(f"layer{layer}")) - timings[STACKED] = measure(ENTER_STATEMENT, {**globals(), **locals()}) + timings[STACKED] = measure(STACKED, ENTER_STATEMENT, {**globals(), **locals()}, loops) return timings @@ -190,7 +223,7 @@ def line(cells: tuple[str, str, str]) -> str: ).rstrip() lines = [rule, line(header), rule, *(line(row) for row in rows), rule] - return "\n".join(lines) + f"\n\n{stamp()}\n" + return "\n".join(lines) def stamp() -> str: @@ -207,6 +240,39 @@ def ratio(times: float) -> str: return f"{times:.1f}" if times < 10 else str(round(times)) # noqa: PLR2004 +# One row of the rendered table, which is how the page hands its numbers back. +ROW = re.compile(r"^(\S.*?)\s{2,}(\d+)\s{2,}[\d.]+$") + + +def carried(document: str) -> str: + """Return the table the page carries right now, rules included, so it can be compared.""" + start = document.index(START) + end = document.index(END, start) + block = document[start:end].splitlines() + rules = [number for number, line in enumerate(block) if line.startswith("==")] + return "\n".join(block[rules[0] : rules[-1] + 1]) + + +def published(table: str) -> dict[str, float]: + """Read the numbers the page already carries, so a rerun can leave them where they are.""" + found = (ROW.match(line) for line in table.splitlines()) + return {row[1]: float(row[2]) for row in found if row is not None and row[1] != "operation"} + + +def steadied(fresh: Mapping[str, float], old: Mapping[str, float]) -> dict[str, float]: + """Keep every published number a rerun would only have jittered. + + A row moves when it moved further than a rerun moves it, so the diff of + this page is a signal rather than the weather on the machine that ran it. + """ + kept = {} + for label, timing in fresh.items(): + was = old.get(label) + settled = was is not None and abs(timing - was) <= was * NOISE + kept[label] = was if settled else timing + return kept + + def splice(document: str, table: str) -> str: """Return document with the region between the markers replaced by table.""" start = document.index(START) + len(START) @@ -220,20 +286,31 @@ def main(argv: Sequence[str] | None = None) -> int: parser.add_argument( "--write", action="store_true", - help="replace the table in the performance page instead of writing to stdout", + help="update the rows of the performance page that moved, instead of writing to stdout", ) args = parser.parse_args(argv) - table = render(run()) + timings = best_of(PASSES) if not args.write: - sys.stdout.write(table) + sys.stdout.write(f"{render(timings)}\n\n{stamp()}\n") return 0 document = PAGE.read_text(encoding="utf-8") if START not in document or END not in document: sys.stderr.write(f"{PAGE}: markers {START} and {END} not found\n") return 1 - PAGE.write_text(splice(document, table), encoding="utf-8") + was = carried(document) + old = published(was) + timings = steadied(timings, old) + table = render(timings) + if table == was: + sys.stderr.write(f"{PAGE}: every row is within {NOISE:.0%} of what it says, left alone\n") + return 0 + PAGE.write_text(splice(document, f"{table}\n\n{stamp()}\n"), encoding="utf-8") + moved = [label for label in ORDER if round(timings[label]) != old.get(label)] + sys.stderr.write(f"{PAGE}: rewrote {len(moved)} of {len(ORDER)} rows\n") + for label in moved: + sys.stderr.write(f" {old.get(label)} -> {round(timings[label])} {label}\n") return 0 diff --git a/docs/content/misc/performance.rst b/docs/content/misc/performance.rst index 463628f..0155fb4 100644 --- a/docs/content/misc/performance.rst +++ b/docs/content/misc/performance.rst @@ -19,27 +19,33 @@ The first rows are one function doing one read, reached six ways, so they can be operation ns × ================================================================ ==== === one read in a function, value passed in as a parameter 23 1.0 -the same read through `use()` 60 2.6 -the same read through `@inject` 61 2.6 -the same read through a `frozen=True` provider 114 4.9 -the same read through a `sealed=True` provider 121 5.2 -the same read through a resolved `lazy` provider 137 5.8 +the same read through `use()` 59 2.6 +the same read through `@inject` 65 2.8 +the same read through a `frozen=True` provider 117 5.1 +the same read through a `sealed=True` provider 124 5.4 +the same read through a resolved `lazy` provider 136 5.9 `use(Config)` on its own, without the call frame 42 1.8 -the same lookup through a `ref()` key 144 6.2 +the same lookup through a `ref()` key 143 6.2 bare `ContextVar.get()`, for reference 16 0.7 -`with provider(...)`, enter and exit 868 37 -the same with 8 providers already open 904 39 -`with provider(..., sealed=True)`, entered and exited 2386 102 -`with provider(lazy(...))`, entered and exited unread 1799 77 -`with provider(..., extend=True)`, over an 8-attribute namespace 1984 85 -`wrap(fn)()`, per call into a thread 539 23 +`with provider(...)`, enter and exit 843 37 +the same with 8 providers already open 881 38 +`with provider(..., sealed=True)`, entered and exited 2363 103 +`with provider(lazy(...))`, entered and exited unread 1799 78 +`with provider(..., extend=True)`, over an 8-attribute namespace 1954 85 +`wrap(fn)()`, per call into a thread 531 23 ================================================================ ==== === -CPython 3.14.5 on macOS 26.6, arm64, measured 2026-08-23. +CPython 3.14.5 on macOS 26.6, arm64, measured 2026-08-24. .. end benchmarks The ``×`` column is against handing the value in as a parameter, which is the alternative nodrill removes from the signatures in between. +It is the column to read, because it is the one that travels. + +Everything here is one thread doing one thing, so what the nanoseconds depend on is how fast one core is, and not how many there are. +A machine with more cores runs the same row at the same speed, and a server core is often slower at this than a laptop one, so a bigger machine is not a faster table. +What a quiet machine buys is a table that says the same thing twice, which is why the numbers are timed over several passes and a row is only republished when it moved further than a rerun moves it. +Your own figures will differ and the ratios between them should not, which is the part any claim below rests on. Reading through :func:`~nodrill.use` costs a little over the parameter it replaces. :func:`~nodrill.inject` costs more, because it fills the argument before the body runs. From 6e166ca2cce3394f82b7430ba582624ea0ca1903 Mon Sep 17 00:00:00 2001 From: Pavel Kutsenko Date: Mon, 24 Aug 2026 02:01:39 +0300 Subject: [PATCH 05/12] docs: cut the padding out of comments, docstrings and prose Twenty docstring paragraphs ran a line longer than they had to, ending on one or two words that the sentence above could have carried. _debug ended a paragraph on "look.", _declare on "avoid." and on "declared.", _portable on "inside.". Each is trimmed by a few words rather than rewritten. Two of them were worth restructuring instead. NoProviderError had "as attributes" stranded at the end, three lines from the "Carries" it belongs to, and annotate_exceptions was ten lines carrying four separate thoughts. Eighteen comments spanned two lines, against a rule that says one, and in almost all of them the second line held five or six words. The two in _frozen and _sealed were the same sentence twice, and the one in _core spent its first line restating what the line under it declares. Sixteen docstrings and comments carried a colon, a semicolon or an em dash. A colon was usually two thoughts glued together, so the versions without one are shorter. The same holds for the contributor docs, where CONTRIBUTING alone had six semicolons, five colons and four em dashes, and where two paragraphs put a second sentence on a line the semantic-line-break rule wants to itself. Also gone: a "Note that" docstring that said nothing the signature did not, four uses of "simply", and an em dash printed as a column separator in two examples. --- .github/CODE_OF_CONDUCT.md | 2 +- .github/CONTRIBUTING.md | 28 +++++++------- .github/PULL_REQUEST_TEMPLATE.md | 4 +- .github/SECURITY.md | 6 ++- README.md | 3 +- benchmarks/bench.py | 9 +++-- docs/conf.py | 15 +++----- .../howto/see-the-context-in-a-traceback.rst | 2 +- docs/content/misc/design.rst | 9 +++-- docs/content/ref/debugging.rst | 2 +- docs/content/ref/declaring.rst | 2 +- docs/content/ref/provider.rst | 2 +- docs/content/topics/declaring.rst | 2 +- src/nodrill/_ambient.py | 3 +- src/nodrill/_audit.py | 6 +-- src/nodrill/_concurrency.py | 14 +++---- src/nodrill/_core.py | 3 +- src/nodrill/_debug.py | 37 ++++++++----------- src/nodrill/_declare.py | 15 +++----- src/nodrill/_errors.py | 5 +-- src/nodrill/_frozen.py | 3 +- src/nodrill/_inject.py | 12 +++--- src/nodrill/_portable.py | 12 +++--- src/nodrill/_refs.py | 14 +++---- src/nodrill/_report.py | 21 ++++++----- src/nodrill/_sealed.py | 9 ++--- tests/cycle/__init__.py | 3 +- tests/cycle/at_import.py | 3 +- tests/test_debug.py | 4 +- tests/test_errors.py | 2 +- tests/test_inject_async.py | 2 +- tests/test_inject_binding.py | 10 ++--- tests/test_inject_codegen.py | 2 +- tests/test_lazy.py | 4 +- tests/test_lazy_hints.py | 2 +- tests/test_refs.py | 3 +- tests/test_sealed.py | 3 +- tests/test_sealed_protocols.py | 3 +- tests/test_threads.py | 2 +- 39 files changed, 126 insertions(+), 157 deletions(-) diff --git a/.github/CODE_OF_CONDUCT.md b/.github/CODE_OF_CONDUCT.md index ed2ca01..c9ed370 100644 --- a/.github/CODE_OF_CONDUCT.md +++ b/.github/CODE_OF_CONDUCT.md @@ -2,7 +2,7 @@ nodrill is not a [Python Software Foundation](https://www.python.org/psf-landing/) project, but everyone interacting in its issues, pull requests and discussions is expected to follow the [PSF Code of Conduct](https://policies.python.org/python.org/code-of-conduct/). -In short: be open, considerate and respectful, whatever anyone's position in the project is. +In short, be open, considerate and respectful, whatever anyone's position in the project is. ## Enforcement diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 072937a..0bf4a8c 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -16,23 +16,23 @@ make install ``` `make install` syncs the locked environment and installs the pre-commit hooks. -Everything else runs through the `Makefile`; `make help` lists the targets. +Everything else runs through the `Makefile`, and `make help` lists the targets. ## The gate ```bash -make # lint, typecheck, coverage, docs, workflow audit — the same order CI runs +make # lint, typecheck, coverage, docs, workflow audit, in the order CI runs them make -k # same, but keep going after a failure so one run reports everything ``` -Individual pieces, when you want a faster loop: +Individual pieces, for when you want a faster loop. | Command | What it does | | --- | --- | | `make format` | ruff format plus the safe ruff fixes | | `make lint` | ruff format `--check` and `ruff check` | | `make typecheck` | mypy and pyright | -| `make test` | pytest; `make test ARGS="-k inject -x"` to narrow it | +| `make test` | pytest, narrowed with `make test ARGS="-k inject -x"` | | `make testcov` | pytest under coverage with the 100 percent gate | | `make docs` | Sphinx with warnings as errors | | `make audit` | zizmor over the GitHub Actions workflows | @@ -44,8 +44,8 @@ A pull request is expected to pass all of it. - **Coverage is 100 percent on branches.** New code arrives with the tests that cover it. A `# pragma: no cover` is not the fix. -- **Two type checkers.** mypy runs strict over `src` and `tests`; pyright checks `src`. - Both must be clean — a few API shapes exist only because the two disagree. +- **Two type checkers.** mypy runs strict over `src` and `tests`, and pyright checks `src`. + Both must be clean, and a few API shapes exist only because the two disagree. - **Ruff with `select = ["ALL"]`.** A new ignore goes in `pyproject.toml` with a comment saying why, rather than a bare `# noqa` at the call site. - **Docs build with `-W`.** @@ -53,16 +53,16 @@ A pull request is expected to pass all of it. ## House style -Docstrings are plain PEP 257 prose: an imperative first line, no reStructuredText roles, no bullet lists. -The reference pages carry the detailed descriptions; docstrings stay terse. +Docstrings are plain PEP 257 prose, an imperative first line, no reStructuredText roles, no bullet lists. +The reference pages carry the detailed descriptions, so docstrings stay terse. Comments explain why, not what. -Prose in `.md` and `.rst` files uses semantic line breaks: a new sentence starts a new line, so rewording a paragraph shows up as a one-line diff. -There is no column limit: a sentence stays on one line however long it runs, and nothing is re-wrapped by hand. -`make lint-md` checks the rest of the Markdown — headings, lists, blank lines — but no tool can check the sentence rule, so that one rides on review. +Prose in `.md` and `.rst` files uses semantic line breaks, so a new sentence starts a new line and rewording a paragraph shows up as a one-line diff. +There is no column limit, so a sentence stays on one line however long it runs and nothing is re-wrapped by hand. +`make lint-md` checks the rest of the Markdown, headings and lists and blank lines, but no tool can check the sentence rule, so that one rides on review. -Public names are load-bearing: `provider`, `use`, `wrap`, `Executor`, `set_default` and `from_ctx` were reviewed and are fixed. -Propose a rename in an issue; please do not perform one in a pull request. +Public names are load-bearing, and `provider`, `use`, `wrap`, `Executor`, `set_default` and `from_ctx` were reviewed and are fixed. +Propose a rename in an issue rather than performing one in a pull request. ## Reporting a bug @@ -74,5 +74,5 @@ For a security issue, follow [SECURITY.md](SECURITY.md) instead of opening an is ## AI-assisted contributions Using an assistant to write a patch is fine. -Submitting one you have not read, run and understood is not: you are the author of the pull request, and review time is the scarce resource here. +Submitting one you have not read, run and understood is not, since you are the author of the pull request and review time is the scarce resource here. Say so in the description if a change was largely machine-generated, and make sure the tests genuinely exercise the behaviour rather than restating the implementation. diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 0419435..1728146 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -4,9 +4,9 @@ ## Checklist - + -- [ ] `make -k` passes: lint, mypy, pyright, 100 percent branch coverage, docs, workflow audit. +- [ ] `make -k` passes, meaning lint, mypy, pyright, 100 percent branch coverage, docs and the workflow audit. - [ ] There are tests for the new or changed behaviour. - [ ] Documentation is updated, including the reference page if the public API changed. - [ ] Prose uses semantic line breaks, one sentence per line. diff --git a/.github/SECURITY.md b/.github/SECURITY.md index 7972f47..cacfa17 100644 --- a/.github/SECURITY.md +++ b/.github/SECURITY.md @@ -5,12 +5,14 @@ Only the latest released version is supported. Fixes ship in a new release rather than as patches to older ones. -nodrill has no runtime dependencies and does not parse untrusted input, so its realistic security surface is narrow: values leaking across a context boundary they should not cross — between threads, between asyncio tasks, or out of a `provider` block that has exited. +nodrill has no runtime dependencies and does not parse untrusted input, so its realistic security surface is narrow. +It is a value leaking across a context boundary it should not cross, between threads, between asyncio tasks, or out of a `provider` block that has exited. Reports in that shape are treated as security issues, not ordinary bugs. ## Reporting a vulnerability -Report privately through GitHub: [open a draft security advisory](https://github.com/paqstd-dev/nodrill/security/advisories/new). Please do not open a public issue for a suspected vulnerability. +Report privately through GitHub by [opening a draft security advisory](https://github.com/paqstd-dev/nodrill/security/advisories/new). +Please do not open a public issue for a suspected vulnerability. Include the smallest program that reproduces the leak, the Python version, and whether threads or asyncio are involved. diff --git a/README.md b/README.md index d2818ba..0046032 100644 --- a/README.md +++ b/README.md @@ -94,7 +94,8 @@ Start with the [tutorial](https://nodrill.readthedocs.io/en/latest/content/intro ## Contributing -Bug reports and small focused pull requests are welcome. See [CONTRIBUTING.md](https://github.com/paqstd-dev/nodrill/blob/main/.github/CONTRIBUTING.md). +Bug reports and small focused pull requests are welcome. +See [CONTRIBUTING.md](https://github.com/paqstd-dev/nodrill/blob/main/.github/CONTRIBUTING.md). `make install` sets up the environment, and `make` runs the same gate CI does. Security issues go through a [private advisory](https://github.com/paqstd-dev/nodrill/security/advisories/new) rather than the issue tracker. diff --git a/benchmarks/bench.py b/benchmarks/bench.py index f7f7856..480e950 100644 --- a/benchmarks/bench.py +++ b/benchmarks/bench.py @@ -7,7 +7,7 @@ The first rows are one function doing one read, reached five ways, so the rows are comparable to each other and to handing the value in as a parameter, which is what nodrill replaces. The rest price the things the -prose claims: entering a scope, entering it with a stack already open, and +prose claims, entering a scope, entering it with a stack already open, and crossing into a thread. Absolute nanoseconds move with the machine, so the whole table is timed @@ -57,7 +57,7 @@ @dataclass class Config: - """The provided value; a dataclass because that is what callers use.""" + """The provided value, a dataclass because that is what callers use.""" dsn: str = "postgres://" @@ -106,7 +106,7 @@ def noop() -> None: # Handing the value in is the alternative nodrill replaces, so it is what the ratios divide by. BASELINE = PASSED -# The published order: the comparable reads, then the floor, then the scope costs. +# The published order, comparable reads first, then the floor, then the scope costs. ORDER = ( PASSED, USED, @@ -212,7 +212,8 @@ def run(loops: dict[str, int]) -> dict[str, float]: def render(timings: Mapping[str, float]) -> str: """Format timings as the reStructuredText block the performance page carries.""" base = timings[BASELINE] - header = ("operation", "ns", "×") # noqa: RUF001 — the sign is the published column heading + # The multiplication sign, since that is the heading the published table carries. + header = ("operation", "ns", "×") # noqa: RUF001 rows = [(label, f"{round(timings[label])}", ratio(timings[label] / base)) for label in ORDER] widths = [max(len(cell) for cell in column) for column in zip(header, *rows, strict=True)] rule = " ".join("=" * width for width in widths) diff --git a/docs/conf.py b/docs/conf.py index c16cfed..829b0dd 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -9,8 +9,7 @@ author = "Pavel Kutsenko" copyright = "2026, Pavel Kutsenko" -# nodrill is installed alongside the docs group, so the built docs always -# report the version they were built from. +# Installed alongside the docs group, so a build reports the version it was built from. release = package_version("nodrill") version = ".".join(release.split(".")[:2]) @@ -23,8 +22,7 @@ templates_path = ["_templates"] exclude_patterns = ["_build", ".DS_Store", "Thumbs.db"] -# Single backticks mean inline code, the way they read in every other file -# in the repo; broken explicit roles still fail the build under nitpicky. +# Single backticks are inline code here, as they read everywhere else in the repo. default_role = "literal" nitpicky = True @@ -48,15 +46,13 @@ html_favicon = "_static/img/favicon.svg" html_copy_source = False -# Read the Docs exports the canonical URL of the version being built, and a URL -# hardcoded here would point every version at latest. +# From Read the Docs, since a URL hardcoded here would point every version at latest. html_baseurl = os.environ.get("READTHEDOCS_CANONICAL_URL", "") html_theme_options = { "accent_color": "teal", "color_mode": "auto", - # The card GitHub shows for the repository, as an absolute URL a crawler can - # fetch. index.rst repeats it in :cover:, which asks for the large preview. + # Absolute, since a crawler fetches it, and index.rst repeats it for the large preview. "og_image_url": ( "https://raw.githubusercontent.com/paqstd-dev/nodrill/main" "/.github/assets/social-preview.png" @@ -78,6 +74,5 @@ ], } -# intersphinx already resolves every CPython target on each build, so linkcheck -# skips the host rather than re-requesting it and collecting HTTP 429s. +# Resolved by intersphinx on every build already, so re-requesting it only collects 429s. linkcheck_ignore = [r"https://docs\.python\.org/.*"] diff --git a/docs/content/howto/see-the-context-in-a-traceback.rst b/docs/content/howto/see-the-context-in-a-traceback.rst index d3b2693..1d475d1 100644 --- a/docs/content/howto/see-the-context-in-a-traceback.rst +++ b/docs/content/howto/see-the-context-in-a-traceback.rst @@ -78,7 +78,7 @@ A ``repr`` that raises an :exc:`Exception` is replaced by `` Callable[P, R]: """Bind fn to a snapshot of the context active when wrap() was called. - Each invocation runs under a fresh copy of the snapshot, so the result - is safe to call concurrently and callee writes stay local. The - snapshot is taken at wrap() time, so wrapping at import binds - import-time state. Async functions are rejected, because asyncio - propagates context itself. + Each invocation runs under a fresh copy of the snapshot, so the result is + safe to call concurrently and callee writes stay local. The snapshot is + taken at wrap() time, so wrapping at import binds import-time state. + Async functions are rejected, because asyncio propagates context itself. """ if inspect.iscoroutinefunction(fn) or inspect.isasyncgenfunction(fn): raise TypeError( @@ -50,9 +49,8 @@ def restore_and_call() -> R: class Executor(ThreadPoolExecutor): """ThreadPoolExecutor whose tasks see the submit-time context. - Each task runs under its own context copy, so worker-side writes never - leak between tasks or back to the submitter. map() inherits the - behavior via submit(). + Each task runs under its own context copy, so worker-side writes never leak + between tasks or back to the submitter. map() inherits it through submit(). """ def submit(self, fn: Callable[P, R], /, *args: P.args, **kwargs: P.kwargs) -> Future[R]: diff --git a/src/nodrill/_core.py b/src/nodrill/_core.py index 90ed1fe..369357c 100644 --- a/src/nodrill/_core.py +++ b/src/nodrill/_core.py @@ -394,8 +394,7 @@ class _Sealing: _sealed = True - # What the mixin reads off whichever provider it sits in front of, declared because - # a self typed as that host would leave super() with nothing to resolve against. + # Declared here, since a self typed as the host provider leaves super() nothing to resolve. _scope: _Scope _token: Token[dict[str | type[Any], Any]] | None diff --git a/src/nodrill/_debug.py b/src/nodrill/_debug.py index 167574c..81d0057 100644 --- a/src/nodrill/_debug.py +++ b/src/nodrill/_debug.py @@ -2,13 +2,11 @@ A miss usually means the provider is open somewhere this frame cannot see, because the call crossed a boundary that does not carry context. The -evidence for that sits in another context, which is where a lookup cannot -look. +evidence sits in another context, which is where a lookup cannot look. -While debug mode is on, every provider block records where it was entered in -a module-level ledger, and a miss reads the ledger to report a cause. A -ContextVar could not hold it, since one would only ever show the scopes this -frame already sees. +While debug mode is on, every provider block records where it was entered +in a module-level ledger that a miss reads to report a cause. A ContextVar +could not hold it, since one shows only the scopes this frame already sees. The contract recorder rides the same instrumentation. NODRILL_CONTRACT arms it at import, every read under an entry point becomes a fact, and the process @@ -264,7 +262,7 @@ def __init__(self, registry: _Registry, owners: dict[_Key, _Reads], entry: str) self.entry = entry def _mark(self, key: _Key) -> None: - """Note that something read the block providing key.""" + """Mark the block providing key as read.""" reads = self.owners.get(key) if reads is not None: reads.hit = True @@ -327,8 +325,7 @@ def _record_fallback(key: _Key, source: str, chain: tuple[Any, ...]) -> None: def _user_site() -> tuple[_Site, int]: """Return the innermost site outside this package, and how far up it is. - The distance is the stacklevel warnings.warn() wants, counted from the - caller. + The distance is the stacklevel warnings.warn() wants, counted from the caller. """ frame = inspect.currentframe() levels = 0 @@ -370,10 +367,9 @@ def _record_enter( ) -> tuple[int | None, _Registry]: """Note an entered provider block, and return its handle with the registry to install. - The handle is the block's serial, which the provider holds until it - exits, and it is None when only the audit is watching, since then the - ledger has nothing to forget. id() would be reused by the next provider - at that address. + The handle is the block's serial, which the provider holds until it exits, + and it is None when only the audit is watching, since the ledger then has + nothing to forget. id() would be reused by whatever is allocated there next. """ handle: int | None = None reads: _Reads | None = None @@ -439,8 +435,8 @@ def _record_exit(handle: int, *, failed: bool) -> None: def _rank(entry: _Block, here: _Where) -> tuple[int, int, int]: """Order the ledger by how likely a block is to explain this frame's miss. - Nearest frame first, then a block still open over one that exited, then - the innermost. + Nearest frame first, then a block still open over one that exited, and the + innermost of those last. """ if entry.where.thread != here.thread: near = 2 @@ -604,12 +600,11 @@ def _codec_lines() -> list[str]: def explain() -> str: """Return a report of the provider blocks open right now, a thread at a time. - Written for a breakpoint, as print(nodrill.explain()). Blocks opened - on other threads and in other tasks are listed too, which is the reason - to read this rather than active(), and the reader's own thread comes - first with its own blocks innermost first. The codec and any suspicious - fallback that has fired are named above them, since nothing else in the - process reports either. + Written for a breakpoint, as print(nodrill.explain()). Blocks opened on + other threads and in other tasks are listed too, which is the reason to read + this rather than active(), and the reader's own thread comes first with its + own blocks innermost first. The codec and any suspicious fallback that has + fired are named above them, since nothing else in the process reports either. """ heading = [*_codec_lines(), *_report_lines()] if not _state.recording: diff --git a/src/nodrill/_declare.py b/src/nodrill/_declare.py index 9789596..e1c6067 100644 --- a/src/nodrill/_declare.py +++ b/src/nodrill/_declare.py @@ -50,8 +50,7 @@ class _Scan: """Remembers the resolution count the last pending scan saw. - A holder rather than a module global, so no writer needs a global - statement. + A holder rather than a module global, so no writer needs a global statement. """ __slots__ = ("at",) @@ -77,10 +76,9 @@ class Declaration: class _Pending: """A declaration waiting for its ref to resolve. - Carries the metadata unassembled, since the Declaration is built with - the resolved key, and eq=False keeps identity comparison, since - comparing the ref inside would force the import this list exists to - avoid. + Carries the metadata unassembled, since the Declaration is built with the + resolved key, and eq=False keeps identity comparison, since comparing the + ref inside would force the import this list exists to avoid. """ ref: _Ref @@ -263,8 +261,7 @@ def keys() -> Mapping[str | type[Any], Declaration]: For a startup check, an admin page or a test. Nothing is imported by the call, so a declaration made through a ref() appears once the ref has - resolved, and the catalogue lists what the modules imported so far have - declared. + resolved, and the catalogue lists what the modules imported so far declared. """ _absorb() with _lock: @@ -325,7 +322,7 @@ def _restore(saved: _Saved) -> None: actually started from. The firings kept are the snapshot's, filtered to keys the merged catalogue still marks suspicious. A pre-block pending declaration whose ref resolved during the block is absorbed on the way - out, since it is pre-block configuration whose moment simply arrived, and + out, since it is pre-block configuration whose moment arrived, and leaving it pending would let it re-land over a later declaration. """ declared, pending, fired, dropped = saved diff --git a/src/nodrill/_errors.py b/src/nodrill/_errors.py index e2626d6..2aca21e 100644 --- a/src/nodrill/_errors.py +++ b/src/nodrill/_errors.py @@ -61,9 +61,8 @@ def _reduced(error: BaseException) -> tuple[Any, tuple[Any, ...]]: class NoProviderError(LookupError): """Raised by use() when no provider is active for the requested key. - Carries the requested key, the active keys, the boundaries a declaration - named for it and, under debug mode, the diagnosis of where the value is, - as attributes. + Carries as attributes the requested key, the active keys, the boundaries a + declaration named for it and, under debug mode, where the value actually is. """ # A class-level default, so one pickled by a release without the field still answers. diff --git a/src/nodrill/_frozen.py b/src/nodrill/_frozen.py index 8faea63..659690c 100644 --- a/src/nodrill/_frozen.py +++ b/src/nodrill/_frozen.py @@ -70,8 +70,7 @@ def __hash__(self) -> int: def __reduce_ex__(self, protocol: SupportsIndex) -> Any: raise TypeError(_UNCOPYABLE) - # On the class, since copy looks these up on the instance and __getattr__ would - # hand back the target's own hook. + # On the class, since copy looks them up on the instance, where __getattr__ answers for it. def __copy__(self) -> Any: raise TypeError(_UNCOPYABLE) diff --git a/src/nodrill/_inject.py b/src/nodrill/_inject.py index ebb8963..23067a4 100644 --- a/src/nodrill/_inject.py +++ b/src/nodrill/_inject.py @@ -563,9 +563,8 @@ def _compile_wrapper( ) -> Callable[..., Any]: """Materialize the rendered wrapper and tie the lifetimes together. - The registered source lives exactly as long as the wrapper, and the - wrapper is popped out of its own globals so nothing needs the cycle - collector to die. + The registered source lives as long as the wrapper, and the wrapper is + popped out of its own globals so nothing needs the cycle collector to die. """ name, source, ns = _render_wrapper(func, sig, plan) filename = f"<@inject {plan.label}-{next(_SOURCE_IDS)}>" @@ -702,10 +701,9 @@ def inject(func: Any = None, /, *, from_: _KeyArg | None = None) -> Any: attribute of use("app"), defaults included, and skips self and cls. Explicitly passed arguments always win, an explicit None included. - Works on plain and async functions, methods, classmethods and - staticmethods in either decorator order. Generator functions are - rejected, because their bodies run after the call, possibly under - different providers. + Works on plain and async functions, methods, classmethods and staticmethods + in either decorator order. Generator functions are rejected, because their + bodies run after the call, possibly under different providers. """ if from_ is not None and not isinstance(from_, str | type) and not _is_ref(from_): raise TypeError( diff --git a/src/nodrill/_portable.py b/src/nodrill/_portable.py index 7aa77ed..6331ef7 100644 --- a/src/nodrill/_portable.py +++ b/src/nodrill/_portable.py @@ -3,8 +3,7 @@ export() renders the providers you name as a plain dict that JSON can hold, and adopt() opens them again wherever that dict arrives. A codec registered with set_codec() maps what JSON cannot hold into what it can, and its result -is checked like any other, so the envelope stays JSON whatever the codec does -inside. +is checked like any other, so the envelope stays JSON whatever a codec does. """ from __future__ import annotations @@ -60,11 +59,10 @@ def export(*names: str) -> dict[str, Any]: Nothing travels unless it is named here, and every value has to be JSON-safe, meaning a str, int, float, bool, None, or a list or dict of - those. Anything else raises rather than being coerced, so a value - arrives on the other side as itself or not at all. Containers are - rebuilt rather than referenced, so a write to a provider never reaches - an envelope already handed on. The result carries a version that - adopt() checks. + those. Anything else raises rather than being coerced, so a value arrives + on the other side as itself or not at all. Containers are rebuilt rather + than referenced, so a write to a provider never reaches an envelope already + handed on. The result carries a version that adopt() checks. """ # Read once, so a set_codec() part way through cannot build one envelope out of two codecs. dump = _codec.dump diff --git a/src/nodrill/_refs.py b/src/nodrill/_refs.py index 7805013..f205374 100644 --- a/src/nodrill/_refs.py +++ b/src/nodrill/_refs.py @@ -1,16 +1,14 @@ """The ref() key, its resolution, and the list of refs created so far. A ref names its target by import path and borrows that target's hash and -equality once it resolves, so the registry entry stored under the class is the -entry a lookup through the ref finds. Nothing branches on a ref. The dict -does the work, which leaves use() untouched and the compiled @inject wrappers -with it. +equality once it resolves, so the registry entry stored under the class is +the entry a lookup through the ref finds. Nothing branches on a ref, and +the dict doing the work is what leaves use() and @inject untouched. Resolution is deterministic and idempotent, so it runs unlocked. The module -lock guards only the lists of created refs that resolve_refs() walks. Holding a -lock across import_module() would order this module's lock against the import -system's per-module locks, which is the deadlock every lazy importer eventually -reports. +lock guards only the lists of created refs that resolve_refs() walks. A lock +held across import_module() would order this module's lock against the import +system's per-module locks, the deadlock every lazy importer eventually reports. """ from __future__ import annotations diff --git a/src/nodrill/_report.py b/src/nodrill/_report.py index 48f4f32..a89374e 100644 --- a/src/nodrill/_report.py +++ b/src/nodrill/_report.py @@ -100,16 +100,17 @@ def _boundary_note(exc: BaseException, where: str) -> None: def annotate_exceptions(*, enabled: bool = True) -> None: """Attach the scope to every exception leaving a provider block, process wide. - An exception passing out of a provider block gains a note naming what - that block provided, and comes out as the same object with __notes__ the - only thing about it that changed. Nested blocks each add their own as it - climbs, innermost first. One block decides for itself with - provider(..., annotate=True) or annotate=False, and enabled=False turns - the switch off again. Rendering a note calls the value's repr while the - block is unwinding, so whatever a provider holds can be printed into a - traceback, and a value carrying a secret should hide it in its own repr. - On Python 3.10 this warns and does nothing, since exception notes are - 3.11 and up. + An exception passing out of a provider block gains a note naming what that + block provided, and comes out as the same object with __notes__ the only + thing about it that changed, innermost block first where several nest. + + One block decides for itself with provider(..., annotate=True) or + annotate=False, and enabled=False turns the switch off again. + + Rendering a note calls the value's repr while the block is unwinding, so + whatever a provider holds can be printed into a traceback, and a value + carrying a secret should hide it in its own repr. On Python 3.10 this + warns and does nothing, since exception notes are 3.11 and up. """ if enabled and _add_note is _drop_note: warnings.warn(_UNSUPPORTED, RuntimeWarning, stacklevel=2) diff --git a/src/nodrill/_sealed.py b/src/nodrill/_sealed.py index 96bcbbf..8b37def 100644 --- a/src/nodrill/_sealed.py +++ b/src/nodrill/_sealed.py @@ -36,8 +36,7 @@ _UNCOPYABLE = "sealed context views cannot be pickled or copied" -# Generated rather than written out, since these three are off the hot path and report -# themselves, unlike the attribute members below which report the name they were given. +# Generated, since these three report themselves where the attribute members below report a name. _COMPARED: dict[str, Callable[..., Any]] = { "__eq__": operator.eq, "__ne__": operator.ne, @@ -110,8 +109,7 @@ def __class__(self) -> type[Any]: # pyright: ignore[reportIncompatibleMethodOve answer: type[Any] = self._nodrill_target.__class__ return answer - # Written out rather than generated, since a generator taking the name through *args - # costs half as much again on the operation a sealed value is read through most. + # Written out, since taking the name through *args costs half as much again on a read. def __getattr__(self, name: str) -> Any: scope = self._nodrill_scope if scope.exited is not None: @@ -137,8 +135,7 @@ def __dir__(self) -> list[str]: def __reduce_ex__(self, protocol: SupportsIndex) -> Any: raise TypeError(_UNCOPYABLE) - # On the class, since copy looks these up on the instance and __getattr__ would - # hand back the target's own hook. + # On the class, since copy looks them up on the instance, where __getattr__ answers for it. def __copy__(self) -> Any: raise TypeError(_UNCOPYABLE) diff --git a/tests/cycle/__init__.py b/tests/cycle/__init__.py index 6c554e4..0429e4a 100644 --- a/tests/cycle/__init__.py +++ b/tests/cycle/__init__.py @@ -6,6 +6,5 @@ written with a plain import and cannot be imported at all, so the cycle here is a real one rather than a described one. -`at_import`, `alias` and `reloadable` carry a case each, described where they -stand. +`at_import`, `alias` and `reloadable` carry a case each, described in place. """ diff --git a/tests/cycle/at_import.py b/tests/cycle/at_import.py index 2e14ebd..649e058 100644 --- a/tests/cycle/at_import.py +++ b/tests/cycle/at_import.py @@ -22,8 +22,7 @@ class Scope: try: - # The module is still initialising here too, but the name that is missing is - # missing from the class, which has nothing to do with the import. + # Still initialising here too, but the missing name is the class's, not the import's. use(ref("tests.cycle.at_import:Scope.missing")) except KeyResolutionError as exc: NESTED_FAILURE = str(exc) diff --git a/tests/test_debug.py b/tests/test_debug.py index 668fd94..0fbecc0 100644 --- a/tests/test_debug.py +++ b/tests/test_debug.py @@ -191,7 +191,7 @@ def test_the_innermost_open_block_is_the_one_reported(self) -> None: assert f"{__file__}:{opened}" in str(error) def test_a_bare_pool_worker_is_diagnosed(self) -> None: - """The case the feature exists for: submitting to a pool that is not nodrill's.""" + """The case the feature exists for, submitting to a pool that is not nodrill's.""" with debug(), provider(Session()), ThreadPoolExecutor(max_workers=1) as pool: error = pool.submit(read_session).result() assert "which did not inherit that context" in str(error) @@ -505,7 +505,7 @@ def test_a_provider_nothing_read_warns_at_the_with_statement(self) -> None: assert record.lineno == opened def test_a_read_provider_is_silent(self) -> None: - """A provider something read is not warned about; warnings are errors here.""" + """A provider something read is not warned about, and warnings are errors here.""" with debug(unused=True), provider(Session()): use(Session) diff --git a/tests/test_errors.py b/tests/test_errors.py index 9bbaf50..98942bc 100644 --- a/tests/test_errors.py +++ b/tests/test_errors.py @@ -71,7 +71,7 @@ def test_no_suggestion_when_nothing_is_close(self) -> None: assert "Did you mean" not in str(exc_info.value) def test_hand_built_error_with_an_odd_key_still_builds_a_message(self) -> None: - """NoProviderError is public API: use() screens keys, a direct caller need not.""" + """NoProviderError is public API, and a direct caller need not screen keys as use() does.""" assert "42" in str(NoProviderError(42)) def test_use_rejects_non_key_types(self) -> None: diff --git a/tests/test_inject_async.py b/tests/test_inject_async.py index 1588c34..f3f547e 100644 --- a/tests/test_inject_async.py +++ b/tests/test_inject_async.py @@ -19,7 +19,7 @@ async def fetch(cfg: FromCtx[Config] = injected, retry: int = 0) -> str: class TestAsyncInjection: def test_wrapper_is_a_coroutine_function(self) -> None: - """The wrapper must be async — never a sync function returning a coroutine.""" + """The wrapper must be async, never a sync function returning a coroutine.""" assert inspect.iscoroutinefunction(fetch) async def test_injects_from_provider(self) -> None: diff --git a/tests/test_inject_binding.py b/tests/test_inject_binding.py index ccbc7aa..26a9cd4 100644 --- a/tests/test_inject_binding.py +++ b/tests/test_inject_binding.py @@ -1,7 +1,7 @@ -"""Argument binding in @inject: the compiled wrapper mirrors the signature. +"""Argument binding in @inject, where the compiled wrapper mirrors the signature. The wrapper's parameter list is generated from the function's own, so the -interpreter binds every call shape natively; these tests pin the shapes down. +interpreter binds every call shape natively, and these tests pin the shapes down. """ from collections.abc import Callable @@ -179,8 +179,7 @@ def handler(request: str, db: FromCtx[Db] = injected) -> str: handler("a", "b", "c") # type: ignore[call-arg, arg-type] def test_a_bad_call_fails_before_resolution(self) -> None: - # The wrapper mirrors the signature, so Python rejects the call exactly - # as it would reject the undecorated function, provider or no provider. + # The wrapper mirrors the signature, so Python rejects the call as it always would. @inject def handler(db: FromCtx[Db] = injected) -> str: return db.dsn @@ -233,8 +232,7 @@ def handler(db: FromCtx[Db], a: str, b: str) -> str: handler() # type: ignore[call-arg] def test_missing_required_argument_beats_the_provider_miss(self) -> None: - # The guard runs before any resolution, so the caller's mistake is - # reported even when no provider is active. + # The guard runs before any resolution, so no provider need be active to report it. @inject def handler(db: FromCtx[Db], tag: str) -> str: return tag diff --git a/tests/test_inject_codegen.py b/tests/test_inject_codegen.py index 2c86c6a..8d457a3 100644 --- a/tests/test_inject_codegen.py +++ b/tests/test_inject_codegen.py @@ -1,4 +1,4 @@ -"""The compiled wrapper as an artifact: its source, its names, its lifetime.""" +"""The compiled wrapper as an artifact, meaning its source, its names and its lifetime.""" import gc import linecache diff --git a/tests/test_lazy.py b/tests/test_lazy.py index 5fee7ac..f6ee53d 100644 --- a/tests/test_lazy.py +++ b/tests/test_lazy.py @@ -58,7 +58,7 @@ def __call__(self) -> Config: class TestResolution: def test_unread_provider_never_builds(self) -> None: - """The whole point: a scope nothing reads costs nothing to open.""" + """The whole point, that a scope nothing reads costs nothing to open.""" factory = Counter() with provider(lazy(Config, factory)): pass @@ -202,7 +202,7 @@ def factory() -> Config: touch() def test_factory_returning_its_own_key_raises(self) -> None: - """The same mistake by return: the cell would otherwise become its own value.""" + """The same mistake by return, where the cell would otherwise become its own value.""" def factory() -> Config: returned: Config = use(Config) diff --git a/tests/test_lazy_hints.py b/tests/test_lazy_hints.py index 4e062f5..e6ae52a 100644 --- a/tests/test_lazy_hints.py +++ b/tests/test_lazy_hints.py @@ -10,7 +10,7 @@ @inject def refers_forward(cfg: FromCtx[DefinedLater] = injected) -> str: - """Decorated while `DefinedLater` does not exist yet — must not raise.""" + """Decorated while `DefinedLater` does not exist yet, which must not raise.""" return cfg.tag diff --git a/tests/test_refs.py b/tests/test_refs.py index 1e5622b..fd5871c 100644 --- a/tests/test_refs.py +++ b/tests/test_refs.py @@ -252,8 +252,7 @@ def test_path_must_be_a_string(self) -> None: ref(Config) # type: ignore[arg-type] def test_a_path_naming_a_module_is_only_refused_at_the_lookup(self) -> None: - # 'package.module' cannot be told from 'module.Name', so the path is accepted - # and the module it names is what fails, as any other non-key target does. + # 'package.module' cannot be told from 'module.Name', so the module it names fails. key = ref("json.decoder") with pytest.raises(TypeError, match="use\\(\\) expects a string name or a class"): use(key) diff --git a/tests/test_sealed.py b/tests/test_sealed.py index 9acbbee..17a901a 100644 --- a/tests/test_sealed.py +++ b/tests/test_sealed.py @@ -165,8 +165,7 @@ class TestGuardsAndCopies: def test_a_defaulting_getattr_does_not_swallow_the_expiry(self) -> None: with provider(Session(), sealed=True) as session: pass - # Not an AttributeError, since getattr(x, name, default) would answer the default - # and hand the caller the silent wrong value sealing exists to report. + # Not an AttributeError, or getattr(x, name, default) would answer with the default. assert not issubclass(ExpiredScopeError, AttributeError) with pytest.raises(ExpiredScopeError): getattr(session, "dsn", "fallback") diff --git a/tests/test_sealed_protocols.py b/tests/test_sealed_protocols.py index 584e862..d9037db 100644 --- a/tests/test_sealed_protocols.py +++ b/tests/test_sealed_protocols.py @@ -359,8 +359,7 @@ def expired(self) -> Everything: def test_every_generated_protocol_checks_the_scope( self, expired: Everything, name: str ) -> None: - # Called with no arguments, since the check runs before the delegation, so - # a missing one shows up as something other than ExpiredScopeError. + # Called with no arguments, since the check runs first and a missing one raises otherwise. args = (1,) if name in _REFLECTED or name in _INPLACE else () method = getattr(type(expired), name) with pytest.raises(ExpiredScopeError, match=rf"Everything\.{name} was used after"): diff --git a/tests/test_threads.py b/tests/test_threads.py index e5b3170..a317377 100644 --- a/tests/test_threads.py +++ b/tests/test_threads.py @@ -38,7 +38,7 @@ def pool() -> Iterator[Executor]: class TestPlainThreads: def test_plain_thread_does_not_see_context(self, in_thread: ThreadRunner) -> None: - """The documented behavior: threading.Thread starts with an empty context.""" + """The documented behavior, that threading.Thread starts with an empty context.""" with provider(Config(tag="main")): assert isinstance(in_thread(read_tag), NoProviderError) From a8371c376440a5780a5483988e4341efe667dd2c Mon Sep 17 00:00:00 2001 From: Pavel Kutsenko Date: Mon, 24 Aug 2026 02:35:07 +0300 Subject: [PATCH 06/12] fix: keep a run's shards together and a silent boundary visible A relative NODRILL_CONTRACT named one place to the process that was armed and another to a child that started somewhere else, so that child's shard landed in a directory nobody renders and a whole boundary left the contract with no diagnostic. The resolved directory is now written back over the variable beside the run id. A declared boundary that opened and read nothing was indistinguishable from one no block opened, so the command told a handler that reads nothing its key had been renamed, on every run. The recorder notes the open as a fourth verb and the render drops it again as soon as that boundary has a read of its own. The benchmark recomputed its ratio column from unrounded timings while republishing the rounded nanoseconds, so a settled table could still rewrite the page. Dividing after rounding makes the table a function of its own ns column. --- benchmarks/bench.py | 6 +- .../howto/record-what-a-handler-reads.rst | 6 ++ docs/content/misc/design.rst | 5 +- docs/content/ref/debugging.rst | 2 +- src/nodrill/_audit.py | 25 +++++-- src/nodrill/_debug.py | 5 ++ tests/test_audit.py | 67 ++++++++++++++++++- 7 files changed, 106 insertions(+), 10 deletions(-) diff --git a/benchmarks/bench.py b/benchmarks/bench.py index 480e950..71f36ae 100644 --- a/benchmarks/bench.py +++ b/benchmarks/bench.py @@ -214,7 +214,11 @@ def render(timings: Mapping[str, float]) -> str: base = timings[BASELINE] # The multiplication sign, since that is the heading the published table carries. header = ("operation", "ns", "×") # noqa: RUF001 - rows = [(label, f"{round(timings[label])}", ratio(timings[label] / base)) for label in ORDER] + # Divided after rounding, so the table is a function of its own ns column and a rerun settles. + rows = [ + (label, f"{round(timings[label])}", ratio(round(timings[label]) / round(base))) + for label in ORDER + ] widths = [max(len(cell) for cell in column) for column in zip(header, *rows, strict=True)] rule = " ".join("=" * width for width in widths) diff --git a/docs/content/howto/record-what-a-handler-reads.rst b/docs/content/howto/record-what-a-handler-reads.rst index ac0f0e8..053d10f 100644 --- a/docs/content/howto/record-what-a-handler-reads.rst +++ b/docs/content/howto/record-what-a-handler-reads.rst @@ -113,6 +113,7 @@ The two boundaries read the same three keys, except that the queue never opens ` That is a bug the code cannot show you and no test fails on, and it is one line of a diff. Pass the same value to the command, so it can tell you about a boundary you named that no block opened, which is what a renamed key looks like. +A boundary that opened and read nothing is a row of the file rather than that message, so the two cases stay apart. Reading the file ---------------- @@ -133,6 +134,10 @@ The second field is the one to read. `default` No provider was open and the ``use(key, default=...)`` at the call site answered. +`opened` + A boundary you named opened and nothing under it read the context, so the third field is `nothing` rather than a key. + It is written only for a boundary that read nothing, which is what keeps a handler reading nothing apart from a boundary the run never reached. + An entry point of `(none)` means no provider block was open at all, which a read can only survive by falling back. It is what an unwrapped worker thread looks like, and what a read at import time looks like. @@ -156,6 +161,7 @@ The contract file has to be committed for the third step to compare anything, si Recording is off unless ``NODRILL_CONTRACT`` is set, and the variable is read once when `nodrill` is imported, which is also why a subprocess your suite spawns records too. Each process writes its own file into the directory and the command merges them, so a suite that shells out or one using a :class:`~concurrent.futures.ProcessPoolExecutor` needs nothing extra. +The directory is resolved once and written back into the environment, so a relative ``.nodrill`` means the same place to a child your suite starts in another directory. A directory reused by a later run is not a problem either, since every process of one run shares a run id and the command reads the newest run and says how many older files it left out. A run id is inherited through the environment, so processes of one run share it only when the process that started them imported `nodrill` itself. diff --git a/docs/content/misc/design.rst b/docs/content/misc/design.rst index 3d07bea..c02f732 100644 --- a/docs/content/misc/design.rst +++ b/docs/content/misc/design.rst @@ -512,13 +512,16 @@ The file is three tab-separated fields, sorted, and carries no file names and no A site moves whenever anything above it moves, so a contract carrying sites churns on every pull request and stops being read, and sites belong in a failure message where the audience is different. A tab rather than aligned columns, because padding means one long key rewrites every line, and rather than two spaces, because ``repr`` escapes a tab and a newline but not a space, so a key holding two spaces in a row would otherwise split into more fields than the format has. The verb carries the whole answer, ``requires`` or ``set_default`` or ``default``, rather than a fourth column, so every line is the same shape and a reviewer greps for what is not ``requires``. +A fourth verb, ``opened``, is written when a declared boundary opened and read nothing, and it is dropped again from the rendered contract as soon as that boundary has a read of its own. +Without it the tool would have to infer that a boundary never ran from the absence of its reads, and it would tell a handler that reads nothing that its key had been renamed, on every run and forever. The switch is an environment variable read once at import, because a child interpreter inherits one. That is what makes a suite that spawns subprocesses, runs under ``xdist`` or uses a process pool record without a special case for any of them. A pool worker needs two more things. :mod:`multiprocessing` exits a worker through :func:`os._exit`, which runs finalizers and never :mod:`atexit`, so the dump is registered both ways and made idempotent rather than registered once and lost. A fork then clears the finalizer registry before the worker body runs, so the child registers the finalizer again from an after-fork hook, which is the one callback :mod:`multiprocessing` runs after that clear. -The directory is resolved to an absolute path when the variable is read, since the hooks run at exit and a program that changed directory would otherwise write somewhere nobody looks. +The directory is resolved to an absolute path when the variable is read, and the resolved path is written back over the variable beside the run id, since the hooks run at exit and a relative directory otherwise names one place to the process that was armed and another to a child that starts somewhere else. +A child recording beside itself is a whole boundary of the contract landing where nothing renders it, which the merge cannot report because it never sees the file. Each process of a run shares a run id minted at arming and written back into the environment, so a directory reused by a later run yields the newer contract rather than the union of both. That inheritance works through the environment, so it reaches a child and not a sibling started by a runner that never imported the library, which is why the variable can also be set from outside. diff --git a/docs/content/ref/debugging.rst b/docs/content/ref/debugging.rst index 47812eb..eefd1e3 100644 --- a/docs/content/ref/debugging.rst +++ b/docs/content/ref/debugging.rst @@ -78,7 +78,7 @@ Recording a contract ``NODRILL_CONTRACT`` names a directory and turns on recording of what each entry point reads. It is read once, at import, like ``NODRILL_DEBUG``, and any non-empty value is a directory rather than a switch, so ``0`` names a directory called ``0``. Every process of a run writes its own file there, including one a suite spawns, and the files are merged when the contract is rendered. -A relative directory is resolved when the variable is read, so a program that changes directory still writes where it was armed. +A relative directory is resolved when the variable is read, and the resolved directory is written back into the environment, so a program that changes directory and a child that starts in another one both write where the run was armed. Recording puts an instrumented registry in front of every read, at the same cost ``unused=True`` pays, so it belongs in a suite rather than in production. ``NODRILL_CONTRACT_ENTRY`` names the provider keys that are boundaries, as rendered keys separated by commas, so ``"'http request',myapp.web:Request"``. diff --git a/src/nodrill/_audit.py b/src/nodrill/_audit.py index 139d0d9..ed3210f 100644 --- a/src/nodrill/_audit.py +++ b/src/nodrill/_audit.py @@ -32,8 +32,10 @@ # Written and diffed on machines nobody here chose, so nothing about the bytes is the platform's. _ENCODING = "utf-8" _NEWLINE = "\n" +# The one verb this file compares against, since the render drops it where a read says more. +_OPENED = "opened" # The vocabulary a fact is written in, which _parse refuses a line outside of. -_VERBS = frozenset({"requires", "set_default", "default"}) +_VERBS = frozenset({"requires", "set_default", "default", _OPENED}) # Owned here with the file, unlike NODRILL_CONTRACT, which gates this import and lives in _debug. _ENTRY_VAR = "NODRILL_CONTRACT_ENTRY" _RUN_VAR = "NODRILL_CONTRACT_RUN" @@ -59,6 +61,16 @@ def _render(reads: _Reads) -> str: return "".join(f"{line}{_NEWLINE}" for line in lines) +def _visible(reads: _Reads) -> _Reads: + """Drop the opened row of a boundary that went on to read, since its reads already say so. + + What survives is the boundary a run opened and read nothing under, which + is a fact about that entry point and not the absence of one. + """ + read = {entry for entry, verb, _ in reads if verb != _OPENED} + return {fact for fact in reads if fact[1] != _OPENED or fact[0] not in read} + + def _refuse(source: str, saw: str, expected: str) -> ValueError: """Build the one refusal, so a caller can say which file and what it expected.""" return ValueError( @@ -151,7 +163,11 @@ def _summary(reads: _Reads, shards: int, stale: int) -> str: def _unseen(reads: _Reads, declared: frozenset[str]) -> str | None: - """Report a declared entry point no block opened, since a renamed key would go quiet.""" + """Report a declared entry point no block opened, since a renamed key would go quiet. + + Read off the whole run rather than off the rendered contract, because a + boundary that opened and read nothing is recorded and not rendered. + """ missing = sorted(declared - {entry for entry, _, _ in reads}) if not missing: return None @@ -180,7 +196,8 @@ def _contract(source: str, target: str | None, declared: frozenset[str]) -> int: f"Run the suite with NODRILL_CONTRACT={source} first" ) return 1 - text = _render(reads) + facts = _visible(reads) + text = _render(facts) if target is None: # Through the buffer, so neither the locale nor the platform edits the artefact. sys.stdout.flush() @@ -192,7 +209,7 @@ def _contract(source: str, target: str | None, declared: frozenset[str]) -> int: except OSError as error: _say(f"nodrill: cannot write {target}, {error.strerror}") return 1 - _say(_summary(reads, shards, stale)) + _say(_summary(facts, shards, stale)) unseen = _unseen(reads, declared) if unseen is not None: _say(unseen) diff --git a/src/nodrill/_debug.py b/src/nodrill/_debug.py index 81d0057..b1aaecb 100644 --- a/src/nodrill/_debug.py +++ b/src/nodrill/_debug.py @@ -199,6 +199,8 @@ def _arm(environ: MutableMapping[str, str]) -> None: # Resolved now, since the hooks below run at exit and a program may have moved by then. directory = str(Path(directory).resolve()) + # Written back too, so a child that starts elsewhere records here and not beside itself. + environ["NODRILL_CONTRACT"] = directory _declared_entries.update(_declared(environ.get(_ENTRY_VAR, ""))) run = environ.get(_RUN_VAR) or _new_run() # Written back so every child joins this run rather than starting one of its own. @@ -384,6 +386,9 @@ def _record_enter( owners: dict[_Key, _Reads] = {} # Only the audit reads a label, and rendering a key is not free on the block path. entry = _key_path(key) if _state.auditing else _NO_ENTRY + # Noted on the open, so a boundary that reads nothing stays apart from one that never ran. + if _state.auditing and entry in _declared_entries: + _note((entry, "opened", "nothing")) if isinstance(enclosing, _InstrumentedRegistry): # Inherited whether or not counting is still on, since it is process-wide. owners = dict(enclosing.owners) diff --git a/tests/test_audit.py b/tests/test_audit.py index 299c73a..78ec32c 100644 --- a/tests/test_audit.py +++ b/tests/test_audit.py @@ -28,6 +28,7 @@ _render, _summary, _unseen, + _visible, main, ) from nodrill._debug import _arm, _declared_entries, _reads, _recording, _state @@ -83,7 +84,7 @@ def armed(recording: set[tuple[str, str, str]]) -> Iterator[list[tuple[Any, ...] def _facts(reads: set[tuple[str, str, str]]) -> set[str]: """Render what was recorded the way the contract file does, minus the header.""" - return {line for line in _render(reads).splitlines() if line != HEADER} + return {line for line in _render(_visible(reads)).splitlines() if line != HEADER} def _entries(reads: set[tuple[str, str, str]]) -> set[str]: @@ -239,6 +240,26 @@ def test_a_declared_key_nothing_opened_is_reported(self) -> None: assert message is not None assert "no block opened 'celery worker'" in message + def test_a_declared_boundary_that_reads_nothing_is_recorded_as_opened( + self, declaring: Any + ) -> None: + """A handler that reads nothing must not read as a boundary a rename took away.""" + with running(), provider("celery worker"): + pass + assert f"'celery worker'{TAB}opened{TAB}nothing" in _facts(declaring) + assert _unseen(declaring, frozenset({"'celery worker'"})) is None + + def test_the_opened_row_is_dropped_where_a_read_says_more(self) -> None: + reads = { + ("'http request'", "opened", "nothing"), + ("'http request'", "requires", "x"), + ("'celery worker'", "opened", "nothing"), + } + assert _visible(reads) == { + ("'http request'", "requires", "x"), + ("'celery worker'", "opened", "nothing"), + } + @pytest.mark.parametrize( ("value", "expected"), [ @@ -409,6 +430,15 @@ def test_a_forked_child_registers_the_finalizer_the_fork_cleared( after_fork(None) assert [name for name, _ in armed] == ["finalize"] + def test_a_child_that_starts_elsewhere_records_where_the_parent_did( + self, tmp_path: Path, armed: list[tuple[Any, ...]], monkeypatch: pytest.MonkeyPatch + ) -> None: + """A relative directory names one place to the parent and another to a child that moved.""" + monkeypatch.chdir(tmp_path) + environ = {"NODRILL_CONTRACT": ".nodrill"} + _arm(environ) + assert environ["NODRILL_CONTRACT"] == str((tmp_path / ".nodrill").resolve()) + def test_a_relative_directory_is_resolved_while_the_program_is_still_there( self, tmp_path: Path, armed: list[tuple[Any, ...]], monkeypatch: pytest.MonkeyPatch ) -> None: @@ -625,6 +655,15 @@ def test_a_declared_key_nothing_opened_reaches_the_output( assert _contract(str(tmp_path), None, frozenset({"'b'"})) == 0 assert "no block opened 'b'" in capsys.readouterr().err + def test_a_boundary_that_opened_and_read_nothing_is_a_row_and_not_a_diagnostic( + self, tmp_path: Path, capsys: Any + ) -> None: + _dump(str(tmp_path), "run", {("'a'", "requires", "x"), ("'b'", "opened", "nothing")}) + assert _contract(str(tmp_path), None, frozenset({"'a'", "'b'"})) == 0 + captured = capsys.readouterr() + assert f"'b'{TAB}opened{TAB}nothing" in captured.out + assert "no block opened" not in captured.err + def test_write_names_the_file(self, tmp_path: Path, capsys: Any) -> None: _dump(str(tmp_path), "run", {("'a'", "requires", "x")}) target = tmp_path / "nodrill.contract" @@ -658,14 +697,16 @@ def test_the_command_says_which_nodrill_wrote_a_contract(self, capsys: Any) -> N assert capsys.readouterr().out.strip() == f"nodrill {nodrill.__version__}" -def _child(program: str, directory: Path, entries: str = "") -> subprocess.CompletedProcess[str]: +def _child( + program: str, directory: Path, entries: str = "", cwd: Path = _ROOT +) -> subprocess.CompletedProcess[str]: """Run a program in a child interpreter with the recorder armed.""" return subprocess.run( # the interpreter running this suite, with a program written above [sys.executable, "-c", program], check=True, capture_output=True, text=True, - cwd=str(_ROOT), + cwd=str(cwd), env={ **os.environ, "NODRILL_CONTRACT": str(directory), @@ -707,6 +748,26 @@ def test_a_subprocess_the_run_spawns_joins_the_same_run(self, tmp_path: Path) -> assert (shards, stale) == (2, 0) assert f"'celery worker'{TAB}set_default{TAB}{APP}:Origin" in _render(reads) + def test_a_subprocess_that_moves_still_records_into_the_same_directory( + self, tmp_path: Path + ) -> None: + """A relative directory is the whole point of the variable being resolved once.""" + elsewhere = tmp_path / "elsewhere" + elsewhere.mkdir() + program = ( + "import subprocess, sys\n" + f"from {APP} import running, serve_http\n" + "with running(): serve_http('ada')\n" + "subprocess.run([sys.executable, '-c'," + f" 'from {APP} import run_job; run_job(\"grace\")']," + f" check=True, cwd={str(elsewhere)!r})\n" + ) + _child(program, Path(".nodrill"), cwd=tmp_path) + assert not (elsewhere / ".nodrill").exists() + reads, shards, stale = _merge(tmp_path / ".nodrill") + assert (shards, stale) == (2, 0) + assert f"'celery worker'{TAB}requires{TAB}{APP}:User" in _render(reads) + def test_a_process_pool_worker_records_its_own_shard(self, tmp_path: Path) -> None: """A worker exits through os._exit, which runs finalizers and never atexit.""" program = ( From bd3328620cf3af2454216d0fc4218f1b60c47d14 Mon Sep 17 00:00:00 2001 From: Pavel Kutsenko Date: Mon, 24 Aug 2026 02:36:48 +0300 Subject: [PATCH 07/12] docs: tighten the prose the recorder fix added --- docs/content/howto/record-what-a-handler-reads.rst | 4 ++-- docs/content/misc/design.rst | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/content/howto/record-what-a-handler-reads.rst b/docs/content/howto/record-what-a-handler-reads.rst index 053d10f..c91eb84 100644 --- a/docs/content/howto/record-what-a-handler-reads.rst +++ b/docs/content/howto/record-what-a-handler-reads.rst @@ -113,7 +113,7 @@ The two boundaries read the same three keys, except that the queue never opens ` That is a bug the code cannot show you and no test fails on, and it is one line of a diff. Pass the same value to the command, so it can tell you about a boundary you named that no block opened, which is what a renamed key looks like. -A boundary that opened and read nothing is a row of the file rather than that message, so the two cases stay apart. +A boundary that opened and read nothing is a row of the file rather than that message. Reading the file ---------------- @@ -136,7 +136,7 @@ The second field is the one to read. `opened` A boundary you named opened and nothing under it read the context, so the third field is `nothing` rather than a key. - It is written only for a boundary that read nothing, which is what keeps a handler reading nothing apart from a boundary the run never reached. + It is what keeps a handler that reads nothing apart from a boundary the run never reached. An entry point of `(none)` means no provider block was open at all, which a read can only survive by falling back. It is what an unwrapped worker thread looks like, and what a read at import time looks like. diff --git a/docs/content/misc/design.rst b/docs/content/misc/design.rst index c02f732..892aa7c 100644 --- a/docs/content/misc/design.rst +++ b/docs/content/misc/design.rst @@ -513,7 +513,7 @@ A site moves whenever anything above it moves, so a contract carrying sites chur A tab rather than aligned columns, because padding means one long key rewrites every line, and rather than two spaces, because ``repr`` escapes a tab and a newline but not a space, so a key holding two spaces in a row would otherwise split into more fields than the format has. The verb carries the whole answer, ``requires`` or ``set_default`` or ``default``, rather than a fourth column, so every line is the same shape and a reviewer greps for what is not ``requires``. A fourth verb, ``opened``, is written when a declared boundary opened and read nothing, and it is dropped again from the rendered contract as soon as that boundary has a read of its own. -Without it the tool would have to infer that a boundary never ran from the absence of its reads, and it would tell a handler that reads nothing that its key had been renamed, on every run and forever. +Without it a boundary that never ran could only be inferred from the absence of its reads, which tells a handler that reads nothing that its key was renamed, on every run. The switch is an environment variable read once at import, because a child interpreter inherits one. That is what makes a suite that spawns subprocesses, runs under ``xdist`` or uses a process pool record without a special case for any of them. @@ -521,7 +521,7 @@ A pool worker needs two more things. :mod:`multiprocessing` exits a worker through :func:`os._exit`, which runs finalizers and never :mod:`atexit`, so the dump is registered both ways and made idempotent rather than registered once and lost. A fork then clears the finalizer registry before the worker body runs, so the child registers the finalizer again from an after-fork hook, which is the one callback :mod:`multiprocessing` runs after that clear. The directory is resolved to an absolute path when the variable is read, and the resolved path is written back over the variable beside the run id, since the hooks run at exit and a relative directory otherwise names one place to the process that was armed and another to a child that starts somewhere else. -A child recording beside itself is a whole boundary of the contract landing where nothing renders it, which the merge cannot report because it never sees the file. +A child recording beside itself puts a whole boundary of the contract where nothing renders it, and the merge cannot report a file it never sees. Each process of a run shares a run id minted at arming and written back into the environment, so a directory reused by a later run yields the newer contract rather than the union of both. That inheritance works through the environment, so it reaches a child and not a sibling started by a runner that never imported the library, which is why the variable can also be set from outside. From 764b25c2af2c2398bcaf9ca62c742fdc732ac7ef Mon Sep 17 00:00:00 2001 From: Pavel Kutsenko Date: Mon, 24 Aug 2026 02:39:06 +0300 Subject: [PATCH 08/12] docs: give the contract file format a reference section of its own The how-to carried the spelling of every field and every verb, which is reference material sitting in a task page, so a reader who wanted only the format had to read a recipe to find it. The how-to keeps the run, the CI wiring and what the file is worth, and points at the format once. --- .../howto/record-what-a-handler-reads.rst | 23 ++------------ docs/content/ref/debugging.rst | 31 ++++++++++++++++++- 2 files changed, 32 insertions(+), 22 deletions(-) diff --git a/docs/content/howto/record-what-a-handler-reads.rst b/docs/content/howto/record-what-a-handler-reads.rst index c91eb84..c57820e 100644 --- a/docs/content/howto/record-what-a-handler-reads.rst +++ b/docs/content/howto/record-what-a-handler-reads.rst @@ -118,28 +118,9 @@ A boundary that opened and read nothing is a row of the file rather than that me Reading the file ---------------- -Three tab-separated fields, sorted, one fact per line. -The first is the entry point, the second is how the read was answered, the third is the key, rendered the way :func:`~nodrill.ref` spells one so two same-named classes in different modules stay apart. -A string key keeps the quotes Python puts on it, which is also what keeps a key holding a tab or a newline from becoming two lines. - The second field is the one to read. - -`requires` - A provider answered, which is the ordinary case. - -`set_default` - No provider was open and a :func:`~nodrill.set_default` factory answered instead. - Every one of these is a boundary that does not open a key somebody registered a fallback for. - -`default` - No provider was open and the ``use(key, default=...)`` at the call site answered. - -`opened` - A boundary you named opened and nothing under it read the context, so the third field is `nothing` rather than a key. - It is what keeps a handler that reads nothing apart from a boundary the run never reached. - -An entry point of `(none)` means no provider block was open at all, which a read can only survive by falling back. -It is what an unwrapped worker thread looks like, and what a read at import time looks like. +A `requires` row is a provider answering, and a `set_default` or a `default` row is a key the boundary never opened, which is the row this file exists for. +:ref:`ref-contract-file` has the whole vocabulary, the entry point `(none)` among it. Wiring it into CI ----------------- diff --git a/docs/content/ref/debugging.rst b/docs/content/ref/debugging.rst index eefd1e3..3eed501 100644 --- a/docs/content/ref/debugging.rst +++ b/docs/content/ref/debugging.rst @@ -99,7 +99,36 @@ Setting it yourself is how a runner that starts its workers directly, such as `` It returns ``0`` when it rendered a contract and ``1`` when it could not, leaving ``2`` to mean the command line itself was wrong. A shard it cannot read is a message and the exit code, never a traceback, and ``python -m nodrill --version`` says which nodrill is reading. -:doc:`/content/howto/record-what-a-handler-reads` is the task-shaped version, with the file format and what it is worth. +:doc:`/content/howto/record-what-a-handler-reads` is the task-shaped version, with a program to run and what the file is worth. + +.. _ref-contract-file: + +The contract file +~~~~~~~~~~~~~~~~~ + +A shard and a rendered contract carry one format, ``# nodrill contract 1`` on the first line and one fact per line under it, sorted, UTF-8 with ``\n`` endings whatever the platform. +A reader refuses a first line it does not know rather than guessing at it, so a file a later version wrote is a message and an exit code. + +A fact is three tab-separated fields, the entry point, the verb, and the key. +The entry point and the key are both rendered the way :func:`~nodrill.ref` spells one, ``myapp.web:Request`` for a class and ``'http request'`` for a string, quotes included, which is also what keeps a key holding a tab or a newline on one line. + +The verb says how the read was answered. + +``requires`` + A provider answered, which is the ordinary case. + +``set_default`` + No provider was open and a :func:`~nodrill.set_default` factory answered instead. + Every one of these is a boundary that leaves a key to a fallback. + +``default`` + No provider was open and the ``use(key, default=...)`` at the call site answered. + +``opened`` + A boundary ``NODRILL_CONTRACT_ENTRY`` names opened and nothing under it read the context, so the third field is the word ``nothing`` rather than a key. + It is dropped again as soon as that boundary has a read of its own, and it is what keeps a handler that reads nothing apart from a boundary the run never reached. + +An entry point of ``(none)`` is a read with no provider block open above it at all, which only a fallback survives, so an unwrapped worker thread and a read at import time both land there. explain ------- From bf83b66f02eb1f6c8f5ecaa0c79d6b18f236825d Mon Sep 17 00:00:00 2001 From: Pavel Kutsenko Date: Wed, 26 Aug 2026 11:57:23 +0300 Subject: [PATCH 09/12] fix: correct a plural, a refusal and an unguarded rewrite _summary hardcoded the plural verb, so one leftover shard printed as "1 shard from an earlier run were left out". The sentence now reads "Left out 1 shard from an earlier run." and agrees at any count. The empty file refusal put the prose "an empty file" through repr, so it read as literal file content. _refuse now takes an already rendered fragment and the three real call sites pass repr themselves. carried() indexed the first rule of the marker block unguarded, so --write against a block with no rule died with IndexError after paying for the whole timing run. It hands back an empty table instead and the block is filled. --- benchmarks/bench.py | 2 ++ src/nodrill/_audit.py | 10 +++++----- tests/test_audit.py | 7 +++++-- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/benchmarks/bench.py b/benchmarks/bench.py index 71f36ae..938de78 100644 --- a/benchmarks/bench.py +++ b/benchmarks/bench.py @@ -255,6 +255,8 @@ def carried(document: str) -> str: end = document.index(END, start) block = document[start:end].splitlines() rules = [number for number, line in enumerate(block) if line.startswith("==")] + if not rules: + return "" return "\n".join(block[rules[0] : rules[-1] + 1]) diff --git a/src/nodrill/_audit.py b/src/nodrill/_audit.py index ed3210f..6d9d119 100644 --- a/src/nodrill/_audit.py +++ b/src/nodrill/_audit.py @@ -74,7 +74,7 @@ def _visible(reads: _Reads) -> _Reads: def _refuse(source: str, saw: str, expected: str) -> ValueError: """Build the one refusal, so a caller can say which file and what it expected.""" return ValueError( - f"{source} is not a nodrill contract this version reads. {expected}, found {saw!r}" + f"{source} is not a nodrill contract this version reads. {expected}, found {saw}" ) @@ -82,16 +82,16 @@ def _parse(text: str, source: str) -> _Reads: """Read a contract or a shard back, refusing anything this reader does not know.""" lines = text.splitlines() if not lines or lines[0] != _HEADER: - opening = lines[0] if lines else "an empty file" + opening = repr(lines[0]) if lines else "an empty file" raise _refuse(source, opening, f"Expected {_HEADER!r} on the first line") found: _Reads = set() for number, line in enumerate(lines[1:], start=2): fields = line.split(_GAP) if len(fields) != 3: # noqa: PLR2004 - raise _refuse(source, line, f"Expected three fields on line {number}") + raise _refuse(source, repr(line), f"Expected three fields on line {number}") entry, verb, key = fields if verb not in _VERBS: - raise _refuse(source, verb, f"Expected one of {sorted(_VERBS)} on line {number}") + raise _refuse(source, repr(verb), f"Expected one of {sorted(_VERBS)} on line {number}") found.add((entry, verb, key)) return found @@ -158,7 +158,7 @@ def _summary(reads: _Reads, shards: int, stale: int) -> str: f"A contract is only as complete as the run that recorded it." ) if stale: - said += f" {_counted(stale, 'shard')} from an earlier run were left out." + said += f" Left out {_counted(stale, 'shard')} from an earlier run." return said diff --git a/tests/test_audit.py b/tests/test_audit.py index 78ec32c..6119f34 100644 --- a/tests/test_audit.py +++ b/tests/test_audit.py @@ -618,8 +618,11 @@ def test_one_of_each_reads_as_a_sentence(self) -> None: "nodrill: 1 fact under 1 entry point, recorded from 1 process." ) - def test_shards_left_out_are_said_rather_than_dropped_quietly(self) -> None: - assert _summary(set(), 1, 3).endswith("3 shards from an earlier run were left out.") + @pytest.mark.parametrize(("stale", "said"), [(1, "1 shard"), (3, "3 shards")]) + def test_shards_left_out_are_said_rather_than_dropped_quietly( + self, stale: int, said: str + ) -> None: + assert _summary(set(), 1, stale).endswith(f"Left out {said} from an earlier run.") @pytest.mark.parametrize( ("count", "rendered"), [(0, "0 processes"), (1, "1 process"), (2, "2 processes")] From e6e2500c968feab9001d926378d16a5512625d5e Mon Sep 17 00:00:00 2001 From: Pavel Kutsenko Date: Wed, 26 Aug 2026 11:57:32 +0300 Subject: [PATCH 10/12] docs: bring the comments and docstrings back in line with the code The bench module docstring said the comparable rows are reached five ways, while ORDER has six and the performance page says six. Its label comment credited the README, which carries no table, rather than the performance page that does. The shortened __copy__ comment in _frozen and _sealed had dropped the reason it exists, that on the instance __getattr__ would hand copy the target's own hook. The __main__ docstring spent half its length explaining the __name__ guard, which every Python reader already knows. CONTRIBUTING said pyright checks src, while pyproject includes src and tests/cycle, and its make test cell read as if pytest is always narrowed. serve_http in the audit app claimed it opens both keys the handler reads, and the handler reads four. Two paragraphs in _portable had been edited without reflowing, leaving a short line inside each. --- .github/CONTRIBUTING.md | 4 ++-- benchmarks/bench.py | 4 ++-- src/nodrill/__main__.py | 6 ++---- src/nodrill/_frozen.py | 2 +- src/nodrill/_portable.py | 16 ++++++++-------- src/nodrill/_sealed.py | 2 +- tests/audit_app/app.py | 2 +- 7 files changed, 17 insertions(+), 19 deletions(-) diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 0bf4a8c..7d6d25f 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -32,7 +32,7 @@ Individual pieces, for when you want a faster loop. | `make format` | ruff format plus the safe ruff fixes | | `make lint` | ruff format `--check` and `ruff check` | | `make typecheck` | mypy and pyright | -| `make test` | pytest, narrowed with `make test ARGS="-k inject -x"` | +| `make test` | pytest, which `make test ARGS="-k inject -x"` narrows | | `make testcov` | pytest under coverage with the 100 percent gate | | `make docs` | Sphinx with warnings as errors | | `make audit` | zizmor over the GitHub Actions workflows | @@ -44,7 +44,7 @@ A pull request is expected to pass all of it. - **Coverage is 100 percent on branches.** New code arrives with the tests that cover it. A `# pragma: no cover` is not the fix. -- **Two type checkers.** mypy runs strict over `src` and `tests`, and pyright checks `src`. +- **Two type checkers.** mypy runs strict over `src` and `tests`, and pyright checks `src` plus `tests/cycle`. Both must be clean, and a few API shapes exist only because the two disagree. - **Ruff with `select = ["ALL"]`.** A new ignore goes in `pyproject.toml` with a comment saying why, rather than a bare `# noqa` at the call site. diff --git a/benchmarks/bench.py b/benchmarks/bench.py index 938de78..f288e60 100644 --- a/benchmarks/bench.py +++ b/benchmarks/bench.py @@ -4,7 +4,7 @@ one in docs/content/misc/performance.rst between the markers, so the published numbers and the script that produced them cannot drift apart. -The first rows are one function doing one read, reached five ways, so the +The first rows are one function doing one read, reached six ways, so the rows are comparable to each other and to handing the value in as a parameter, which is what nodrill replaces. The rest price the things the prose claims, entering a scope, entering it with a stack already open, and @@ -86,7 +86,7 @@ def noop() -> None: """Do nothing, so the wrap() row prices wrap() and not its target.""" -# Labels are the README's row headings, so changing one rewrites the published table. +# Labels are the performance page's row headings, so changing one rewrites the published table. PASSED = "one read in a function, value passed in as a parameter" USED = "the same read through `use()`" INJECTED = "the same read through `@inject`" diff --git a/src/nodrill/__main__.py b/src/nodrill/__main__.py index 81e3010..c5020cf 100644 --- a/src/nodrill/__main__.py +++ b/src/nodrill/__main__.py @@ -1,9 +1,7 @@ """Dispatch for python -m nodrill, which is the whole command line surface. -Not a console script, so nothing lands on a PATH and the package still -declares none, which leaves adding one later possible and removing one never -necessary. Guarded, since importing a module must not exit the process that -imported it, and a package walker imports this one like any other. +Not a console script, so nothing lands on a PATH and the package still declares +none, which leaves adding one later possible and removing one never necessary. """ from ._audit import main diff --git a/src/nodrill/_frozen.py b/src/nodrill/_frozen.py index 659690c..bbba45b 100644 --- a/src/nodrill/_frozen.py +++ b/src/nodrill/_frozen.py @@ -70,7 +70,7 @@ def __hash__(self) -> int: def __reduce_ex__(self, protocol: SupportsIndex) -> Any: raise TypeError(_UNCOPYABLE) - # On the class, since copy looks them up on the instance, where __getattr__ answers for it. + # On the class, since on the instance __getattr__ would hand copy the target's own hook. def __copy__(self) -> Any: raise TypeError(_UNCOPYABLE) diff --git a/src/nodrill/_portable.py b/src/nodrill/_portable.py index 6331ef7..a82a1a8 100644 --- a/src/nodrill/_portable.py +++ b/src/nodrill/_portable.py @@ -85,9 +85,9 @@ def adopt( expects, which is what to reach for when the producer is not yours. annotate decides for these blocks what it decides for a provider() block, and annotate=False keeps a payload somebody else wrote out of a traceback - this process renders. What no check can say is - whether the values are true, and an adopted value is input with the same - trust as any other request field. + this process renders. What no check can say is whether the values are + true, and an adopted value is input with the same trust as any other + request field. """ return _adopting(_adopted(payload, only), annotate=annotate) @@ -114,11 +114,11 @@ def set_codec(*, dump: _Hook | None = None, load: _Hook | None = None) -> None: and never writes into it, since the containers below the top level are the exporting block's own. A load runs after the payload has been checked, never before, so a malformed one is refused without reaching - the codec at all. Each call states the whole codec, and - set_codec() with no arguments clears both, while a service that only - produces or only consumes registers the one half it needs. Both ends of - a boundary have to agree on the format, which is why this is startup - configuration rather than something a scope decides. + the codec at all. Each call states the whole codec, and set_codec() with + no arguments clears both, while a service that only produces or only + consumes registers the one half it needs. Both ends of a boundary have to + agree on the format, which is why this is startup configuration rather + than something a scope decides. """ for role, hook in (("dump", dump), ("load", load)): if hook is not None and not callable(hook): diff --git a/src/nodrill/_sealed.py b/src/nodrill/_sealed.py index 8b37def..574ec92 100644 --- a/src/nodrill/_sealed.py +++ b/src/nodrill/_sealed.py @@ -135,7 +135,7 @@ def __dir__(self) -> list[str]: def __reduce_ex__(self, protocol: SupportsIndex) -> Any: raise TypeError(_UNCOPYABLE) - # On the class, since copy looks them up on the instance, where __getattr__ answers for it. + # On the class, since on the instance __getattr__ would hand copy the target's own hook. def __copy__(self) -> Any: raise TypeError(_UNCOPYABLE) diff --git a/tests/audit_app/app.py b/tests/audit_app/app.py index 9af9578..65a5012 100644 --- a/tests/audit_app/app.py +++ b/tests/audit_app/app.py @@ -56,7 +56,7 @@ def running() -> Iterator[None]: def serve_http(name: str) -> str: - """The web entry point, which opens both keys the handler reads.""" + """The web entry point, which opens every key the handler reads but the settings.""" with provider("http request", route="/writes"), provider(User(name)): with provider(Origin("http")): return f"{record_write()} {open_connection()} {use('http request').route}" From bb58ecaa2ba0f0a4c8144cd7a9254795256479f1 Mon Sep 17 00:00:00 2001 From: Pavel Kutsenko Date: Thu, 27 Aug 2026 19:29:27 +0300 Subject: [PATCH 11/12] fix: say what a leftover shard is left over from The summary called leftovers "from an earlier run", which is one run, while three recordings into one directory leave shards from three different ones. It says "from before this run" now, which holds whether the leftovers came from one earlier run, from several, or from one run that had several processes. --- src/nodrill/_audit.py | 2 +- tests/test_audit.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/nodrill/_audit.py b/src/nodrill/_audit.py index 6d9d119..037c10c 100644 --- a/src/nodrill/_audit.py +++ b/src/nodrill/_audit.py @@ -158,7 +158,7 @@ def _summary(reads: _Reads, shards: int, stale: int) -> str: f"A contract is only as complete as the run that recorded it." ) if stale: - said += f" Left out {_counted(stale, 'shard')} from an earlier run." + said += f" Left out {_counted(stale, 'shard')} from before this run." return said diff --git a/tests/test_audit.py b/tests/test_audit.py index 6119f34..2897794 100644 --- a/tests/test_audit.py +++ b/tests/test_audit.py @@ -622,7 +622,7 @@ def test_one_of_each_reads_as_a_sentence(self) -> None: def test_shards_left_out_are_said_rather_than_dropped_quietly( self, stale: int, said: str ) -> None: - assert _summary(set(), 1, stale).endswith(f"Left out {said} from an earlier run.") + assert _summary(set(), 1, stale).endswith(f"Left out {said} from before this run.") @pytest.mark.parametrize( ("count", "rendered"), [(0, "0 processes"), (1, "1 process"), (2, "2 processes")] From fbd548107970b11d43b4944c17996132b84f0f19 Mon Sep 17 00:00:00 2001 From: Pavel Kutsenko Date: Thu, 27 Aug 2026 19:29:38 +0300 Subject: [PATCH 12/12] feat: price what the performance page claimed, and tell two revisions apart The wrap() row was labelled "per call into a thread" and no thread is involved, since wrap() replays a snapshot into a fresh context and returns. It is labelled as the replay it measures, and an Executor.submit row sits beside it for what a real handoff costs, which is more than an order of magnitude on top. Three reads the page discussed had no row, a namespace attribute, which is the shape extend=True and adopt() both produce, a miss that falls back to a call-site default, which turns out to be the dearest read in the table, and a read under debug(unused=True), where the page quoted a multiple nothing reproduced. Measuring it put that multiple at three and a half rather than three, which the two other pages quoting it now say as well. Entering a provider was priced at one provider open and at eight, four percent apart, which reads as flat against a claim that the copy scales with depth. A sixty-four row shows the slope the other two hide. The page told the reader to measure both revisions in one sitting and gave no way to do it. --save records a run and --against compares the next one to it, printing the deltas and marking what a rerun would not explain, and it refuses a file saved on another machine. --passes raises the pass count for a row that will not settle. A run whose rows and ORDER disagree now fails after one pass rather than at render, and the write path refuses a table that does not read back as its own rows. --- benchmarks/bench.py | 232 +++++++++++++----- .../find-out-why-the-context-is-missing.rst | 2 +- docs/content/misc/performance.rst | 35 ++- docs/content/ref/debugging.rst | 2 +- 4 files changed, 200 insertions(+), 71 deletions(-) diff --git a/benchmarks/bench.py b/benchmarks/bench.py index f288e60..02ecc3d 100644 --- a/benchmarks/bench.py +++ b/benchmarks/bench.py @@ -4,11 +4,10 @@ one in docs/content/misc/performance.rst between the markers, so the published numbers and the script that produced them cannot drift apart. -The first rows are one function doing one read, reached six ways, so the -rows are comparable to each other and to handing the value in as a -parameter, which is what nodrill replaces. The rest price the things the -prose claims, entering a scope, entering it with a stack already open, and -crossing into a thread. +The first rows are one function doing one read, reached six ways, so the rows +are comparable to each other and to handing the value in as a parameter, which +is what nodrill replaces. Then the lookups on their own, what a scope costs +to open, and what carrying one to a worker costs. Absolute nanoseconds move with the machine, so the whole table is timed several times over and every row keeps its own best pass. Timing one row to @@ -16,15 +15,18 @@ machine did during its own second, which moved the ratios as well, since the baseline every ratio divides by is one of the rows. -A published number is only replaced when it moved further than a rerun -moves it, so running this on an unchanged tree writes nothing and a diff -means a real change. Nothing here runs in CI, because timing on a shared -runner measures the runner. +A published number is only replaced when it moved further than a rerun moves +it, so running this on an unchanged tree writes nothing and a diff means a +real change. Two revisions are told apart with --save on one and --against +on the other, which prints the deltas and says which of them a rerun would +not explain. Nothing here runs in CI, because timing on a shared runner +measures the runner. """ from __future__ import annotations import argparse +import json import platform import re import sys @@ -36,7 +38,7 @@ from datetime import date from pathlib import Path -from nodrill import FromCtx, inject, injected, lazy, provider, ref, use, wrap +from nodrill import Executor, FromCtx, debug, inject, injected, lazy, provider, ref, use, wrap PAGE = Path(__file__).resolve().parent.parent / "docs/content/misc/performance.rst" START = ".. benchmarks generated by benchmarks/bench.py, do not edit by hand" @@ -45,6 +47,9 @@ # Deep enough that a per-provider cost would show, shallow enough to stay realistic. STACK_DEPTH = 8 +# Deep enough that the slope shows rather than hiding under the fixed cost of a block. +DEEP_STACK = 64 + # What a request scope carries by the time the layers are done accumulating. NAMESPACE_WIDTH = 8 @@ -83,7 +88,7 @@ def read_passed(_request: str, cfg: Config) -> str: def noop() -> None: - """Do nothing, so the wrap() row prices wrap() and not its target.""" + """Do nothing, so the wrap() and Executor rows price the carry and not its target.""" # Labels are the performance page's row headings, so changing one rewrites the published table. @@ -94,19 +99,24 @@ def noop() -> None: SEALED = "the same read through a `sealed=True` provider" LAZY = "the same read through a resolved `lazy` provider" ALONE = "`use(Config)` on its own, without the call frame" +ATTRIBUTE = "`use('scope').field`, one attribute off a namespace" REF = "the same lookup through a `ref()` key" +FALLBACK = "`use('absent', default=...)`, a miss that falls back" +COUNTED = "`use(Config)` under `debug(unused=True)`" REFERENCE = "bare `ContextVar.get()`, for reference" ENTER = "`with provider(...)`, enter and exit" STACKED = f"the same with {STACK_DEPTH} providers already open" +DEEP = f"the same with {DEEP_STACK} providers already open" SEALED_ENTER = "`with provider(..., sealed=True)`, entered and exited" LAZY_ENTER = "`with provider(lazy(...))`, entered and exited unread" EXTEND = f"`with provider(..., extend=True)`, over an {NAMESPACE_WIDTH}-attribute namespace" -THREAD = "`wrap(fn)()`, per call into a thread" +REPLAY = "`wrap(fn)()`, the context replay per call" +WORKER = "`Executor.submit(fn).result()`, a round trip through a worker" # Handing the value in is the alternative nodrill replaces, so it is what the ratios divide by. BASELINE = PASSED -# The published order, comparable reads first, then the floor, then the scope costs. +# The published order, comparable reads first, then the lookups, the scopes and the handoffs. ORDER = ( PASSED, USED, @@ -115,14 +125,19 @@ def noop() -> None: SEALED, LAZY, ALONE, + ATTRIBUTE, REF, + FALLBACK, + COUNTED, REFERENCE, ENTER, STACKED, + DEEP, SEALED_ENTER, LAZY_ENTER, EXTEND, - THREAD, + REPLAY, + WORKER, ) ENTER_STATEMENT = "\nwith provider(config):\n pass\n" @@ -137,11 +152,12 @@ def noop() -> None: (INJECTED, "read_injected('r')"), (ALONE, "use(Config)"), (REF, "use(CONFIG_REF)"), + (FALLBACK, "use('absent', default=None)"), (REFERENCE, "reference.get()"), (ENTER, ENTER_STATEMENT), (SEALED_ENTER, SEALED_ENTER_STATEMENT), (LAZY_ENTER, LAZY_ENTER_STATEMENT), - (THREAD, "bound()"), + (REPLAY, "bound()"), ) @@ -167,12 +183,23 @@ def best_of(passes: int) -> dict[str, float]: """ loops: dict[str, int] = {} best: dict[str, float] = {} - for _ in range(passes): - for label, timing in run(loops).items(): + for number in range(passes): + timings = run(loops) + if not number: + checked(timings) + for label, timing in timings.items(): best[label] = min(best.get(label, timing), timing) return best +def checked(timings: Mapping[str, float]) -> None: + """Refuse a run and an ORDER that disagree, after one pass rather than at render.""" + missing = [label for label in ORDER if label not in timings] + unpublished = [label for label in timings if label not in ORDER] + if missing or unpublished: + raise KeyError(f"ORDER and run() disagree, missing {missing}, unpublished {unpublished}") + + def run(loops: dict[str, int]) -> dict[str, float]: """Time every case once, each under the context its row describes.""" config = Config() @@ -196,19 +223,46 @@ def run(loops: dict[str, int]) -> dict[str, float]: with provider(lazy(Config, Config)): timings[LAZY] = measure(LAZY, "read_used('r')", {**globals(), **locals()}, loops) + # Counting is what a recorded contract installs too, so this row prices both. + with debug(unused=True), provider(config): + timings[COUNTED] = measure(COUNTED, "use(Config)", {**globals(), **locals()}, loops) + # An extending layer copies the enclosing namespace too, so it is priced over a full one. with provider("scope", **{f"field{i}": i for i in range(NAMESPACE_WIDTH)}): - timings[EXTEND] = measure(EXTEND, EXTEND_STATEMENT, {**globals(), **locals()}, loops) + namespace = {**globals(), **locals()} + timings[EXTEND] = measure(EXTEND, EXTEND_STATEMENT, namespace, loops) + # The shape extend=True and adopt() both produce, which is a lookup and one getattr. + timings[ATTRIBUTE] = measure(ATTRIBUTE, "use('scope').field0", namespace, loops) # Entering copies the registry, so the claim that the copy scales with depth is priced here. - with ExitStack() as stack: - for layer in range(STACK_DEPTH): - stack.enter_context(provider(f"layer{layer}")) - timings[STACKED] = measure(STACKED, ENTER_STATEMENT, {**globals(), **locals()}, loops) + for label, depth in ((STACKED, STACK_DEPTH), (DEEP, DEEP_STACK)): + with ExitStack() as stack: + for layer in range(depth): + stack.enter_context(provider(f"layer{layer}")) + timings[label] = measure(label, ENTER_STATEMENT, {**globals(), **locals()}, loops) + + # A real handoff, so the replay row above is not read as the price of a thread. + with provider(config), Executor(max_workers=1) as pool: + pool.submit(noop).result() + timings[WORKER] = measure( + WORKER, "pool.submit(noop).result()", {**globals(), **locals()}, loops + ) return timings +def line(cells: Sequence[str], widths: Sequence[int]) -> str: + """Format one row, padded to the column widths the whole table shares.""" + return " ".join(cell.ljust(width) for cell, width in zip(cells, widths, strict=True)).rstrip() + + +def table(header: Sequence[str], rows: Sequence[Sequence[str]]) -> str: + """Format rows as the reStructuredText simple table both outputs share.""" + widths = [max(len(cell) for cell in column) for column in zip(header, *rows, strict=True)] + rule = " ".join("=" * width for width in widths) + return "\n".join([rule, line(header, widths), rule, *(line(row, widths) for row in rows), rule]) + + def render(timings: Mapping[str, float]) -> str: """Format timings as the reStructuredText block the performance page carries.""" base = timings[BASELINE] @@ -219,27 +273,38 @@ def render(timings: Mapping[str, float]) -> str: (label, f"{round(timings[label])}", ratio(round(timings[label]) / round(base))) for label in ORDER ] - widths = [max(len(cell) for cell in column) for column in zip(header, *rows, strict=True)] - rule = " ".join("=" * width for width in widths) + return table(header, rows) - def line(cells: tuple[str, str, str]) -> str: - return " ".join( - cell.ljust(width) for cell, width in zip(cells, widths, strict=True) - ).rstrip() - lines = [rule, line(header), rule, *(line(row) for row in rows), rule] - return "\n".join(lines) +def compared(fresh: Mapping[str, float], old: Mapping[str, float]) -> str: + """Format this run against a saved one, so a real change reads apart from the weather.""" + rows = [] + for label in ORDER: + now = fresh[label] + was = old.get(label) + if was is None: + rows.append((label, "-", f"{round(now)}", "-", "new")) + continue + change = (now - was) / was + verdict = "moved" if abs(change) > NOISE else "settled" + rows.append((label, f"{round(was)}", f"{round(now)}", f"{change:+.0%}", verdict)) + return table(("operation", "was", "now", "change", "verdict"), rows) -def stamp() -> str: - """Describe the interpreter, the machine and the day that produced these numbers.""" +def machine() -> str: + """Describe the interpreter and the machine, which a comparison has to hold fixed.""" where = platform.platform(terse=True).replace("-", " ") return ( - f"{platform.python_implementation()} {platform.python_version()} on {where}, " - f"{platform.machine()}, measured {date.today().isoformat()}." # noqa: DTZ011 + f"{platform.python_implementation()} {platform.python_version()} " + f"on {where}, {platform.machine()}" ) +def stamp() -> str: + """Describe the interpreter, the machine and the day that produced these numbers.""" + return f"{machine()}, measured {date.today().isoformat()}." # noqa: DTZ011 + + def ratio(times: float) -> str: """Format a multiple of the baseline, keeping a decimal only where it says something.""" return f"{times:.1f}" if times < 10 else str(round(times)) # noqa: PLR2004 @@ -260,9 +325,9 @@ def carried(document: str) -> str: return "\n".join(block[rules[0] : rules[-1] + 1]) -def published(table: str) -> dict[str, float]: +def published(table_text: str) -> dict[str, float]: """Read the numbers the page already carries, so a rerun can leave them where they are.""" - found = (ROW.match(line) for line in table.splitlines()) + found = (ROW.match(line) for line in table_text.splitlines()) return {row[1]: float(row[2]) for row in found if row is not None and row[1] != "operation"} @@ -280,44 +345,91 @@ def steadied(fresh: Mapping[str, float], old: Mapping[str, float]) -> dict[str, return kept -def splice(document: str, table: str) -> str: - """Return document with the region between the markers replaced by table.""" +def splice(document: str, table_text: str) -> str: + """Return document with the region between the markers replaced by the table.""" start = document.index(START) + len(START) end = document.index(END, start) - return f"{document[:start]}\n\n{table}\n{document[end:]}" + return f"{document[:start]}\n\n{table_text}\n{document[end:]}" -def main(argv: Sequence[str] | None = None) -> int: - """Write the table to stdout, or into the performance page with --write.""" - parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) - parser.add_argument( - "--write", - action="store_true", - help="update the rows of the performance page that moved, instead of writing to stdout", - ) - args = parser.parse_args(argv) - - timings = best_of(PASSES) - if not args.write: - sys.stdout.write(f"{render(timings)}\n\n{stamp()}\n") - return 0 - +def write_page(timings: Mapping[str, float]) -> int: + """Rewrite the rows of the performance page that moved, and report what changed.""" document = PAGE.read_text(encoding="utf-8") if START not in document or END not in document: sys.stderr.write(f"{PAGE}: markers {START} and {END} not found\n") return 1 was = carried(document) old = published(was) - timings = steadied(timings, old) - table = render(timings) - if table == was: + steady = steadied(timings, old) + fresh = render(steady) + if set(published(fresh)) != set(ORDER): + sys.stderr.write(f"{PAGE}: the table does not read back as its own rows, nothing written\n") + return 1 + if fresh == was: sys.stderr.write(f"{PAGE}: every row is within {NOISE:.0%} of what it says, left alone\n") return 0 - PAGE.write_text(splice(document, f"{table}\n\n{stamp()}\n"), encoding="utf-8") - moved = [label for label in ORDER if round(timings[label]) != old.get(label)] + PAGE.write_text(splice(document, f"{fresh}\n\n{stamp()}\n"), encoding="utf-8") + moved = [label for label in ORDER if round(steady[label]) != old.get(label)] sys.stderr.write(f"{PAGE}: rewrote {len(moved)} of {len(ORDER)} rows\n") for label in moved: - sys.stderr.write(f" {old.get(label)} -> {round(timings[label])} {label}\n") + sys.stderr.write(f" {old.get(label)} -> {round(steady[label])} {label}\n") + return 0 + + +def against(timings: Mapping[str, float], source: str) -> int: + """Print this run against the one saved in source, refusing another machine's numbers.""" + saved = json.loads(Path(source).read_text(encoding="utf-8")) + if saved.get("machine") != machine(): + sys.stderr.write( + f"{source} was saved on {saved.get('machine')} and this is {machine()}, " + f"so the deltas would mean nothing\n" + ) + return 1 + sys.stdout.write(f"{compared(timings, saved['timings'])}\n\n{stamp()}\n") + return 0 + + +def main(argv: Sequence[str] | None = None) -> int: + """Write the table to stdout, into the performance page, or against a saved run.""" + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + parser.add_argument( + "--passes", + type=int, + default=PASSES, + metavar="N", + help="how many times to time the whole table, keeping each row's best pass", + ) + parser.add_argument( + "--save", + metavar="FILE", + help="also write the raw timings as JSON, for another revision to compare against", + ) + output = parser.add_mutually_exclusive_group() + output.add_argument( + "--write", + action="store_true", + help="update the rows of the performance page that moved, instead of writing to stdout", + ) + output.add_argument( + "--against", + metavar="FILE", + help="print the deltas against timings --save wrote, instead of a table of ns", + ) + args = parser.parse_args(argv) + if args.passes < 1: + parser.error("--passes takes a positive count") + + timings = best_of(args.passes) + if args.save: + saved = {"machine": machine(), "timings": timings} + Path(args.save).write_text(json.dumps(saved, indent=2), encoding="utf-8") + sys.stderr.write(f"{args.save}: saved {len(timings)} rows\n") + + if args.against: + return against(timings, args.against) + if args.write: + return write_page(timings) + sys.stdout.write(f"{render(timings)}\n\n{stamp()}\n") return 0 diff --git a/docs/content/howto/find-out-why-the-context-is-missing.rst b/docs/content/howto/find-out-why-the-context-is-missing.rst index 4ed9c90..12f6d9e 100644 --- a/docs/content/howto/find-out-why-the-context-is-missing.rst +++ b/docs/content/howto/find-out-why-the-context-is-missing.rst @@ -117,7 +117,7 @@ Finding a provider nothing reads That is usually a key that moved or a layer whose readers went away, and nothing else makes it visible. Reads are counted per block, so a shadowed provider is reported even when something read the inner one under the same key. -It is off by default even inside debug mode, since a warning changes what a program prints and a counting read costs roughly three times a plain one, and a block whose body raised is never blamed. +It is off by default even inside debug mode, since a warning changes what a program prints and a counting read costs roughly three and a half times a plain one, and a block whose body raised is never blamed. The warning is an :exc:`~nodrill.UnusedProviderWarning`, so `warnings.filterwarnings` can silence it by category. A miss inside an adopt block diff --git a/docs/content/misc/performance.rst b/docs/content/misc/performance.rst index 0155fb4..4211e71 100644 --- a/docs/content/misc/performance.rst +++ b/docs/content/misc/performance.rst @@ -25,24 +25,29 @@ the same read through a `frozen=True` provider 117 5.1 the same read through a `sealed=True` provider 124 5.4 the same read through a resolved `lazy` provider 136 5.9 `use(Config)` on its own, without the call frame 42 1.8 +`use('scope').field`, one attribute off a namespace 62 2.7 the same lookup through a `ref()` key 143 6.2 +`use('absent', default=...)`, a miss that falls back 227 9.9 +`use(Config)` under `debug(unused=True)` 158 6.9 bare `ContextVar.get()`, for reference 16 0.7 `with provider(...)`, enter and exit 843 37 the same with 8 providers already open 881 38 +the same with 64 providers already open 1277 56 `with provider(..., sealed=True)`, entered and exited 2363 103 `with provider(lazy(...))`, entered and exited unread 1799 78 `with provider(..., extend=True)`, over an 8-attribute namespace 1954 85 -`wrap(fn)()`, per call into a thread 531 23 +`wrap(fn)()`, the context replay per call 550 24 +`Executor.submit(fn).result()`, a round trip through a worker 9120 397 ================================================================ ==== === -CPython 3.14.5 on macOS 26.6, arm64, measured 2026-08-24. +CPython 3.14.5 on macOS 26.6, arm64, measured 2026-08-26. .. end benchmarks The ``×`` column is against handing the value in as a parameter, which is the alternative nodrill removes from the signatures in between. It is the column to read, because it is the one that travels. -Everything here is one thread doing one thing, so what the nanoseconds depend on is how fast one core is, and not how many there are. +Every row but the last is one thread doing one thing, so what the nanoseconds depend on is how fast one core is, and not how many there are. A machine with more cores runs the same row at the same speed, and a server core is often slower at this than a laptop one, so a bigger machine is not a faster table. What a quiet machine buys is a table that says the same thing twice, which is why the numbers are timed over several passes and a row is only republished when it moved further than a rerun moves it. Your own figures will differ and the ratios between them should not, which is the part any claim below rests on. @@ -51,10 +56,16 @@ Reading through :func:`~nodrill.use` costs a little over the parameter it replac :func:`~nodrill.inject` costs more, because it fills the argument before the body runs. ``frozen=True``, :func:`~nodrill.lazy` and ``sealed=True`` add a proxy hop to every attribute the consumer touches, and the sealed hop is the frozen one plus a liveness check, which is the few nanoseconds between those two rows. A :func:`~nodrill.ref` key pays for a Python-level hash and one equality check where a class hashes in C, on the lookups that go through a ref and on no others. +A string-named namespace costs that same lookup and one attribute read on top, which is the shape ``extend=True`` and :func:`~nodrill.adopt` both produce. +A miss that falls back to a call-site ``default=`` is the dearest read here, since it is the only one that leaves the dict and walks the fallback order. +A key read that way on every request is worth providing instead. +``debug(unused=True)`` routes every read through an instrumented registry, which the row above prices at roughly three and a half times a plain hit. +``NODRILL_CONTRACT`` installs the same registry and pays the same, which is why recording a contract belongs in a suite and not in a running service. A request that reads a provided value a hundred times spends microseconds in nodrill, against hundreds of microseconds for one round trip to a database. Entering a provider is the expensive end, because it copies the registry so that sibling tasks stay isolated. -That copy is proportional to how many providers are open, which the ``with provider(...)`` rows price at one and at eight, and it happens once per scope rather than once per lookup. +That copy is proportional to how many providers are open, which the three ``with provider(...)`` rows price at one, at eight and at sixty-four, and it happens once per scope rather than once per lookup. +Most of what a block costs is fixed, so eight providers are barely dearer than one and the sixty-four row is where the copy itself shows. A lazy provider pays for the cell it allocates on top, which is the trade the feature is for, a microsecond on entry against a value that is never built at all on the requests that never read it. An extending layer copies the enclosing namespace on top of the registry, so its row grows with how many attributes have accumulated rather than with how many layers are open, and that second copy is what keeps a sibling task from seeing a layer opened after it started. @@ -63,13 +74,17 @@ That is a per-scope cost paid by the blocks that ask for it, and it buys the sit The second read is what tells an :class:`~contextlib.ExitStack` or an explicit close from a plain ``with``, whose exit is on the line it opened. A provider that does not ask pays one branch on entry and nothing on exit, which is a few percent of opening a scope and nothing at all on a lookup. +Carrying context to a worker is two prices. +:func:`~nodrill.wrap` replays a snapshot into a fresh context on every call, which is all its row measures, since no thread is involved. +Handing the same callable to :class:`~nodrill.Executor` costs the round trip through a worker as well, which the last row prices at more than an order of magnitude on top. +That difference is the thread rather than the context, which is why a worker is worth a batch of work rather than one lookup. + What has no row --------------- -Debug mode has none, because it is not for the hot path. -:func:`~nodrill.debug` makes entering a provider read the stack and write to a ledger, and leaves a lookup that hits costing what it always cost. -``debug(unused=True)`` also routes every read through an instrumented registry, which puts a hit at roughly three times its usual price. -``NODRILL_CONTRACT`` installs the same registry and pays the same, which is why recording a contract belongs in a suite and not in a running service. +Plain :func:`~nodrill.debug` has none on the read side, because a lookup that hits costs what it always cost. +What it costs is a stack read and a ledger write per provider entered, which is the entry path and not the hot one. +Counting reads is the part that reaches a lookup, and that has a row above. Exception notes have none either, because nothing in that path runs until an exception is already leaving a block. A block that exits cleanly costs one pointer comparison more than it did before :func:`~nodrill.annotate_exceptions` existed. @@ -79,7 +94,9 @@ How to read this The absolute numbers move with the machine, and the ratios are the part worth reading. A rerun on one machine lands within ten or fifteen percent, so read the digits as approximate and treat a single-row change of that size as noise rather than as a regression. -The way to tell them apart is to measure both revisions in one sitting, alternating between them, which is what a change to the lookup path is expected to do before it claims anything. +The way to tell them apart is to measure both revisions in one sitting, which is what a change to the lookup path is expected to do before it claims anything. +``--save`` records one and ``--against`` compares the other to it, printing the deltas and marking the rows a rerun would not explain. +Raise ``--passes`` when a row will not settle, since a verdict off a single pass is mostly weather. Regenerate the table with ``make bench ARGS=--write``, which measures on your machine and rewrites the block above. Nothing here runs in CI, because timing on a shared runner measures the runner. diff --git a/docs/content/ref/debugging.rst b/docs/content/ref/debugging.rst index 3eed501..9521f2b 100644 --- a/docs/content/ref/debugging.rst +++ b/docs/content/ref/debugging.rst @@ -66,7 +66,7 @@ debug Debug mode is not for production. Every provider entered reads the stack and writes to the ledger, while a lookup that hits costs what it costs with debug mode off. - ``unused=True`` puts a counting registry in front of every read on top of that, which is roughly three times a plain hit. + ``unused=True`` puts a counting registry in front of every read on top of that, which :ref:`misc-performance` prices at roughly three and a half times a plain hit. :ref:`howto-find-out-why-the-context-is-missing` runs all of it on a live program.