Promote develop to main: hold build_dist's lazily imported audit module

scripts/build_dist.py

@@ -27,6 +27,7 @@
 from __future__ import annotations
 
 import argparse
+import functools
 import hashlib
 import json
 import os
@@ -169,20 +170,38 @@ def write_plugin_manifest(names):
 _MAX_INDENT = 3
 
 
-def _fence_step(line, marker, marker_len):
-    """spec/audit.py's fence reading, imported on first use rather than at module import.
-
-    One reading of CommonMark across the fidelity checks, the review ledger, and this generator,
-    rather than a second one here that could disagree with them. Imported lazily because
-    scripts/skills_install.py imports this module on hosts whose Python predates what audit.py
-    needs, and installing never fills a region.
+# Deliberately lru_cache(maxsize=None) rather than functools.cache, which needs 3.9, and the noqa below is what stops ruff rewriting it back under this repo's own py313 target.
+# The module scripts/skills_install.py imports this one at module scope, and host-setup/*/install-skills.* and scripts/skills_install.sh each commit to Python 3.7+, because that path runs before the fleet's own toolchain exists to install a newer interpreter.
+# So functools.cache here would raise AttributeError at import on 3.7 and 3.8, breaking the one path that has to survive an old host, and spec/host-tools.json's python3 entry records that 3.7 floor and why it sits below the fleet's own.
+@functools.lru_cache(maxsize=None)  # noqa: UP033
+def _audit_module():
+    """spec/audit.py, imported on first use rather than at module import, then held.
+
+    Imported lazily because scripts/skills_install.py imports this module on hosts whose Python
+    predates what audit.py needs, and installing never fills a region. Held because the caller
+    below reaches for it once per line, and re-resolving it there costs a sys.path scan and a
+    sys.modules lookup every time: one pass over the 24 authored skills, 4644 lines across the 40
+    Markdown files they comprise, measured warm in one process, best of five, costs 17.5 ms
+    re-resolved against 0.9 ms held, where calling audit directly costs 0.7 ms. One --check run
+    makes 18708 of these calls, four times that corpus, because it scans the authored tree and
+    both generated trees, and it runs on every pull request. The cache sits here rather than on
+    _fence_step, whose arguments vary per line and would make one unbounded.
     """
     spec = str(ROOT / "spec")
     if spec not in sys.path:
         sys.path.insert(0, spec)
     import audit
 
-    return audit._fence_step(line, marker, marker_len)
+    return audit
+
+
+def _fence_step(line, marker, marker_len):
+    """spec/audit.py's fence reading.
+
+    One reading of CommonMark across the fidelity checks, the review ledger, and this generator,
+    rather than a second one here that could disagree with them.
+    """
+    return _audit_module()._fence_step(line, marker, marker_len)
 
 
 def _lf(text):

scripts/tests/test_build_dist.py

@@ -10,6 +10,7 @@
 import io
 import json
 import os
+import subprocess
 import sys
 import tempfile
 import unittest
@@ -775,11 +776,55 @@ def test_a_body_leaving_a_fence_open_is_refused(self) -> None:
             build_dist.regenerate()
 
     def test_fence_step_adds_the_spec_directory_to_sys_path_once(self) -> None:
+        """The cache would otherwise hide this: held, the accessor's body runs once per process, so
+        the `not in sys.path` guard could be deleted and the path still grow by only one. Clearing
+        between calls makes the body run each time, which is what the guard is there for."""
         before = len(sys.path)
         for _ in range(3):
+            build_dist._audit_module.cache_clear()
             build_dist._fence_step("plain", None, 0)
         self.assertLessEqual(len(sys.path) - before, 1)
 
+    def test_the_audit_module_is_resolved_once_however_many_lines_are_read(self) -> None:
+        """`_fence_step` reaches for it per line, so resolving it per line is what this stops."""
+        build_dist._audit_module.cache_clear()
+
+        for _ in range(50):
+            build_dist._fence_step("plain", None, 0)
+
+        self.assertEqual(build_dist._audit_module.cache_info().misses, 1)
+        self.assertEqual(build_dist._audit_module.cache_info().hits, 49)
+
+    def test_importing_this_module_does_not_import_audit(self) -> None:
+        """The laziness `scripts/skills_install.py` depends on, which caching must not give up.
+
+        It imports this module on hosts whose Python predates what `audit.py` needs, and installing
+        never fills a region, so importing `audit` at module scope would break that path. Run in a
+        fresh interpreter because this test process has already imported `audit` by other means.
+
+        The measurement is the delta across the import rather than the child's absolute state, since
+        a `sitecustomize` or `usercustomize` hook runs before `-c` does and could import a module of
+        that name itself. That would be this case failing to run rather than the import being wrong,
+        which is a boundary to report rather than a finding to raise, so it skips and says so.
+        """
+        scripts_dir = str(Path(build_dist.__file__).parent)
+        source = (
+            "import sys; before = 'audit' in sys.modules;"
+            f" sys.path.insert(0, {scripts_dir!r}); import build_dist;"
+            " print(before, 'audit' in sys.modules, end='')"
+        )
+
+        result = subprocess.run(
+            [sys.executable, "-c", source], capture_output=True, text=True, check=True
+        )
+
+        before, after = result.stdout.split()
+        if before == "True":
+            self.skipTest(
+                "this interpreter imports a module named audit at startup, so the import delta cannot be read here"
+            )
+        self.assertEqual(after, "False", "importing build_dist pulled in audit")
+
 
 if __name__ == "__main__":
     unittest.main()