From fb7bf9e800f9d682e3b8e4b83f3d69cc8642fe69 Mon Sep 17 00:00:00 2001 From: thekevinbot <248217931+thekevinbot@users.noreply.github.com> Date: Fri, 4 Sep 2026 22:01:02 -0400 Subject: [PATCH 1/2] refactor(python): split resolve_config_extensions into one module per function The one-function-per-file gate at max_lines = 1 flags every module-scope function past the first substantial one. resolve_config_extensions.py held four: _load_toml_module, _load_extension_entries, _has_bare_name and the module's named export resolve_config_extension_specs. Keep the export in place and move the three helpers out, one module each: toml_module.py (carrying the _toml binding it produces), load_extension_entries.py and has_bare_name.py. The plural resolver now imports the two it uses from their own modules rather than through the singular one; its test pins where each is bound from. With this the whole packages/python/dirsql tree is clean at max_lines = 1. Refs #1079 --- ...026-09-05-config-extension-loader-split.md | 3 + packages/python/dirsql/has_bare_name.py | 14 ++ packages/python/dirsql/has_bare_name_test.py | 41 +++++ .../python/dirsql/load_extension_entries.py | 28 ++++ .../dirsql/load_extension_entries_test.py | 92 +++++++++++ .../dirsql/resolve_config_extensions.py | 54 +------ .../dirsql/resolve_config_extensions_test.py | 149 ++++-------------- .../dirsql/resolve_configs_extension_specs.py | 3 +- .../resolve_configs_extension_specs_test.py | 14 ++ packages/python/dirsql/toml_module.py | 21 +++ packages/python/dirsql/toml_module_test.py | 40 +++++ 11 files changed, 286 insertions(+), 173 deletions(-) create mode 100644 packages/python/changelog.d/2026-09-05-config-extension-loader-split.md create mode 100644 packages/python/dirsql/has_bare_name.py create mode 100644 packages/python/dirsql/has_bare_name_test.py create mode 100644 packages/python/dirsql/load_extension_entries.py create mode 100644 packages/python/dirsql/load_extension_entries_test.py create mode 100644 packages/python/dirsql/toml_module.py create mode 100644 packages/python/dirsql/toml_module_test.py diff --git a/packages/python/changelog.d/2026-09-05-config-extension-loader-split.md b/packages/python/changelog.d/2026-09-05-config-extension-loader-split.md new file mode 100644 index 000000000..faa85b546 --- /dev/null +++ b/packages/python/changelog.d/2026-09-05-config-extension-loader-split.md @@ -0,0 +1,3 @@ +**Changed** + +- **The config-extension loader split into four modules.** `_load_toml_module` (with the `_toml` binding it produces) moved to `dirsql/toml_module.py`, `_load_extension_entries` to `dirsql/load_extension_entries.py` and `_has_bare_name` to `dirsql/has_bare_name.py`; `dirsql/resolve_config_extensions.py` keeps the intervene-or-not decision, `resolve_config_extension_specs`. Every function keeps its name and behavior — internal reorganisation with no change to the published API or CLI behavior. (#1079) diff --git a/packages/python/dirsql/has_bare_name.py b/packages/python/dirsql/has_bare_name.py new file mode 100644 index 000000000..dabf6dc4d --- /dev/null +++ b/packages/python/dirsql/has_bare_name.py @@ -0,0 +1,14 @@ +"""Whether any of a config's extension entries names a package.""" + +from __future__ import annotations + +from .is_bare_name import is_bare_name + + +def _has_bare_name(entries): + return any( + isinstance(e, dict) + and isinstance(e.get("path"), str) + and is_bare_name(e["path"]) + for e in entries + ) diff --git a/packages/python/dirsql/has_bare_name_test.py b/packages/python/dirsql/has_bare_name_test.py new file mode 100644 index 000000000..9f3d70716 --- /dev/null +++ b/packages/python/dirsql/has_bare_name_test.py @@ -0,0 +1,41 @@ +"""Unit tests for `_has_bare_name`. + +The one collaborator, the pure `is_bare_name`, is mocked so the probe's own +filtering -- non-dict entries, non-string paths, short-circuiting -- is what +these observe. +""" + +from unittest import mock + +import dirsql.has_bare_name as mod + + +def _patch(): + return mock.patch.object( + mod, "is_bare_name", side_effect=lambda path: not path.endswith(".so") + ) + + +def describe_has_bare_name(): + def it_is_false_for_no_entries(): + with _patch() as is_bare_name: + assert mod._has_bare_name([]) is False + is_bare_name.assert_not_called() + + def it_is_false_when_every_path_is_literal(): + with _patch(): + assert mod._has_bare_name([{"path": "ext/a.so"}]) is False + + def it_is_true_when_some_path_is_a_bare_name(): + with _patch(): + assert mod._has_bare_name([{"path": "ext/a.so"}, {"path": "vec"}]) is True + + def it_skips_an_entry_that_is_not_a_table(): + with _patch() as is_bare_name: + assert mod._has_bare_name([1, "vec"]) is False + is_bare_name.assert_not_called() + + def it_skips_an_entry_whose_path_is_not_a_string(): + with _patch() as is_bare_name: + assert mod._has_bare_name([{"path": 42}, {}]) is False + is_bare_name.assert_not_called() diff --git a/packages/python/dirsql/load_extension_entries.py b/packages/python/dirsql/load_extension_entries.py new file mode 100644 index 000000000..120d75886 --- /dev/null +++ b/packages/python/dirsql/load_extension_entries.py @@ -0,0 +1,28 @@ +"""Reading one TOML config's ``[[dirsql.extension]]`` array off disk.""" + +from __future__ import annotations + +import os + +from .toml_module import _toml + + +def _load_extension_entries(config_path): + """Return ``(entries, base_dir)`` for a config's ``[[dirsql.extension]]``. + + ``None`` when the config is missing, unreadable/malformed, or declares no + extension array -- the caller should leave such configs to the core. + """ + if not os.path.isfile(config_path): + return None + try: + with open(config_path, "rb") as f: + doc = _toml.load(f) + except (OSError, _toml.TOMLDecodeError): + # Leave a malformed / unreadable config for the core to report. + return None + + entries = (doc.get("dirsql") or {}).get("extension") + if not isinstance(entries, list): + return None + return entries, os.path.dirname(os.path.abspath(config_path)) diff --git a/packages/python/dirsql/load_extension_entries_test.py b/packages/python/dirsql/load_extension_entries_test.py new file mode 100644 index 000000000..25efc6a2e --- /dev/null +++ b/packages/python/dirsql/load_extension_entries_test.py @@ -0,0 +1,92 @@ +"""Unit tests for `_load_extension_entries`. + +Real `.dirsql.toml` files under a temp dir are parsed by the real TOML module: +the unit under test is the shape-checking around the parse, and a stub parser +would only restate it. The unreadable-file arm mocks `open`. +""" + +import os +import tempfile +from unittest import mock + +import pytest + +import dirsql.load_extension_entries as mod + + +@pytest.fixture +def cfg_dir(): + with tempfile.TemporaryDirectory() as d: + yield d + + +def _write(cfg_dir, body): + path = os.path.join(cfg_dir, ".dirsql.toml") + with open(path, "w") as f: + f.write(body) + return path + + +def describe_configs_with_no_usable_extension_array(): + def it_returns_none_when_the_config_is_missing(): + assert mod._load_extension_entries("/nope/.dirsql.toml") is None + + def it_returns_none_on_malformed_toml(cfg_dir): + path = _write(cfg_dir, "this is not = valid = toml") + assert mod._load_extension_entries(path) is None + + def it_returns_none_when_the_config_cannot_be_read(cfg_dir): + path = _write(cfg_dir, '[[dirsql.extension]]\npath = "vec"\n') + with mock.patch("builtins.open", side_effect=OSError("denied")): + assert mod._load_extension_entries(path) is None + + def it_returns_none_when_there_is_no_dirsql_section(cfg_dir): + path = _write( + cfg_dir, + '[[table]]\nname = "t"\nddl = "CREATE TABLE t (x TEXT)"\nglob = "*"\n', + ) + assert mod._load_extension_entries(path) is None + + def it_returns_none_when_no_extensions_are_declared(cfg_dir): + path = _write(cfg_dir, '[dirsql]\nignore = ["x"]\n') + assert mod._load_extension_entries(path) is None + + def it_returns_none_when_extension_is_not_a_list(cfg_dir): + path = _write(cfg_dir, '[dirsql]\nextension = "not-a-list"\n') + assert mod._load_extension_entries(path) is None + + +def describe_configs_declaring_extensions(): + def it_returns_the_entries_and_the_configs_own_directory(cfg_dir): + path = _write( + cfg_dir, + "[[dirsql.extension]]\n" + 'path = "sqlite_vec"\n' + 'entrypoint = "sqlite3_vec_init"\n\n' + "[[dirsql.extension]]\n" + 'path = "ext/local.so"\n', + ) + assert mod._load_extension_entries(path) == ( + [ + {"path": "sqlite_vec", "entrypoint": "sqlite3_vec_init"}, + {"path": "ext/local.so"}, + ], + cfg_dir, + ) + + def it_returns_entries_that_are_not_tables_verbatim(cfg_dir): + # Shape-checking each entry is the caller's job; this only asserts the + # array is a list. + path = _write(cfg_dir, "[dirsql]\nextension = [1, 2]\n") + assert mod._load_extension_entries(path) == ([1, 2], cfg_dir) + + def it_normalizes_the_base_directory(cfg_dir): + # A traversal segment in the config path must not survive into the + # base directory every extension is then resolved against. + sub = os.path.join(cfg_dir, "sub") + os.mkdir(sub) + _write(sub, '[[dirsql.extension]]\npath = "vec"\n') + _, base = mod._load_extension_entries( + os.path.join(cfg_dir, "sub", "..", "sub", ".dirsql.toml") + ) + assert base == sub diff --git a/packages/python/dirsql/resolve_config_extensions.py b/packages/python/dirsql/resolve_config_extensions.py index 4470ac52c..35b22d726 100644 --- a/packages/python/dirsql/resolve_config_extensions.py +++ b/packages/python/dirsql/resolve_config_extensions.py @@ -14,59 +14,9 @@ from __future__ import annotations -import os -import sys -from importlib import import_module - +from .has_bare_name import _has_bare_name +from .load_extension_entries import _load_extension_entries from .resolve_entries import _resolve_entries -from .resolve_extension import is_bare_name - - -def _load_toml_module(): - """Return the TOML parser module for the running interpreter. - - ``tomllib`` is stdlib only on 3.11+; on 3.10 the ``tomli`` backport it was - derived from (a version-gated dependency) provides the same surface. - Imported by name so a unit test can exercise both arms on any - interpreter -- a literal ``import tomllib`` is unreachable on 3.10 no - matter what ``sys.version_info`` claims. - """ - if sys.version_info >= (3, 11): - return import_module("tomllib") - return import_module("tomli") - - -_toml = _load_toml_module() - - -def _load_extension_entries(config_path): - """Return ``(entries, base_dir)`` for a config's ``[[dirsql.extension]]``. - - ``None`` when the config is missing, unreadable/malformed, or declares no - extension array -- the caller should leave such configs to the core. - """ - if not os.path.isfile(config_path): - return None - try: - with open(config_path, "rb") as f: - doc = _toml.load(f) - except (OSError, _toml.TOMLDecodeError): - # Leave a malformed / unreadable config for the core to report. - return None - - entries = (doc.get("dirsql") or {}).get("extension") - if not isinstance(entries, list): - return None - return entries, os.path.dirname(os.path.abspath(config_path)) - - -def _has_bare_name(entries): - return any( - isinstance(e, dict) - and isinstance(e.get("path"), str) - and is_bare_name(e["path"]) - for e in entries - ) def resolve_config_extension_specs(config_path): diff --git a/packages/python/dirsql/resolve_config_extensions_test.py b/packages/python/dirsql/resolve_config_extensions_test.py index 70be44c87..ed1fae286 100644 --- a/packages/python/dirsql/resolve_config_extensions_test.py +++ b/packages/python/dirsql/resolve_config_extensions_test.py @@ -1,132 +1,41 @@ """Unit tests for `resolve_config_extension_specs`. -Real `.dirsql.toml` files are parsed; `_resolve_entries` (which owns the -effectful per-entry resolution) is mocked, returning a `base:path` sentinel -per entry. The pure `is_bare_name` stays real. +Every collaborator -- the config reader, the bare-name probe and the effectful +per-entry resolution -- is mocked, so these isolate the intervene-or-not +decision from the filesystem. """ -import os -import sys -import tempfile from unittest import mock -import pytest +import dirsql.resolve_config_extensions as mod -import dirsql.resolve_config_extensions as rce - -@pytest.fixture -def cfg_dir(): - with tempfile.TemporaryDirectory() as d: - yield d - - -def _write(path, body): - with open(path, "w") as f: - f.write(body) - - -def _patch(): - return mock.patch.object( - rce, - "_resolve_entries", - side_effect=lambda entries, base: [f"{base}:{e['path']}" for e in entries], +def _patch(*, loaded, bare, resolved="SPECS"): + return ( + mock.patch.object(mod, "_load_extension_entries", return_value=loaded), + mock.patch.object(mod, "_has_bare_name", return_value=bare), + mock.patch.object(mod, "_resolve_entries", return_value=resolved), ) -def describe_load_toml_module(): - # `import_module` is mocked and `version_info` patched so both arms run on - # every interpreter -- neither `tomllib` (absent on 3.10) nor `tomli` - # (absent on 3.11+) is importable everywhere. - def _requested(version): - with ( - mock.patch.object(sys, "version_info", version), - mock.patch.object(rce, "import_module") as import_module, - ): - assert rce._load_toml_module() is import_module.return_value - (name,) = import_module.call_args.args - return name - - # Exactly `(3, 11)`, not `(3, 11, 0)`: the latter compares greater than the - # bare `(3, 11)` literal, so it cannot tell `>=` from `>`. - def it_uses_the_stdlib_parser_on_the_version_that_gained_it(): - assert _requested((3, 11)) == "tomllib" - - def it_uses_the_stdlib_parser_on_newer_versions(): - assert _requested((3, 12, 4)) == "tomllib" - - def it_uses_the_tomli_backport_below_311(): - assert _requested((3, 10, 17)) == "tomli" - - def describe_resolve_config_extension_specs(): - def it_returns_none_when_the_config_is_missing(): - assert rce.resolve_config_extension_specs("/nope/.dirsql.toml") is None - - def it_returns_none_on_malformed_toml(cfg_dir): - path = os.path.join(cfg_dir, ".dirsql.toml") - _write(path, "this is not = valid = toml") - assert rce.resolve_config_extension_specs(path) is None - - def it_returns_none_when_there_is_no_dirsql_section(cfg_dir): - path = os.path.join(cfg_dir, ".dirsql.toml") - _write( - path, '[[table]]\nname = "t"\nddl = "CREATE TABLE t (x TEXT)"\nglob = "*"\n' - ) - assert rce.resolve_config_extension_specs(path) is None - - def it_returns_none_when_no_extensions_are_declared(cfg_dir): - path = os.path.join(cfg_dir, ".dirsql.toml") - _write(path, '[dirsql]\nignore = ["x"]\n') - assert rce.resolve_config_extension_specs(path) is None - - def it_returns_none_when_extension_is_not_a_list(cfg_dir): - path = os.path.join(cfg_dir, ".dirsql.toml") - _write(path, '[dirsql]\nextension = "not-a-list"\n') - assert rce.resolve_config_extension_specs(path) is None - - def it_returns_none_when_extension_entries_are_not_tables(cfg_dir): - path = os.path.join(cfg_dir, ".dirsql.toml") - _write(path, "[dirsql]\nextension = [1, 2]\n") - assert rce.resolve_config_extension_specs(path) is None - - def it_returns_none_when_all_paths_are_literal(cfg_dir): - path = os.path.join(cfg_dir, ".dirsql.toml") - _write(path, '[[dirsql.extension]]\npath = "ext/a.so"\n') - with _patch() as resolve_entries: - assert rce.resolve_config_extension_specs(path) is None - resolve_entries.assert_not_called() - - def it_resolves_every_entry_when_a_path_is_a_bare_package_name(cfg_dir): - path = os.path.join(cfg_dir, ".dirsql.toml") - _write( - path, - "[[dirsql.extension]]\n" - 'path = "sqlite_vec"\n' - 'entrypoint = "sqlite3_vec_init"\n\n' - "[[dirsql.extension]]\n" - 'path = "ext/local.so"\n', - ) - with _patch() as resolve_entries: - specs = rce.resolve_config_extension_specs(path) - assert specs == [f"{cfg_dir}:sqlite_vec", f"{cfg_dir}:ext/local.so"] - resolve_entries.assert_called_once_with( - [ - {"path": "sqlite_vec", "entrypoint": "sqlite3_vec_init"}, - {"path": "ext/local.so"}, - ], - cfg_dir, - ) - - def it_skips_a_non_string_path_in_the_package_name_probe(cfg_dir): - # A dict entry whose `path` isn't a string is not treated as a package - # name; a sibling bare name still triggers resolution of every entry. - path = os.path.join(cfg_dir, ".dirsql.toml") - _write( - path, - "[[dirsql.extension]]\npath = 42\n\n" - '[[dirsql.extension]]\npath = "sqlite_vec"\n', - ) - with _patch(): - specs = rce.resolve_config_extension_specs(path) - assert specs == [f"{cfg_dir}:42", f"{cfg_dir}:sqlite_vec"] + def it_returns_none_when_the_config_declares_no_extensions(): + load, bare, resolve = _patch(loaded=None, bare=True) + with load as loader, bare, resolve as resolve_entries: + assert mod.resolve_config_extension_specs("/cfg/.dirsql.toml") is None + loader.assert_called_once_with("/cfg/.dirsql.toml") + resolve_entries.assert_not_called() + + def it_returns_none_when_every_path_is_literal(): + load, bare, resolve = _patch(loaded=([{"path": "a.so"}], "/cfg"), bare=False) + with load, bare as has_bare_name, resolve as resolve_entries: + assert mod.resolve_config_extension_specs("/cfg/.dirsql.toml") is None + has_bare_name.assert_called_once_with([{"path": "a.so"}]) + resolve_entries.assert_not_called() + + def it_resolves_every_entry_when_a_path_is_a_bare_package_name(): + entries = [{"path": "sqlite_vec"}, {"path": "ext/local.so"}] + load, bare, resolve = _patch(loaded=(entries, "/cfg"), bare=True) + with load, bare, resolve as resolve_entries: + assert mod.resolve_config_extension_specs("/cfg/.dirsql.toml") == "SPECS" + resolve_entries.assert_called_once_with(entries, "/cfg") diff --git a/packages/python/dirsql/resolve_configs_extension_specs.py b/packages/python/dirsql/resolve_configs_extension_specs.py index ed07dbccb..112e917d9 100644 --- a/packages/python/dirsql/resolve_configs_extension_specs.py +++ b/packages/python/dirsql/resolve_configs_extension_specs.py @@ -7,7 +7,8 @@ from __future__ import annotations -from .resolve_config_extensions import _has_bare_name, _load_extension_entries +from .has_bare_name import _has_bare_name +from .load_extension_entries import _load_extension_entries from .resolve_entries import _resolve_entries diff --git a/packages/python/dirsql/resolve_configs_extension_specs_test.py b/packages/python/dirsql/resolve_configs_extension_specs_test.py index c5d64085e..72c095d57 100644 --- a/packages/python/dirsql/resolve_configs_extension_specs_test.py +++ b/packages/python/dirsql/resolve_configs_extension_specs_test.py @@ -75,3 +75,17 @@ def it_resolves_each_config_against_its_own_directory(cfg_dir): with _patch(): specs = mod.resolve_configs_extension_specs([a, b]) assert specs == [f"{cfg_dir}:sqlite_vec", f"{other}:ext/b.so"] + + +def describe_module_wiring(): + # The helpers live in their own modules; these pin where this module binds + # them from, so a mis-pointed import is a failure rather than a silent + # re-export. + def it_reads_each_config_through_the_shared_loader(): + assert mod._load_extension_entries.__module__ == "dirsql.load_extension_entries" + + def it_probes_for_bare_names_through_the_shared_helper(): + assert mod._has_bare_name.__module__ == "dirsql.has_bare_name" + + def it_resolves_entries_through_the_shared_resolver(): + assert mod._resolve_entries.__module__ == "dirsql.resolve_entries" diff --git a/packages/python/dirsql/toml_module.py b/packages/python/dirsql/toml_module.py new file mode 100644 index 000000000..838fddad9 --- /dev/null +++ b/packages/python/dirsql/toml_module.py @@ -0,0 +1,21 @@ +"""The TOML parser this interpreter reads ``.dirsql.toml`` files with.""" + +import sys +from importlib import import_module + + +def _load_toml_module(): + """Return the TOML parser module for the running interpreter. + + ``tomllib`` is stdlib only on 3.11+; on 3.10 the ``tomli`` backport it was + derived from (a version-gated dependency) provides the same surface. + Imported by name so a unit test can exercise both arms on any + interpreter -- a literal ``import tomllib`` is unreachable on 3.10 no + matter what ``sys.version_info`` claims. + """ + if sys.version_info >= (3, 11): + return import_module("tomllib") + return import_module("tomli") + + +_toml = _load_toml_module() diff --git a/packages/python/dirsql/toml_module_test.py b/packages/python/dirsql/toml_module_test.py new file mode 100644 index 000000000..8602c6d83 --- /dev/null +++ b/packages/python/dirsql/toml_module_test.py @@ -0,0 +1,40 @@ +"""Unit tests for `_load_toml_module`. + +`import_module` is mocked and `sys.version_info` patched so both arms run on +every interpreter -- neither `tomllib` (absent on 3.10) nor `tomli` (absent on +3.11+) is importable everywhere. +""" + +from unittest import mock + +import dirsql.toml_module as mod + + +def _requested(version): + with ( + mock.patch.object(mod.sys, "version_info", version), + mock.patch.object(mod, "import_module") as import_module, + ): + assert mod._load_toml_module() is import_module.return_value + (name,) = import_module.call_args.args + return name + + +def describe_load_toml_module(): + # Exactly `(3, 11)`, not `(3, 11, 0)`: the latter compares greater than the + # bare `(3, 11)` literal, so it cannot tell `>=` from `>`. + def it_uses_the_stdlib_parser_on_the_version_that_gained_it(): + assert _requested((3, 11)) == "tomllib" + + def it_uses_the_stdlib_parser_on_newer_versions(): + assert _requested((3, 12, 4)) == "tomllib" + + def it_uses_the_tomli_backport_below_311(): + assert _requested((3, 10, 17)) == "tomli" + + +def describe_module_binding(): + def it_binds_the_parser_the_running_interpreter_asked_for(): + assert mod._toml.__name__ == ( + "tomllib" if mod.sys.version_info >= (3, 11) else "tomli" + ) From 1b53705d7511de73e9f8a50bf86f561caac7f1de Mon Sep 17 00:00:00 2001 From: thekevinbot <248217931+thekevinbot@users.noreply.github.com> Date: Fri, 4 Sep 2026 22:01:48 -0400 Subject: [PATCH 2/2] e2e attestation for claude/1079-p2-resolve-config-extensions --- .../claude-1079-p2-resolve-config-extensions.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 packages/python/e2e-attestations/claude-1079-p2-resolve-config-extensions.json diff --git a/packages/python/e2e-attestations/claude-1079-p2-resolve-config-extensions.json b/packages/python/e2e-attestations/claude-1079-p2-resolve-config-extensions.json new file mode 100644 index 000000000..8dabe3547 --- /dev/null +++ b/packages/python/e2e-attestations/claude-1079-p2-resolve-config-extensions.json @@ -0,0 +1,7 @@ +{ + "command": "just test-e2e", + "ran_at": 1788573708, + "exit_code": 0, + "commit": "fb7bf9e800f9d682e3b8e4b83f3d69cc8642fe69", + "branch": "claude/1079-p2-resolve-config-extensions" +}