Python semantic layer: semantic_layer() and measure collection - #249
Draft
jat255 wants to merge 15 commits into
Draft
Python semantic layer: semantic_layer() and measure collection#249jat255 wants to merge 15 commits into
jat255 wants to merge 15 commits into
Conversation
jat255
force-pushed
the
jat255/wwmt-layer-collection
branch
from
September 2, 2026 03:14
433b1ce to
fa3ebf0
Compare
jat255
force-pushed
the
jat255/wwmt-layer-collection
branch
from
September 2, 2026 04:02
fa3ebf0 to
f5c7fd7
Compare
jat255
force-pushed
the
jat255/wwmt-layer-collection
branch
from
September 2, 2026 22:26
f5c7fd7 to
04dba7c
Compare
_collect()'s list/tuple branch merged harvested source with sources.update(), so within a nested list the last definition of a Python name won; semantic_layer() itself uses setdefault, so the first wins. Use the same rule in both places so source_text is independent of how measures are nested.
…es on import failure
…against name collisions
…port machinery access
…emantic_layer() calls
_load_module_from_path named every loaded module commons._measure_sources.<stem>_<digest>, so a user's measure file got commons._measure_sources as its __package__: a relative import in their own file failed hunting through commons instead of with Python's own "no known parent package" error, and worse, a name like `.._citations` could resolve into commons' own internals. Switch to a single-segment name with no dotted parent. The digest alone did not make the name unique across repeat loads of the same path, so a second semantic_layer() call on one path clobbered the first load's sys.modules entry, leaving the first layer's Measure.func pointing at a module name that now resolves to the second module. Add a load counter so every load gets its own key; re-execution on repeat loads is unaffected and still matches the R behaviour. Also: fold the stdlib-specific collision check into the general find_spec() check so exactly one error fires per colliding file, and reword its message to state the real direction (stdlib wins, the user's file becomes unreachable, not the reverse); thread the originally requested path through the collision check so a sibling file's collision message says what triggered the scan; move as_measure() and _humanize() next to measure(), which uses them, instead of sitting between unrelated functions; and add a short README to tests/measure_sources/ flagging the two ways that fixture directory silently changes test expectations.
…load counter The load counter minted a fresh sys.modules entry on every load, and none was ever removed: repeated semantic_layer(path) calls, e.g. one per agent session, retained every discarded module and everything it held onto. Cache by resolved path and mtime instead, keyed off the path digest alone. A cache hit reuses the existing module and skips re-execution and the directory-collision check entirely; a miss executes and replaces the entry. This bounds sys.modules growth by the number of distinct files loaded rather than the number of loads. The tradeoff: a file edited mid-process now reloads under the same name, so an earlier layer's Measure.func.__module__ mapping resolves to the newer module object -- a development-time scenario, not a leak. Also: sanitize the path stem before building the module name. A dotted filename like sales.q3.py produced a dotted module name even after the single-segment fix, since path.stem for it is "sales.q3", undoing the fix by giving the loaded file a non-empty __package__ again.
The cache-hit test compared only mtime, never confirming the cached module was actually this file. The module name carries just the first 8 hex characters of the path digest, so two distinct files with the same sanitized stem can collide on that 32-bit value; if their mtimes also matched, a load of the second file silently returned the first file's module and therefore its measures. Add the missing identity check: reuse a cached module only if its own __file__ also resolves to the requested path. That makes the digest's length irrelevant to correctness, since it is then only a name, and a collision degrades to a reload rather than to wrong measures. A miss on a colliding name still replaces the sys.modules entry rather than erroring, which is safe: nothing depends on that name continuing to point at the other file's module, since a Measure holds its function directly, not a lookup through the module name. Also: switch the recorded load-time mtime to st_mtime_ns. A float st_mtime can lose enough filesystem timestamp precision that two rapid edits look identical and a stale module stays cached.
st_mtime_ns fixed our own cache's staleness detection but not SourceFileLoader's: it validates its .pyc by whole-second mtime and size, coarser than what we compare against. An edit within the same second that leaves the file's size unchanged (changing one digit, say) is exactly the case our cache detects and Python's own bytecode cache does not, so the reload ran exec_module() against a module we correctly decided to re-execute, and SourceFileLoader silently handed back the stale compiled code anyway -- worse than staleness, since it looks like a successful reload and returns the wrong answer. Remove the file's own .pyc via importlib.util.cache_from_source() right before re-executing, and only then: on a first load there is nothing stale to invalidate. Best-effort and scoped to the one file being reloaded, since __pycache__ entries are disposable but a permission error removing one should not block loading. Replaced the test's two-second mtime workaround with the real case: same whole second, same file size, pinned explicitly rather than read off the file and nudged so the test cannot straddle a real second boundary and become flaky.
… detected reload Gating _invalidate_bytecode_cache() on name in _load_mtimes only closed the case where this process had already loaded the file. A stale, timestamp-valid .pyc can also predate this process entirely: an earlier process writes it, the file is edited same-second same-size, and a new process's first load of it has no _load_mtimes entry to have noticed anything, so nothing invalidates and SourceFileLoader runs the old bytecode. Same silent wrong answer as before, reached on a first load instead of a reload. Drop the condition and invalidate before every execution. This removes a branch and a state distinction rather than adding one; the cost is recompiling small measure files at construction time, which is rare and cheap given how rarely those files change while any given process is running. The identity check, the st_mtime_ns comparison, and the tolerant error handling around the unlink are unchanged.
jat255
force-pushed
the
jat255/wwmt-layer-collection
branch
from
September 3, 2026 00:31
04dba7c to
39b4f99
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Third of four stacked PRs building the Python semantic layer (M3). Based on #248.
semantic_layer()collects measures from inline decorated functions, bare records, lists, module objects,.pyfile paths, and directories of them. Duplicate names, non-measure items, and missing paths all fail at construction.The layer keeps two mappings, keyed differently on purpose.
measuresis keyed by measure name andsource_textby Python function name. The two differ whenever@measure(name=...)renames a measure, so one map would lose the other. Both are wrapped inMappingProxyType, because a plaindicton a frozen dataclass is still mutable in place. Only text is kept: the worker session reads measure definitions and never receives a callable, which is the security propertypkg-r/R/measures.Rstates in its header.The R package has no equivalent of loading files by path, because
read_measures()sources every measure file into one shared environment and helpers resolve for free. Python modules do not work that way, so a measure file has to import its helpers. Making that true took more than the docstring line originally planned.While loading a file by path, its parent directory goes on
sys.pathand comes off again in afinally, and only if this code put it there. It is appended rather than inserted at the front. Both forms let a sibling import resolve, but prepending also lets ajson.pysitting beside the measures shadow the standard library for everything imported afterwards, which is the usual script footgun and hard to diagnose.Appending removes that hazard and leaves a smaller one, so loading a directory first checks every
.pyfile in it and refuses when a name would collide. The check consultsimportlib.util.find_spec()as well assys.modules, because the case most likely to bite is a package that is installed but not yet imported: without the spec lookup, aduckdb.pynext to the measures passes every other test and then the sibling'simport duckdbsilently resolves to the installed package, leaving the author's helper unreachable with no error.sys.modulesis still consulted alongside it, for a helper another measure directory already loaded under its bare name, which has no discoverable spec.The check, the
sys.pathchange, and module execution are serialized under a reentrant lock. Reentrant because the lock is held while user code runs, and a measure file that constructs another layer during its own import would otherwise deadlock its own thread.A loaded file gets a single-segment module name, sanitized so a dotted filename cannot produce one either. An earlier form used the
commons._measure_sources.prefix, which quietly madecommonsthe import parent of the user's own file: a measure file doingfrom ..util import doublefailed withModuleNotFoundError: No module named 'commons.util', sending the author hunting through this package for a bug in theirs.Modules are cached by resolved path and modification time, so repeated construction reuses them instead of accumulating one
sys.modulesentry per load. A cache hit also checks the cached module's own file, so two files that happen to share a generated name return their own measures rather than each other's. On any execution the source's.pycis invalidated first, because Python's bytecode cache keys on second-resolution mtime plus size: without that, editingreturn 1toreturn 2and reloading within the same second silently returned the old value.Rejected: giving loaded files a synthetic package context so relative imports work. It does not help absolute imports, and relative imports inside a directory that is not a package read strangely.
Not in this PR: injection resolution and the public exports, which are the PR above it.