From 883ace6a51aaa9f606a956c457416c853aeb2aa0 Mon Sep 17 00:00:00 2001 From: khushalsonawat Date: Wed, 11 Nov 2026 11:19:48 +0530 Subject: [PATCH 01/12] fix(harness): answer Dileep's P3 comments on the scan's memory budget - A folder is charged once across the walk's two passes. The side pass charged every folder again, so the folder charge grew to about twice what os.walk holds. - The estimate's sizes name the Python they were measured on (3.12, the hosted kit venv's), and a test fails when that venv's Python changes. The str size is 3.11's, which is high on 3.12 and 3.13. - A held-budget stop while joining logs "passed its memory budget", and says the rows found are dropped. - The unused per-file join constant is dropped. A fan-out test (one entry, 200 hubs, 20,000 leaves) holds the join's charge without its base. TH-9449 Signed-off-by: khushalsonawat (cherry picked from commit 7c8bf7569bed979cf85ebbb0780d5264aa305183) Signed-off-by: khushalsonawat --- src/fi/alk/harness/scan_walk.py | 37 +++++++++++------- tests/harness/test_scan_walk.py | 69 +++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 14 deletions(-) diff --git a/src/fi/alk/harness/scan_walk.py b/src/fi/alk/harness/scan_walk.py index 1c3741dad..ecb91a3b4 100644 --- a/src/fi/alk/harness/scan_walk.py +++ b/src/fi/alk/harness/scan_walk.py @@ -212,20 +212,20 @@ class _Unread(Exception): pass -# CPython 3.12's sizes on a 64-bit build, as asked of the allocator: a str (`_text`), a tuple +# CPython's sizes on a 64-bit build, as asked of the allocator, measured on 3.12, the Python the +# hosted image builds the kit's venv with (`_SIZES_MEASURED_ON`): a str (`_text`, which uses +# 3.11's 49-byte header, so on 3.12 and 3.13, at 41, it is 8 bytes high), a tuple # (`_TUPLE` and a slot per item), a set's own object, one dict or set entry with its share of # the table, a facts object, and the join's share for a file that could be an agent (its sign, # its component bookkeeping, its row) beside three path strings `_project_of` and # `_subfolder` make. Each is then charged at the block the allocator gives it (`_block`). +_SIZES_MEASURED_ON = (3, 12) _TUPLE = 40 _SLOT = 8 _SET = 216 _ENTRY = 112 _FACTS = 512 _JOIN_SHARE = 768 -# What the join holds for every kept file however it's joined: its place in the sorted list of -# paths and in one search's set of files seen. -_JOIN_PER_FILE = 64 # What the join holds whatever was kept: its result, and up to `_MAX_AGENTS` rows with their # names and evidence. Charged once, when the walk starts. A build hazard's own look is # bounded on its own (`_HAZARD_PATH_BYTES`). @@ -268,9 +268,11 @@ def _join_share(path: str) -> int: def _held_for_join(path: str, facts: _Facts) -> int: - """What the join will hold for one kept file: its import list, its place in the sorted - paths and one search's seen set, and, for a file with its own sign, its share.""" - size = _ENTRY + _tuple(len(facts.imports)) + _JOIN_PER_FILE + """What the join will hold for one kept file: its import list, with an entry's charge that + also covers its place in the sorted paths and one search's seen set (measured on a fan-out + of 20,000 files; `test_each_thing_kept_is_charged_at_least_what_it_holds`), and, for a file + with its own sign, its share.""" + size = _ENTRY + _tuple(len(facts.imports)) if _sign(path, facts, {}) is not None: size += _join_share(path) return size @@ -915,7 +917,8 @@ def _source_files( Installed trees and the folders a build skips are never entered. ``side`` False gives only files outside test, example and other side folders; True gives only the files in them. ``charge`` counts each folder kept to enter, at its full path, which is what `os.walk` - holds until it enters it.""" + holds until it enters it. With ``side`` True, the second pass, only folders the first pass + never entered are counted: side folders, and folders below one.""" for directory, directories, files in os.walk(start, followlinks=False): directories.sort() files.sort() @@ -928,9 +931,10 @@ def _source_files( if side is False: directories[:] = [name for name in directories if not _is_side_part(name)] if charge is not None and directories: - charge( - sum(2 * _SLOT + _text(os.path.join(directory, name)) for name in directories) - ) + new = directories + if side is True and not any(_is_side_part(part) for part in relative): + new = [name for name in directories if _is_side_part(name)] + charge(sum(2 * _SLOT + _text(os.path.join(directory, name)) for name in new)) folder = "/".join(relative) for name in files: if side is None or _read_with_side_files(_join(folder, name)) is side: @@ -1473,13 +1477,18 @@ def walk_repo( logger.warning("agent walk: joining files into agents ran out of time") walk.facts.clear() return _empty_result(walk, "clock") - except (_ResultTooBig, _CapReached): + except _ResultTooBig: # Too many candidates reach each other to hold: the same end as the walk's other caps. - # The join's lookups all meet folders the walk already charged, so none charges again; - # if one ever did, past a held stop, it ends here too rather than in the crash path. logger.warning("agent walk: joining files into agents passed its edge budget") walk.facts.clear() return _empty_result(walk, "limit") + except _CapReached: + # The join's lookups all meet folders the walk already charged, so none charges again; + # if one ever did, past a held stop, it ends here rather than in the crash path. The + # rows found so far are dropped with the facts, unlike a stop during the walk. + logger.warning("agent walk: joining files into agents passed its memory budget") + walk.facts.clear() + return _empty_result(walk, "limit") logger.info( "agent walk: files_read=%d unread=%d reader_faults=%d agents=%d left_out=%d stopped=%s", walk.reader.files_read, diff --git a/tests/harness/test_scan_walk.py b/tests/harness/test_scan_walk.py index a781e0381..fe1af2d45 100644 --- a/tests/harness/test_scan_walk.py +++ b/tests/harness/test_scan_walk.py @@ -2297,3 +2297,72 @@ def charging(self: object, **kwargs: object) -> object: monkeypatch.setattr(scan_walk._Walk, "result", charging) assert _walk(tmp_path).stopped == "limit" + + +def test_each_folder_is_charged_once_across_the_two_passes(tmp_path: Path) -> None: + """The side pass walks the customer's own folders again; only the folders the first pass + never entered (side folders, and folders below one) are charged there (Dileep, #273 P3).""" + for index in range(40): + (tmp_path / f"app{index:02}" / "inner").mkdir(parents=True) + (tmp_path / "tests" / f"t{index:02}" / "deep").mkdir(parents=True) + (tmp_path / f"app{index:02}" / "examples" / "e").mkdir(parents=True) + charged: dict[str, int] = {"own": 0, "side": 0, "once": 0} + + def into(key: str): + def charge(size: int) -> None: + charged[key] += size + + return charge + + top = str(tmp_path) + for _ in scan_walk._source_files(top, top, side=False, charge=into("own")): + pass + for _ in scan_walk._source_files(top, top, side=True, charge=into("side")): + pass + for _ in scan_walk._source_files(top, top, charge=into("once")): + pass + + assert charged["own"] > 0 and charged["side"] > 0 + assert charged["own"] + charged["side"] == charged["once"], charged + + +def test_the_estimates_sizes_were_measured_on_the_hosted_kits_python() -> None: + """The sizes are CPython 3.12's. On 3.14 a tuple's header is larger, so the estimate would be + low there (Dileep, #273 P3). This fails when the hosted image's kit venv moves to another + Python, so the sizes are measured again before the bound is trusted.""" + dockerfile = (Path(__file__).resolve().parents[2] / "Dockerfile.hosted").read_text() + major, minor = scan_walk._SIZES_MEASURED_ON + assert f"python{major}.{minor} -m venv /opt/alk-venv" in dockerfile + + +def test_a_memory_budget_stop_while_joining_says_so( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +) -> None: + """A held-budget stop during the join ends at `limit`, with its own cause in the log, not the + edge budget's (Dileep, #273 P3).""" + _files(tmp_path, {"bot.py": CLIENT}) + + def reached(self, *, upload: bool): + raise scan_walk._CapReached() + + monkeypatch.setattr(scan_walk._Walk, "result", reached) + with caplog.at_level("WARNING", logger=scan_walk.logger.name): + result = scan_walk.walk_repo(tmp_path, deadline=FOREVER, clock=lambda: 0.0) + + assert result.stopped == "limit" + assert "passed its memory budget" in caplog.text + assert "edge budget" not in caplog.text + + +def test_a_fan_out_join_is_charged_without_its_base(tmp_path: Path) -> None: + """One entry, 200 hubs, 20,000 leaves: the join walks every leaf. What it really holds is + covered by the per-file charge alone, without `_JOIN_BASE` (Dileep, #273 P3).""" + hubs = {f"h{h:03}.py": "".join(f"import l{h:03}_{n:03}\n" for n in range(100)) for h in range(200)} + leaves = {f"l{h:03}_{n:03}.py": "x = 1\n" for h in range(200) for n in range(100)} + entry = "".join(f"import h{h:03}\n" for h in range(200)) + CLIENT + _files(tmp_path, {"bot.py": entry, **hubs, **leaves}) + + seen = _measure(tmp_path) + + assert seen["join_rise"] > 1_000_000, seen + assert seen["join_rise"] <= seen["join_charged"] - scan_walk._JOIN_BASE, seen From 4ee326c952010dc95e5656e493105aeff1368fc7 Mon Sep 17 00:00:00 2001 From: khushalsonawat Date: Wed, 11 Nov 2026 11:25:26 +0530 Subject: [PATCH 02/12] test(harness): watch os.walk's stack strictly, and only where it has one Dileep's P3: on Python 3.11 os.walk is recursive and has no `stack` local, so the watcher read nothing and the bound test passed vacuously. The watcher now reads `stack` by key, so a missing one fails loudly, and both stack tests are skipped below 3.12 with that reason. TH-9511 Signed-off-by: khushalsonawat (cherry picked from commit b755aab5aa761d115b00fd0bf004df2aef780db7) Signed-off-by: khushalsonawat --- tests/harness/test_framework_detection.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/harness/test_framework_detection.py b/tests/harness/test_framework_detection.py index dfbbf5ea3..6de08382d 100644 --- a/tests/harness/test_framework_detection.py +++ b/tests/harness/test_framework_detection.py @@ -1756,7 +1756,8 @@ def _watch_the_stack(monkeypatch: pytest.MonkeyPatch) -> list[int]: def watching(*args: Any, **kwargs: Any) -> Any: walk = real_walk(*args, **kwargs) for item in walk: - stack = walk.gi_frame.f_locals.get("stack", []) if walk.gi_frame else [] + # A missing `stack` fails loudly rather than reading nothing (Dileep, #275 P3). + stack = walk.gi_frame.f_locals["stack"] if walk.gi_frame else [] held.append(sum(_allocated(sys.getsizeof(path)) + 8 for path in stack if isinstance(path, str))) yield item @@ -1764,6 +1765,12 @@ def watching(*args: Any, **kwargs: Any) -> Any: return held +# os.walk keeps its folders still to visit in a `stack` local only from 3.12; before, it recurses. +_STACK_WALK = pytest.mark.skipif( + sys.version_info < (3, 12), reason="os.walk has a `stack` list to watch only from Python 3.12" +) + + _STACK_CASES = { # top length (None: tmp_path itself), depth, siblings, chain stem, sibling stem # Sized so the whole list passes 2 MiB even under a Linux tmp_path of about 50 characters, and @@ -1777,6 +1784,7 @@ def watching(*args: Any, **kwargs: Any) -> Any: } +@_STACK_WALK @pytest.mark.parametrize("case", list(_STACK_CASES)) def test_the_walks_own_list_never_passes_the_folder_bound( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, case: str @@ -1800,6 +1808,7 @@ def test_the_walks_own_list_never_passes_the_folder_bound( assert max(held) <= bound, (max(held), bound) +@_STACK_WALK def test_the_walks_own_list_grows_past_the_test_bound_without_it( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: From c650e05ff8a5a8e9e33084f9abeb7bd055040c97 Mon Sep 17 00:00:00 2001 From: khushalsonawat Date: Wed, 11 Nov 2026 11:23:39 +0530 Subject: [PATCH 03/12] fix(harness): read the clock inside a huge folder in the source look Dileep's P3: inside one folder the look never read the clock, so a folder of millions of entries ran to the end of its listing past the deadline. It now reads the clock every 4,096 entries (_LOOK_CLOCK_EVERY) and stops as "clock". An empty source still never reads it. TH-9525 Signed-off-by: khushalsonawat (cherry picked from commit 228c33d8f558c749c6df9caf7c8e66bed74532f7) Signed-off-by: khushalsonawat --- src/fi/alk/harness/agent_scan.py | 11 +++++++++-- tests/harness/test_agent_scan.py | 13 +++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/fi/alk/harness/agent_scan.py b/src/fi/alk/harness/agent_scan.py index 821263a45..2a192cd67 100644 --- a/src/fi/alk/harness/agent_scan.py +++ b/src/fi/alk/harness/agent_scan.py @@ -141,6 +141,9 @@ def _check_job(job: HarnessJob) -> HarnessFailure | None: # The most the source look's list of folders still to visit may hold, by its count (TH-9525). _LOOK_HELD = 67_108_864 +# Inside one folder, the clock is read again after this many entries, so a huge folder can't keep +# the look past its deadline. +_LOOK_CLOCK_EVERY = 4_096 def _path_cost(path: str) -> int: @@ -158,7 +161,9 @@ def _look( `_LOOK_HELD` first) or ``"clock"`` (the deadline passed first), with the folders read and what the list held. Each folder is read once, entry by entry: its subfolders are taken on until the bound, and the read goes on past the bound for a regular file, which wins wherever it is in the - folder. A folder or entry that can't be read is passed over, as os.walk does.""" + folder. A folder or entry that can't be read is passed over, as os.walk does. The clock is read + before each folder after the source's own, and every `_LOOK_CLOCK_EVERY` entries inside any + folder; an empty source never reaches either, so it is "none" whatever the time.""" try: if not source.is_dir(): return "none", 0, 0 @@ -179,7 +184,9 @@ def _look( over = False try: with os.scandir(folder) as entries: - for entry in entries: + for index, entry in enumerate(entries, 1): + if not index % _LOOK_CLOCK_EVERY and clock() > deadline: + return "clock", folders, held try: if entry.is_dir(follow_symlinks=False): if not over: diff --git a/tests/harness/test_agent_scan.py b/tests/harness/test_agent_scan.py index 74ad60709..a0707c470 100644 --- a/tests/harness/test_agent_scan.py +++ b/tests/harness/test_agent_scan.py @@ -1536,6 +1536,19 @@ def test_the_clock_is_first_read_after_the_sources_own_folder(tmp_path) -> None: assert (word, folders) == ("clock", 1) +def test_the_clock_is_read_inside_a_huge_folder(tmp_path) -> None: + """One folder of 5,000 empty folders, past the deadline: the look stops partway through that + folder's listing, not after reading all of it (Dileep, #277 P3).""" + source = tmp_path / "src" + folders = _empty_folders(source, [f"f{index:05}" for index in range(5_000)]) + whole = _cost(str(source)) + sum(_cost(folder) for folder in folders) + + word, read, held = agent_scan._look(source, deadline=0.0, clock=lambda: 1.0) + + assert (word, read) == ("clock", 1) + assert held < whole - 500 * _cost(folders[0]) + + def test_the_look_says_why_it_stopped_in_one_line_of_counts(tmp_path, monkeypatch, caplog) -> None: source = tmp_path / "src" folders = _empty_folders(source, [f"folder_{index:03}" for index in range(200)]) From 2295d9ae195b37e6fb6f502386700cdff88e9540 Mon Sep 17 00:00:00 2001 From: khushalsonawat Date: Wed, 11 Nov 2026 11:26:40 +0530 Subject: [PATCH 04/12] fix(harness): sort only the hazard look's folders Dileep's P3 nit: sorting the files changed nothing, since only their count and agent.py's are used. The line is dropped, and the comment says why the folders alone are sorted. TH-9514 Signed-off-by: khushalsonawat (cherry picked from commit 9daa22e839b4800ea4f75a3415619b120e7b2941) Signed-off-by: khushalsonawat --- src/fi/alk/harness/scan_walk.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/fi/alk/harness/scan_walk.py b/src/fi/alk/harness/scan_walk.py index ecb91a3b4..3951929bf 100644 --- a/src/fi/alk/harness/scan_walk.py +++ b/src/fi/alk/harness/scan_walk.py @@ -1551,9 +1551,9 @@ def _hazard( # folders below it. if index and clock() > deadline: return "" - # Sorted, so that a look stopped by a cap stops at the same folder on every file system. + # Folders sorted, so that a look stopped by a cap stops at the same folder on every file + # system. Files need no order: only their count and `agent.py`'s are used. directories.sort() - files.sort() directories[:] = [name for name in directories if name not in _BUILD_IGNORED] found += files.count("agent.py") if found > 1: From c88837374856264e42fcb062b6f10a2f2e2f560d Mon Sep 17 00:00:00 2001 From: khushalsonawat Date: Wed, 11 Nov 2026 11:55:10 +0530 Subject: [PATCH 05/12] test(harness): hold the source look's clock on links and past the bound The change review of the in-folder clock (two reviewers, cleared) found the test never covered links, never passed the 64 MiB bound, and didn't pin where the clock is read. Two tests are added, and the first one now asserts the exact count held at the 4,096th entry. TH-9525 Signed-off-by: khushalsonawat --- tests/harness/test_agent_scan.py | 37 +++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/tests/harness/test_agent_scan.py b/tests/harness/test_agent_scan.py index a0707c470..560b2f694 100644 --- a/tests/harness/test_agent_scan.py +++ b/tests/harness/test_agent_scan.py @@ -1546,7 +1546,42 @@ def test_the_clock_is_read_inside_a_huge_folder(tmp_path) -> None: word, read, held = agent_scan._look(source, deadline=0.0, clock=lambda: 1.0) assert (word, read) == ("clock", 1) - assert held < whole - 500 * _cost(folders[0]) + # Read at the 4,096th entry, after 4,095 folders were taken on: where and how often. + assert held == (agent_scan._LOOK_CLOCK_EVERY - 1) * _cost(folders[0]) < whole + + +def test_the_clock_is_read_inside_a_huge_folder_of_links(tmp_path, monkeypatch) -> None: + """Links are entries too: 5,000 dangling links past the deadline stop at the clock without + each being looked at (Dileep named links).""" + source = tmp_path / "src" + source.mkdir() + for index in range(5_000): + os.symlink("nowhere", source / f"l{index:05}") + calls = [0] + real = os.lstat + + def counted(path, *args, **kwargs): + calls[0] += 1 + return real(path, *args, **kwargs) + + monkeypatch.setattr(agent_scan.os, "lstat", counted) + + word, read, _ = agent_scan._look(source, deadline=0.0, clock=lambda: 1.0) + + assert (word, read) == ("clock", 1) + assert calls[0] < agent_scan._LOOK_CLOCK_EVERY + + +def test_the_clock_is_read_inside_a_huge_folder_past_the_bound(tmp_path, monkeypatch) -> None: + """Past the bound the read goes on for a regular file; the clock still stops it (Dileep's case).""" + source = tmp_path / "src" + folders = _empty_folders(source, [f"f{index:05}" for index in range(5_000)]) + monkeypatch.setattr(agent_scan, "_LOOK_HELD", 50 * _cost(folders[0])) + + word, read, held = agent_scan._look(source, deadline=0.0, clock=lambda: 1.0) + + assert (word, read) == ("clock", 1) + assert held > agent_scan._LOOK_HELD def test_the_look_says_why_it_stopped_in_one_line_of_counts(tmp_path, monkeypatch, caplog) -> None: From 8dcaa826b861869d0fa362a97289e132f89d5670 Mon Sep 17 00:00:00 2001 From: khushalsonawat Date: Wed, 11 Nov 2026 11:55:10 +0530 Subject: [PATCH 06/12] test(harness): drop the stack watcher's fallback that can't run The change review's Low: inside the loop the generator's frame always exists. TH-9511 Signed-off-by: khushalsonawat --- tests/harness/test_framework_detection.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/harness/test_framework_detection.py b/tests/harness/test_framework_detection.py index 6de08382d..1a79d6950 100644 --- a/tests/harness/test_framework_detection.py +++ b/tests/harness/test_framework_detection.py @@ -1757,7 +1757,7 @@ def watching(*args: Any, **kwargs: Any) -> Any: walk = real_walk(*args, **kwargs) for item in walk: # A missing `stack` fails loudly rather than reading nothing (Dileep, #275 P3). - stack = walk.gi_frame.f_locals["stack"] if walk.gi_frame else [] + stack = walk.gi_frame.f_locals["stack"] held.append(sum(_allocated(sys.getsizeof(path)) + 8 for path in stack if isinstance(path, str))) yield item From 209f8f877860076f4b9e84a046c2edf399201ae7 Mon Sep 17 00:00:00 2001 From: khushalsonawat Date: Wed, 11 Nov 2026 11:55:10 +0530 Subject: [PATCH 07/12] fix(harness): keep the join's per-file charge, held by a test that fails without it The change review (round 1, a High) found that dropping _JOIN_PER_FILE let the join's real rise pass its charge, on a fan-out whose leaves each import one shared module. The author's test had leaves that imported nothing, so they shared CPython's empty tuple and hid the cost. The constant is back at 64, and the fan-out test's leaves import a shared module, so the test fails with the constant at 0. TH-9449 Signed-off-by: khushalsonawat --- src/fi/alk/harness/scan_walk.py | 12 +++++++----- tests/harness/test_scan_walk.py | 10 ++++++---- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/fi/alk/harness/scan_walk.py b/src/fi/alk/harness/scan_walk.py index 3951929bf..922491bdf 100644 --- a/src/fi/alk/harness/scan_walk.py +++ b/src/fi/alk/harness/scan_walk.py @@ -226,6 +226,10 @@ class _Unread(Exception): _ENTRY = 112 _FACTS = 512 _JOIN_SHARE = 768 +# What the join holds for every kept file however it's joined: its place in the sorted list of +# paths and in one search's set of files seen. Held by +# `test_a_fan_out_join_is_charged_without_its_base`, which fails with this at 0. +_JOIN_PER_FILE = 64 # What the join holds whatever was kept: its result, and up to `_MAX_AGENTS` rows with their # names and evidence. Charged once, when the walk starts. A build hazard's own look is # bounded on its own (`_HAZARD_PATH_BYTES`). @@ -268,11 +272,9 @@ def _join_share(path: str) -> int: def _held_for_join(path: str, facts: _Facts) -> int: - """What the join will hold for one kept file: its import list, with an entry's charge that - also covers its place in the sorted paths and one search's seen set (measured on a fan-out - of 20,000 files; `test_each_thing_kept_is_charged_at_least_what_it_holds`), and, for a file - with its own sign, its share.""" - size = _ENTRY + _tuple(len(facts.imports)) + """What the join will hold for one kept file: its import list, its place in the sorted + paths and one search's seen set, and, for a file with its own sign, its share.""" + size = _ENTRY + _tuple(len(facts.imports)) + _JOIN_PER_FILE if _sign(path, facts, {}) is not None: size += _join_share(path) return size diff --git a/tests/harness/test_scan_walk.py b/tests/harness/test_scan_walk.py index fe1af2d45..49077de4b 100644 --- a/tests/harness/test_scan_walk.py +++ b/tests/harness/test_scan_walk.py @@ -2355,12 +2355,14 @@ def reached(self, *, upload: bool): def test_a_fan_out_join_is_charged_without_its_base(tmp_path: Path) -> None: - """One entry, 200 hubs, 20,000 leaves: the join walks every leaf. What it really holds is - covered by the per-file charge alone, without `_JOIN_BASE` (Dileep, #273 P3).""" + """One entry, 200 hubs, 20,000 leaves that each import one shared module: the join walks + every leaf. What it really holds is covered by the per-file charge alone, without + `_JOIN_BASE`, and only with `_JOIN_PER_FILE` (Dileep, #273 P3). Leaves that import nothing + share CPython's empty tuple, which would hide the per-file cost.""" hubs = {f"h{h:03}.py": "".join(f"import l{h:03}_{n:03}\n" for n in range(100)) for h in range(200)} - leaves = {f"l{h:03}_{n:03}.py": "x = 1\n" for h in range(200) for n in range(100)} + leaves = {f"l{h:03}_{n:03}.py": "import common\n" for h in range(200) for n in range(100)} entry = "".join(f"import h{h:03}\n" for h in range(200)) + CLIENT - _files(tmp_path, {"bot.py": entry, **hubs, **leaves}) + _files(tmp_path, {"bot.py": entry, "common.py": "x = 1\n", **hubs, **leaves}) seen = _measure(tmp_path) From 0579eb0bd9252cf3b66a33507b49df9ccf8f2589 Mon Sep 17 00:00:00 2001 From: khushalsonawat Date: Wed, 11 Nov 2026 11:56:36 +0530 Subject: [PATCH 08/12] test(harness): cover dot folders in the once-charged test, and anchor the venv line The change review's Lows: the folder-charge test had no dot folder, so a side pass that skipped them went unseen; and the Dockerfile check was a substring, so a later line re-creating the venv on another Python passed. The tree gains .github/workflows and app/.hidden/x, and the check finds the one line that makes /opt/alk-venv. TH-9449 Signed-off-by: khushalsonawat --- tests/harness/test_scan_walk.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/harness/test_scan_walk.py b/tests/harness/test_scan_walk.py index 49077de4b..cc4206a4a 100644 --- a/tests/harness/test_scan_walk.py +++ b/tests/harness/test_scan_walk.py @@ -2306,6 +2306,8 @@ def test_each_folder_is_charged_once_across_the_two_passes(tmp_path: Path) -> No (tmp_path / f"app{index:02}" / "inner").mkdir(parents=True) (tmp_path / "tests" / f"t{index:02}" / "deep").mkdir(parents=True) (tmp_path / f"app{index:02}" / "examples" / "e").mkdir(parents=True) + (tmp_path / f"app{index:02}" / ".hidden" / "x").mkdir(parents=True) + (tmp_path / ".github" / "workflows").mkdir(parents=True) charged: dict[str, int] = {"own": 0, "side": 0, "once": 0} def into(key: str): @@ -2332,7 +2334,10 @@ def test_the_estimates_sizes_were_measured_on_the_hosted_kits_python() -> None: Python, so the sizes are measured again before the bound is trusted.""" dockerfile = (Path(__file__).resolve().parents[2] / "Dockerfile.hosted").read_text() major, minor = scan_walk._SIZES_MEASURED_ON - assert f"python{major}.{minor} -m venv /opt/alk-venv" in dockerfile + made = [line for line in dockerfile.splitlines() if "-m venv" in line and "/opt/alk-venv" in line] + assert len(made) == 1, made + assert f"python{major}.{minor} -m venv /opt/alk-venv" in made[0], made + assert made[0].count("-m venv") == 1, made def test_a_memory_budget_stop_while_joining_says_so( From 832e1faa62049e0f76c04b425156d0d156a41b06 Mon Sep 17 00:00:00 2001 From: khushalsonawat Date: Wed, 11 Nov 2026 11:57:45 +0530 Subject: [PATCH 09/12] test(harness): put the fan-out join test just past the import table's growth Change review B (round 1): at 20,201 files the join's import table sat just below its resize at 21,846, its cheapest point. The test now has 110 hubs of 199 importing leaves (22,002 files). It passes with the per-file charge at 64 and fails at 48, 32 and 0. TH-9449 Signed-off-by: khushalsonawat --- tests/harness/test_scan_walk.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/harness/test_scan_walk.py b/tests/harness/test_scan_walk.py index cc4206a4a..ad8251669 100644 --- a/tests/harness/test_scan_walk.py +++ b/tests/harness/test_scan_walk.py @@ -2360,13 +2360,13 @@ def reached(self, *, upload: bool): def test_a_fan_out_join_is_charged_without_its_base(tmp_path: Path) -> None: - """One entry, 200 hubs, 20,000 leaves that each import one shared module: the join walks - every leaf. What it really holds is covered by the per-file charge alone, without + """One entry, 110 hubs, 21,890 leaves that each import one shared module: the join walks + every leaf, and its import table has just grown (past 21,846 entries), its dearest point. What it really holds is covered by the per-file charge alone, without `_JOIN_BASE`, and only with `_JOIN_PER_FILE` (Dileep, #273 P3). Leaves that import nothing share CPython's empty tuple, which would hide the per-file cost.""" - hubs = {f"h{h:03}.py": "".join(f"import l{h:03}_{n:03}\n" for n in range(100)) for h in range(200)} - leaves = {f"l{h:03}_{n:03}.py": "import common\n" for h in range(200) for n in range(100)} - entry = "".join(f"import h{h:03}\n" for h in range(200)) + CLIENT + hubs = {f"h{h:03}.py": "".join(f"import l{h:03}_{n:03}\n" for n in range(199)) for h in range(110)} + leaves = {f"l{h:03}_{n:03}.py": "import common\n" for h in range(110) for n in range(199)} + entry = "".join(f"import h{h:03}\n" for h in range(110)) + CLIENT _files(tmp_path, {"bot.py": entry, "common.py": "x = 1\n", **hubs, **leaves}) seen = _measure(tmp_path) From edf7bea6bf97f6202cd776b5e91ef2658ada11df Mon Sep 17 00:00:00 2001 From: khushalsonawat Date: Wed, 11 Nov 2026 12:22:31 +0530 Subject: [PATCH 10/12] test(harness): hold the join at its dearest fan-out, and the facts let go at a budget stop Change review round 2 (A, Lows): a constant of 56 still passed the one-entry fan-out. A second shape, two entries over the same hubs, is dearer (about 59.5 bytes a file) and fails at 56. The venv check now counts every line that makes /opt/alk-venv, and the budget-stop test checks the facts are let go before the empty result is built. TH-9449 Signed-off-by: khushalsonawat --- tests/harness/test_scan_walk.py | 46 ++++++++++++++++++++++++--------- 1 file changed, 34 insertions(+), 12 deletions(-) diff --git a/tests/harness/test_scan_walk.py b/tests/harness/test_scan_walk.py index ad8251669..d663b8401 100644 --- a/tests/harness/test_scan_walk.py +++ b/tests/harness/test_scan_walk.py @@ -2334,7 +2334,11 @@ def test_the_estimates_sizes_were_measured_on_the_hosted_kits_python() -> None: Python, so the sizes are measured again before the bound is trusted.""" dockerfile = (Path(__file__).resolve().parents[2] / "Dockerfile.hosted").read_text() major, minor = scan_walk._SIZES_MEASURED_ON - made = [line for line in dockerfile.splitlines() if "-m venv" in line and "/opt/alk-venv" in line] + made = [ + line + for line in dockerfile.splitlines() + if "/opt/alk-venv" in line and ("-m venv" in line or "uv venv" in line or "virtualenv" in line) + ] assert len(made) == 1, made assert f"python{major}.{minor} -m venv /opt/alk-venv" in made[0], made assert made[0].count("-m venv") == 1, made @@ -2351,23 +2355,41 @@ def reached(self, *, upload: bool): raise scan_walk._CapReached() monkeypatch.setattr(scan_walk._Walk, "result", reached) + kept: list[int] = [] + real_empty = scan_walk._empty_result + + def empty(walk, stopped): + kept.append(len(walk.facts)) + return real_empty(walk, stopped) + + monkeypatch.setattr(scan_walk, "_empty_result", empty) with caplog.at_level("WARNING", logger=scan_walk.logger.name): result = scan_walk.walk_repo(tmp_path, deadline=FOREVER, clock=lambda: 0.0) assert result.stopped == "limit" assert "passed its memory budget" in caplog.text assert "edge budget" not in caplog.text - - -def test_a_fan_out_join_is_charged_without_its_base(tmp_path: Path) -> None: - """One entry, 110 hubs, 21,890 leaves that each import one shared module: the join walks - every leaf, and its import table has just grown (past 21,846 entries), its dearest point. What it really holds is covered by the per-file charge alone, without - `_JOIN_BASE`, and only with `_JOIN_PER_FILE` (Dileep, #273 P3). Leaves that import nothing - share CPython's empty tuple, which would hide the per-file cost.""" - hubs = {f"h{h:03}.py": "".join(f"import l{h:03}_{n:03}\n" for n in range(199)) for h in range(110)} - leaves = {f"l{h:03}_{n:03}.py": "import common\n" for h in range(110) for n in range(199)} - entry = "".join(f"import h{h:03}\n" for h in range(110)) + CLIENT - _files(tmp_path, {"bot.py": entry, "common.py": "x = 1\n", **hubs, **leaves}) + assert kept == [0] # the facts are let go before the empty result is built + + +@pytest.mark.parametrize("shape", ["one_entry", "two_entries"]) +def test_a_fan_out_join_is_charged_without_its_base(tmp_path: Path, shape: str) -> None: + """110 hubs of leaves that each import one shared module, about 22,000 files: the join walks + every leaf. One entry puts the import table just past its growth at 21,846 entries. Two + entries are dearer still: the first fills that table, then the second's own search grows its + seen set while the table stays full (about 59.5 bytes a file). What the join really holds is + covered by the per-file charge alone, without `_JOIN_BASE`, and only with `_JOIN_PER_FILE` + at 64: at 56 the two-entry shape fails (Dileep, #273 P3). Leaves that import nothing share + CPython's empty tuple, which would hide the per-file cost.""" + per_hub = 199 if shape == "one_entry" else 198 + hubs = { + f"h{h:03}.py": "".join(f"import l{h:03}_{n:03}\n" for n in range(per_hub)) for h in range(110) + } + leaves = {f"l{h:03}_{n:03}.py": "import common\n" for h in range(110) for n in range(per_hub)} + entries = {"a.py": "".join(f"import h{h:03}\n" for h in range(110)) + CLIENT} + if shape == "two_entries": + entries["b.py"] = "".join(f"import h{h:03}\n" for h in range(100)) + CLIENT + _files(tmp_path, {**entries, "common.py": "x = 1\n", **hubs, **leaves}) seen = _measure(tmp_path) From 4a074b0c71f6b8962308d7c1b4231afdd993b583 Mon Sep 17 00:00:00 2001 From: khushalsonawat Date: Wed, 11 Nov 2026 17:22:54 +0530 Subject: [PATCH 11/12] fix(harness): charge each folder's listing once, and keep reviewers' names out of comments Dileep's P3s on this PR: - With each subfolder charged once, a wide folder's own names, which os.walk holds while the walk is in it, were no longer covered by the old double charge: 50,200 empty folders charged 10.44 MB against a 12.59 MB peak. Each folder's listing is now charged once, when the walk reaches it, before any name is filtered out. A test of 30,000 empty folders holds the walk's peak under the charge. - The new tests' docstrings and comments no longer name the reviewer or the review; the reasons stay. TH-9449 Signed-off-by: khushalsonawat --- src/fi/alk/harness/scan_walk.py | 10 ++++++++-- tests/harness/test_agent_scan.py | 6 +++--- tests/harness/test_framework_detection.py | 2 +- tests/harness/test_scan_walk.py | 19 +++++++++++++++---- 4 files changed, 27 insertions(+), 10 deletions(-) diff --git a/src/fi/alk/harness/scan_walk.py b/src/fi/alk/harness/scan_walk.py index 922491bdf..32f765303 100644 --- a/src/fi/alk/harness/scan_walk.py +++ b/src/fi/alk/harness/scan_walk.py @@ -919,12 +919,18 @@ def _source_files( Installed trees and the folders a build skips are never entered. ``side`` False gives only files outside test, example and other side folders; True gives only the files in them. ``charge`` counts each folder kept to enter, at its full path, which is what `os.walk` - holds until it enters it. With ``side`` True, the second pass, only folders the first pass - never entered are counted: side folders, and folders below one.""" + holds until it enters it, and each folder's own listing, its subfolder and file names, which + `os.walk` holds while the walk is in that folder. With ``side`` True, the second pass, only + folders the first pass never entered are counted: side folders, and folders below one.""" for directory, directories, files in os.walk(start, followlinks=False): directories.sort() files.sort() relative = directory[len(top) :].split(os.sep)[1:] + if charge is not None and ( + side is not True or any(_is_side_part(part) for part in relative) + ): + # Charged once, in the pass that first reaches the folder, before any is filtered out. + charge(sum(_SLOT + _text(name) for name in (*directories, *files))) if relative and any(name.endswith(".dist-info") for name in directories): # An installed tree, not the customer's code. directories[:] = [] diff --git a/tests/harness/test_agent_scan.py b/tests/harness/test_agent_scan.py index 560b2f694..7e6193235 100644 --- a/tests/harness/test_agent_scan.py +++ b/tests/harness/test_agent_scan.py @@ -1538,7 +1538,7 @@ def test_the_clock_is_first_read_after_the_sources_own_folder(tmp_path) -> None: def test_the_clock_is_read_inside_a_huge_folder(tmp_path) -> None: """One folder of 5,000 empty folders, past the deadline: the look stops partway through that - folder's listing, not after reading all of it (Dileep, #277 P3).""" + folder's listing, not after reading all of it.""" source = tmp_path / "src" folders = _empty_folders(source, [f"f{index:05}" for index in range(5_000)]) whole = _cost(str(source)) + sum(_cost(folder) for folder in folders) @@ -1552,7 +1552,7 @@ def test_the_clock_is_read_inside_a_huge_folder(tmp_path) -> None: def test_the_clock_is_read_inside_a_huge_folder_of_links(tmp_path, monkeypatch) -> None: """Links are entries too: 5,000 dangling links past the deadline stop at the clock without - each being looked at (Dileep named links).""" + each being looked at.""" source = tmp_path / "src" source.mkdir() for index in range(5_000): @@ -1573,7 +1573,7 @@ def counted(path, *args, **kwargs): def test_the_clock_is_read_inside_a_huge_folder_past_the_bound(tmp_path, monkeypatch) -> None: - """Past the bound the read goes on for a regular file; the clock still stops it (Dileep's case).""" + """Past the bound the read goes on for a regular file; the clock still stops it.""" source = tmp_path / "src" folders = _empty_folders(source, [f"f{index:05}" for index in range(5_000)]) monkeypatch.setattr(agent_scan, "_LOOK_HELD", 50 * _cost(folders[0])) diff --git a/tests/harness/test_framework_detection.py b/tests/harness/test_framework_detection.py index 1a79d6950..c834526c3 100644 --- a/tests/harness/test_framework_detection.py +++ b/tests/harness/test_framework_detection.py @@ -1756,7 +1756,7 @@ def _watch_the_stack(monkeypatch: pytest.MonkeyPatch) -> list[int]: def watching(*args: Any, **kwargs: Any) -> Any: walk = real_walk(*args, **kwargs) for item in walk: - # A missing `stack` fails loudly rather than reading nothing (Dileep, #275 P3). + # A missing `stack` fails loudly rather than reading nothing. stack = walk.gi_frame.f_locals["stack"] held.append(sum(_allocated(sys.getsizeof(path)) + 8 for path in stack if isinstance(path, str))) yield item diff --git a/tests/harness/test_scan_walk.py b/tests/harness/test_scan_walk.py index d663b8401..9a905b444 100644 --- a/tests/harness/test_scan_walk.py +++ b/tests/harness/test_scan_walk.py @@ -2301,7 +2301,7 @@ def charging(self: object, **kwargs: object) -> object: def test_each_folder_is_charged_once_across_the_two_passes(tmp_path: Path) -> None: """The side pass walks the customer's own folders again; only the folders the first pass - never entered (side folders, and folders below one) are charged there (Dileep, #273 P3).""" + never entered (side folders, and folders below one) are charged there.""" for index in range(40): (tmp_path / f"app{index:02}" / "inner").mkdir(parents=True) (tmp_path / "tests" / f"t{index:02}" / "deep").mkdir(parents=True) @@ -2330,7 +2330,7 @@ def charge(size: int) -> None: def test_the_estimates_sizes_were_measured_on_the_hosted_kits_python() -> None: """The sizes are CPython 3.12's. On 3.14 a tuple's header is larger, so the estimate would be - low there (Dileep, #273 P3). This fails when the hosted image's kit venv moves to another + low there. This fails when the hosted image's kit venv moves to another Python, so the sizes are measured again before the bound is trusted.""" dockerfile = (Path(__file__).resolve().parents[2] / "Dockerfile.hosted").read_text() major, minor = scan_walk._SIZES_MEASURED_ON @@ -2348,7 +2348,7 @@ def test_a_memory_budget_stop_while_joining_says_so( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture ) -> None: """A held-budget stop during the join ends at `limit`, with its own cause in the log, not the - edge budget's (Dileep, #273 P3).""" + edge budget's.""" _files(tmp_path, {"bot.py": CLIENT}) def reached(self, *, upload: bool): @@ -2379,7 +2379,7 @@ def test_a_fan_out_join_is_charged_without_its_base(tmp_path: Path, shape: str) entries are dearer still: the first fills that table, then the second's own search grows its seen set while the table stays full (about 59.5 bytes a file). What the join really holds is covered by the per-file charge alone, without `_JOIN_BASE`, and only with `_JOIN_PER_FILE` - at 64: at 56 the two-entry shape fails (Dileep, #273 P3). Leaves that import nothing share + at 64: at 56 the two-entry shape fails. Leaves that import nothing share CPython's empty tuple, which would hide the per-file cost.""" per_hub = 199 if shape == "one_entry" else 198 hubs = { @@ -2395,3 +2395,14 @@ def test_a_fan_out_join_is_charged_without_its_base(tmp_path: Path, shape: str) assert seen["join_rise"] > 1_000_000, seen assert seen["join_rise"] <= seen["join_charged"] - scan_walk._JOIN_BASE, seen + + +def test_a_wide_folder_is_charged_its_listing(tmp_path: Path) -> None: + """One folder of many empty folders: what `os.walk` holds for the folder it is in, its names, + is charged once when the walk reaches it, so the charge is never below the walk's peak.""" + for index in range(30_000): + (tmp_path / f"f{index:05}").mkdir() + + seen = _measure(tmp_path) + + assert seen["walk_peak"] <= seen["charged"], seen From a9475e4dfcaba509cc5764e3c7351bf6ef692e56 Mon Sep 17 00:00:00 2001 From: khushalsonawat Date: Wed, 11 Nov 2026 17:44:29 +0530 Subject: [PATCH 12/12] fix(harness): sum a folder's names list by list, and hold the charge before the filter The one-reviewer check of the listing charge cleared, with two Mediums: - Building one tuple of every name held a slot a name while os.walk still held the listing, so 250,000 files charged 18.26 MB against a 19.06 MB peak. The two lists are summed apart. - Nothing held "before any name is filtered out". The wide-folder test now covers folders, files and links to a folder (listed as folders, then left out), and a charge moved after the filter fails it. Also: the first-pass test is computed once per folder, and two older test docstrings no longer name the reviewer. TH-9449 Signed-off-by: khushalsonawat --- src/fi/alk/harness/scan_walk.py | 14 ++++++++------ tests/harness/test_scan_walk.py | 25 ++++++++++++++++++------- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/src/fi/alk/harness/scan_walk.py b/src/fi/alk/harness/scan_walk.py index 32f765303..db6891b9a 100644 --- a/src/fi/alk/harness/scan_walk.py +++ b/src/fi/alk/harness/scan_walk.py @@ -926,11 +926,13 @@ def _source_files( directories.sort() files.sort() relative = directory[len(top) :].split(os.sep)[1:] - if charge is not None and ( - side is not True or any(_is_side_part(part) for part in relative) - ): - # Charged once, in the pass that first reaches the folder, before any is filtered out. - charge(sum(_SLOT + _text(name) for name in (*directories, *files))) + first_pass = side is not True or any(_is_side_part(part) for part in relative) + if charge is not None and first_pass: + # Summed list by list: one tuple of every name would itself hold a slot a name. + charge( + sum(_SLOT + _text(name) for name in directories) + + sum(_SLOT + _text(name) for name in files) + ) if relative and any(name.endswith(".dist-info") for name in directories): # An installed tree, not the customer's code. directories[:] = [] @@ -940,7 +942,7 @@ def _source_files( directories[:] = [name for name in directories if not _is_side_part(name)] if charge is not None and directories: new = directories - if side is True and not any(_is_side_part(part) for part in relative): + if not first_pass: new = [name for name in directories if _is_side_part(name)] charge(sum(2 * _SLOT + _text(os.path.join(directory, name)) for name in new)) folder = "/".join(relative) diff --git a/tests/harness/test_scan_walk.py b/tests/harness/test_scan_walk.py index 9a905b444..e54bf9a7d 100644 --- a/tests/harness/test_scan_walk.py +++ b/tests/harness/test_scan_walk.py @@ -1393,7 +1393,7 @@ def broken(data: bytes, shadowed: object) -> object: def test_the_step_after_the_walk_watches_the_clock(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: - """Dileep's shape, small: many files that build a client, each importing a web of modules. + """The reviewed shape, small: many files that build a client, each importing a web of modules. Joining them into agents must stop at its deadline and end as a clock stop, never run on to the helper's hard stop.""" files = { @@ -1430,7 +1430,7 @@ def clock() -> float: def test_the_step_after_the_walk_holds_a_bounded_number_of_edges( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: - """Dileep's shape, small: every candidate imports one package that re-exports modules, so + """The reviewed shape, small: every candidate imports one package that re-exports modules, so each candidate reaches every other and the pairs grow with the square of the candidates.""" files = {"pkg/__init__.py": "".join(f"from .m{index} import x\n" for index in range(20))} files.update({f"pkg/m{index}.py": "x = 1\n" for index in range(20)}) @@ -2397,11 +2397,22 @@ def test_a_fan_out_join_is_charged_without_its_base(tmp_path: Path, shape: str) assert seen["join_rise"] <= seen["join_charged"] - scan_walk._JOIN_BASE, seen -def test_a_wide_folder_is_charged_its_listing(tmp_path: Path) -> None: - """One folder of many empty folders: what `os.walk` holds for the folder it is in, its names, - is charged once when the walk reaches it, so the charge is never below the walk's peak.""" - for index in range(30_000): - (tmp_path / f"f{index:05}").mkdir() +@pytest.mark.parametrize("entries", ["folders", "files", "links"]) +def test_a_wide_folder_is_charged_its_listing(tmp_path: Path, entries: str) -> None: + """One folder of many entries: what `os.walk` holds for the folder it is in, its names, is + charged once when the walk reaches it, before any name is filtered out (links to a folder are + listed as folders and then left out), so the charge is never below the walk's peak.""" + target = tmp_path / "target" + target.mkdir() + wide = tmp_path / "wide" + wide.mkdir() + for index in range(80_000 if entries == "files" else 30_000): + if entries == "folders": + (wide / f"f{index:05}").mkdir() + elif entries == "files": + (wide / f"f{index:05}.txt").touch() + else: + (wide / f"f{index:05}").symlink_to(target) seen = _measure(tmp_path)