Skip to content

Python semantic layer: semantic_layer() and measure collection - #249

Draft
jat255 wants to merge 15 commits into
jat255/wwmt-schema-fixturefrom
jat255/wwmt-layer-collection
Draft

Python semantic layer: semantic_layer() and measure collection#249
jat255 wants to merge 15 commits into
jat255/wwmt-schema-fixturefrom
jat255/wwmt-layer-collection

Conversation

@jat255

@jat255 jat255 commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

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, .py file 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. measures is keyed by measure name and source_text by Python function name. The two differ whenever @measure(name=...) renames a measure, so one map would lose the other. Both are wrapped in MappingProxyType, because a plain dict on 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 property pkg-r/R/measures.R states 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.path and comes off again in a finally, 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 a json.py sitting 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 .py file in it and refuses when a name would collide. The check consults importlib.util.find_spec() as well as sys.modules, because the case most likely to bite is a package that is installed but not yet imported: without the spec lookup, a duckdb.py next to the measures passes every other test and then the sibling's import duckdb silently resolves to the installed package, leaving the author's helper unreachable with no error. sys.modules is 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.path change, 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 made commons the import parent of the user's own file: a measure file doing from ..util import double failed with ModuleNotFoundError: 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.modules entry 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 .pyc is invalidated first, because Python's bytecode cache keys on second-resolution mtime plus size: without that, editing return 1 to return 2 and 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.

@jat255 jat255 changed the title jat255/wwmt layer collection Python semantic layer: semantic_layer() and measure collection Sep 2, 2026
@jat255 jat255 added the py Affects the Python implementation label Sep 2, 2026
@jat255
jat255 force-pushed the jat255/wwmt-layer-collection branch from 433b1ce to fa3ebf0 Compare September 2, 2026 03:14
@jat255
jat255 force-pushed the jat255/wwmt-layer-collection branch from fa3ebf0 to f5c7fd7 Compare September 2, 2026 04:02
@jat255
jat255 force-pushed the jat255/wwmt-layer-collection branch from f5c7fd7 to 04dba7c Compare September 2, 2026 22:26
@jat255 jat255 added this to the py-M3: semantic layer milestone Sep 2, 2026
_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.
_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
jat255 force-pushed the jat255/wwmt-layer-collection branch from 04dba7c to 39b4f99 Compare September 3, 2026 00:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

py Affects the Python implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant