From aa25def91f8f4c47efa982d04332436eed89f571 Mon Sep 17 00:00:00 2001 From: James Wiesebron Date: Wed, 29 Apr 2026 10:11:40 -0700 Subject: [PATCH 1/5] Fix #2248: add max-file-size lint to cap Python source-file growth Adds scripts/check-file-sizes.py + scripts/file-size-allowlist.yaml. The lint walks Python sources under orchestrator/, gateway/, shared/, sandbox/, scripts/, and config/ (test files exempt) and rejects files past 1500 lines or 100KB. 15 currently-oversize files are grandfathered with their line + byte baselines; further growth past those baselines is rejected so decomposition follow-ups land monotonically. Wires through the existing scripts/check-*.py auto-discovery in `make lint-custom`. --- scripts/check-file-sizes.py | 293 +++++++++++++++++++++++++ scripts/file-size-allowlist.yaml | 81 +++++++ tests/scripts/test_check_file_sizes.py | 176 +++++++++++++++ 3 files changed, 550 insertions(+) create mode 100755 scripts/check-file-sizes.py create mode 100644 scripts/file-size-allowlist.yaml create mode 100644 tests/scripts/test_check_file_sizes.py diff --git a/scripts/check-file-sizes.py b/scripts/check-file-sizes.py new file mode 100755 index 0000000000..47ee793e98 --- /dev/null +++ b/scripts/check-file-sizes.py @@ -0,0 +1,293 @@ +#!/usr/bin/env python3 +"""Lint check: cap Python source-file line and byte counts. + +Oversize source files force every BRC implement-phase agent into a +Grep-then-paginated-Read workflow because the Read tool's hard limits +(256KB / ~25k tokens) reject the file in one shot. Each paginated read is +another LLM turn, and BRC cycles re-pay the cost from a cleared context. +See issue #2248 for the operational evidence. + +This check walks tracked Python sources under the source roots +(orchestrator/, gateway/, shared/, sandbox/, scripts/, config/) and rejects +any file that exceeds the configured caps: + +- Hard cap (failure): 1500 lines OR 100,000 bytes (~25k tokens of Python). +- Soft cap (warning): 800 lines OR 60,000 bytes (~15k tokens). + +Test files are exempt -- parametrized cases legitimately push line counts +past these caps and decomposing them mechanically would hurt readability. + +Files already over the hard cap on day one are listed in +``scripts/file-size-allowlist.yaml`` with their line and byte baselines. +The lint allows allowlisted files to stay over the cap, but rejects further +growth past the recorded baseline -- decomposition follow-ups land as the +file shrinks. Removing a file from the allowlist (or letting it grow under +the cap) is encouraged as cleanup proceeds. + +Usage: + scripts/check-file-sizes.py + scripts/check-file-sizes.py --update-allowlist # rewrite baselines + scripts/check-file-sizes.py --list # report all files + +Exit codes: + 0 no violations + 1 one or more files exceed caps or grew past their allowlist baseline +""" + +from __future__ import annotations + +import argparse +import sys +from dataclasses import dataclass +from pathlib import Path +from typing import Any + +import yaml + +REPO_ROOT = Path(__file__).resolve().parent.parent +ALLOWLIST_PATH = Path(__file__).resolve().parent / "file-size-allowlist.yaml" + +SOURCE_ROOTS = ("orchestrator", "gateway", "shared", "sandbox", "scripts", "config") + +# Directories under SOURCE_ROOTS that are excluded -- tests bundle parametrized +# cases that legitimately exceed source-file caps. +EXCLUDED_DIR_NAMES = frozenset({"tests", "__pycache__"}) + + +@dataclass(frozen=True) +class Caps: + hard_lines: int + hard_bytes: int + soft_lines: int + soft_bytes: int + + +@dataclass(frozen=True) +class Baseline: + lines: int + bytes: int + + +@dataclass(frozen=True) +class FileStats: + path: Path + lines: int + bytes: int + + +@dataclass +class Config: + caps: Caps + baselines: dict[str, Baseline] + + +def load_config(path: Path = ALLOWLIST_PATH) -> Config: + raw = yaml.safe_load(path.read_text()) or {} + caps_raw = raw.get("caps") or {} + caps = Caps( + hard_lines=int(caps_raw["hard_lines"]), + hard_bytes=int(caps_raw["hard_bytes"]), + soft_lines=int(caps_raw["soft_lines"]), + soft_bytes=int(caps_raw["soft_bytes"]), + ) + files_raw: dict[str, Any] = raw.get("files") or {} + baselines = { + rel: Baseline(lines=int(entry["lines"]), bytes=int(entry["bytes"])) + for rel, entry in files_raw.items() + } + return Config(caps=caps, baselines=baselines) + + +def is_test_file(rel: Path) -> bool: + if rel.name.startswith("test_") or rel.name.endswith("_test.py"): + return True + return any(part in EXCLUDED_DIR_NAMES for part in rel.parts) + + +def iter_source_files(repo_root: Path = REPO_ROOT) -> list[Path]: + """Yield every tracked Python source file under SOURCE_ROOTS, sorted.""" + out: list[Path] = [] + for root_name in SOURCE_ROOTS: + root = repo_root / root_name + if not root.is_dir(): + continue + for p in root.rglob("*.py"): + rel = p.relative_to(repo_root) + if is_test_file(rel): + continue + out.append(p) + out.sort() + return out + + +def measure(path: Path) -> FileStats: + data = path.read_bytes() + # Count physical lines without splitting the entire file twice. + line_count = data.count(b"\n") + if data and not data.endswith(b"\n"): + line_count += 1 + return FileStats(path=path, lines=line_count, bytes=len(data)) + + +def evaluate( + stats: FileStats, + rel: str, + config: Config, +) -> tuple[list[str], list[str]]: + """Return (errors, warnings) for a single file.""" + errors: list[str] = [] + warnings: list[str] = [] + caps = config.caps + baseline = config.baselines.get(rel) + + over_hard_lines = stats.lines > caps.hard_lines + over_hard_bytes = stats.bytes > caps.hard_bytes + + if over_hard_lines or over_hard_bytes: + if baseline is None: + errors.append( + f"{rel}: {stats.lines} lines / {stats.bytes} bytes exceeds hard cap " + f"({caps.hard_lines} lines / {caps.hard_bytes} bytes). " + "Decompose the file or, if you cannot in this PR, add it to " + "scripts/file-size-allowlist.yaml with a tracking issue." + ) + else: + if stats.lines > baseline.lines: + errors.append( + f"{rel}: {stats.lines} lines exceeds allowlist baseline " + f"({baseline.lines}). Reduce the file or open a separate PR " + "raising the baseline with justification." + ) + if stats.bytes > baseline.bytes: + errors.append( + f"{rel}: {stats.bytes} bytes exceeds allowlist baseline " + f"({baseline.bytes}). Reduce the file or open a separate PR " + "raising the baseline with justification." + ) + return errors, warnings + + # Soft warnings are skipped for allowlisted files -- they're already + # tracked for decomposition. + if baseline is not None: + return errors, warnings + + if stats.lines > caps.soft_lines: + warnings.append( + f"{rel}: {stats.lines} lines exceeds soft cap ({caps.soft_lines}). " + "Consider decomposing before it hits the hard cap." + ) + if stats.bytes > caps.soft_bytes: + warnings.append( + f"{rel}: {stats.bytes} bytes exceeds soft cap ({caps.soft_bytes}). " + "Consider decomposing before it hits the hard cap." + ) + return errors, warnings + + +def check_all(repo_root: Path = REPO_ROOT) -> tuple[list[str], list[str], list[str]]: + """Run the full check. Returns (errors, warnings, stale_allowlist_entries).""" + config = load_config() + errors: list[str] = [] + warnings: list[str] = [] + seen: set[str] = set() + + for path in iter_source_files(repo_root): + rel = str(path.relative_to(repo_root)) + seen.add(rel) + stats = measure(path) + file_errors, file_warnings = evaluate(stats, rel, config) + errors.extend(file_errors) + warnings.extend(file_warnings) + + stale = sorted(rel for rel in config.baselines if rel not in seen) + return errors, warnings, stale + + +def write_allowlist(config: Config, baselines: dict[str, Baseline]) -> None: + """Rewrite the allowlist file preserving caps + sorted baselines.""" + payload: dict[str, Any] = { + "caps": { + "hard_lines": config.caps.hard_lines, + "hard_bytes": config.caps.hard_bytes, + "soft_lines": config.caps.soft_lines, + "soft_bytes": config.caps.soft_bytes, + }, + "files": { + rel: {"lines": b.lines, "bytes": b.bytes} for rel, b in sorted(baselines.items()) + }, + } + ALLOWLIST_PATH.write_text(yaml.safe_dump(payload, sort_keys=False)) + + +def update_allowlist(repo_root: Path = REPO_ROOT) -> int: + """Refresh allowlist baselines from current file sizes.""" + config = load_config() + new_baselines: dict[str, Baseline] = {} + for path in iter_source_files(repo_root): + rel = str(path.relative_to(repo_root)) + stats = measure(path) + if stats.lines > config.caps.hard_lines or stats.bytes > config.caps.hard_bytes: + new_baselines[rel] = Baseline(lines=stats.lines, bytes=stats.bytes) + write_allowlist(config, new_baselines) + print(f"Wrote {len(new_baselines)} entries to {ALLOWLIST_PATH.name}") + return 0 + + +def list_files(repo_root: Path = REPO_ROOT) -> int: + """Print every Python source file with its line and byte counts.""" + rows = [] + for path in iter_source_files(repo_root): + rel = str(path.relative_to(repo_root)) + stats = measure(path) + rows.append((stats.lines, stats.bytes, rel)) + rows.sort(reverse=True) + for lines, bts, rel in rows: + print(f"{lines:>6} {bts:>8} {rel}") + return 0 + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__.split("\n")[0] if __doc__ else "") + parser.add_argument( + "--update-allowlist", + action="store_true", + help="Rewrite scripts/file-size-allowlist.yaml from current file sizes.", + ) + parser.add_argument( + "--list", + action="store_true", + help="List every source file with its size; takes no action.", + ) + args = parser.parse_args(argv) + + if args.update_allowlist: + return update_allowlist() + if args.list: + return list_files() + + errors, warnings, stale = check_all() + + for w in warnings: + print(f"warning: {w}") + + for s in stale: + print( + f"warning: stale allowlist entry: {s} no longer exists. " + "Remove it from scripts/file-size-allowlist.yaml." + ) + + if errors: + print() + print("ERROR: file-size lint failed") + print("=" * 76) + for e in errors: + print(f" - {e}") + print() + print("Run `scripts/check-file-sizes.py --list` to see all files ranked by size.") + return 1 + + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/file-size-allowlist.yaml b/scripts/file-size-allowlist.yaml new file mode 100644 index 0000000000..a6c5d4eb9c --- /dev/null +++ b/scripts/file-size-allowlist.yaml @@ -0,0 +1,81 @@ +# Allowlist for scripts/check-file-sizes.py. +# +# Each entry grandfathers a Python source file at the line and byte count it +# had when added here. The lint allows files in this list to remain over the +# global cap, but rejects any further growth past the recorded baselines. +# +# Decompose listed files in follow-up PRs and remove them from the allowlist +# when they drop back under the global cap. New files are NOT eligible — add +# only files that pre-date the lint or whose decomposition is already tracked +# as a separate issue. +# +# Schema: +# caps: { hard_lines, hard_bytes, soft_lines, soft_bytes } +# files: { : { lines: int, bytes: int, issue: str|null } } +caps: + hard_lines: 1500 + hard_bytes: 100000 + soft_lines: 800 + soft_bytes: 60000 + +files: + orchestrator/routes/pipelines.py: + lines: 15356 + bytes: 669950 + issue: "2248" + gateway/gateway.py: + lines: 9753 + bytes: 373015 + issue: "2248" + sandbox/egg_lib/orch_cli.py: + lines: 3512 + bytes: 127924 + issue: "2248" + orchestrator/mcp_tools.py: + lines: 2817 + bytes: 118448 + issue: "2248" + orchestrator/gateway_client.py: + lines: 2392 + bytes: 88655 + issue: "2248" + shared/egg_contracts/checkpoint_cli.py: + lines: 2233 + bytes: 81972 + issue: "2248" + sandbox/entrypoint.py: + lines: 2109 + bytes: 85051 + issue: "2248" + gateway/worktree_manager.py: + lines: 2090 + bytes: 83664 + issue: "2248" + gateway/git_client.py: + lines: 2032 + bytes: 66936 + issue: "2248" + orchestrator/overseer/monitor.py: + lines: 2005 + bytes: 83921 + issue: "2248" + orchestrator/peer_consensus.py: + lines: 1988 + bytes: 85268 + issue: "2248" + orchestrator/routes/signals.py: + lines: 1986 + bytes: 74042 + issue: "2248" + gateway/checkpoint_handler.py: + lines: 1655 + bytes: 61597 + issue: "2248" + scripts/select_tests.py: + lines: 1650 + bytes: 63828 + issue: "2248" + orchestrator/routes/deployment.py: + lines: 1604 + bytes: 56130 + issue: "2248" diff --git a/tests/scripts/test_check_file_sizes.py b/tests/scripts/test_check_file_sizes.py new file mode 100644 index 0000000000..389e62bdb5 --- /dev/null +++ b/tests/scripts/test_check_file_sizes.py @@ -0,0 +1,176 @@ +"""Tests for scripts/check-file-sizes.py.""" + +from __future__ import annotations + +import importlib.util +import sys +from pathlib import Path + +import pytest + +_SCRIPT_PATH = Path(__file__).resolve().parents[2] / "scripts" / "check-file-sizes.py" +_spec = importlib.util.spec_from_file_location("check_file_sizes", _SCRIPT_PATH) +assert _spec and _spec.loader +_mod = importlib.util.module_from_spec(_spec) +# Register in sys.modules so dataclasses can resolve __module__ during class +# construction (Python 3.14 _is_type lookup). +sys.modules["check_file_sizes"] = _mod +_spec.loader.exec_module(_mod) + +Caps = _mod.Caps +Baseline = _mod.Baseline +Config = _mod.Config +FileStats = _mod.FileStats +evaluate = _mod.evaluate +is_test_file = _mod.is_test_file +iter_source_files = _mod.iter_source_files +measure = _mod.measure + + +@pytest.fixture +def caps() -> Caps: + return Caps(hard_lines=1500, hard_bytes=100_000, soft_lines=800, soft_bytes=60_000) + + +@pytest.fixture +def empty_config(caps: Caps) -> Config: + return Config(caps=caps, baselines={}) + + +def _stats(lines: int, bts: int, name: str = "x.py") -> FileStats: + return FileStats(path=Path(name), lines=lines, bytes=bts) + + +class TestEvaluate: + def test_under_all_caps_passes(self, empty_config: Config) -> None: + errors, warnings = evaluate(_stats(100, 5_000), "x.py", empty_config) + assert errors == [] + assert warnings == [] + + def test_over_hard_lines_fails(self, empty_config: Config) -> None: + errors, warnings = evaluate(_stats(1501, 50_000), "x.py", empty_config) + assert len(errors) == 1 + assert "exceeds hard cap" in errors[0] + assert warnings == [] + + def test_over_hard_bytes_fails(self, empty_config: Config) -> None: + errors, warnings = evaluate(_stats(100, 100_001), "x.py", empty_config) + assert len(errors) == 1 + assert "exceeds hard cap" in errors[0] + + def test_over_soft_lines_warns(self, empty_config: Config) -> None: + errors, warnings = evaluate(_stats(801, 30_000), "x.py", empty_config) + assert errors == [] + assert len(warnings) == 1 + assert "soft cap" in warnings[0] + + def test_over_soft_bytes_warns(self, empty_config: Config) -> None: + errors, warnings = evaluate(_stats(100, 60_001), "x.py", empty_config) + assert errors == [] + assert len(warnings) == 1 + + def test_at_caps_passes(self, empty_config: Config) -> None: + errors, warnings = evaluate(_stats(1500, 100_000), "x.py", empty_config) + # Exactly at the hard cap is allowed; soft cap is exceeded -> warns. + assert errors == [] + assert len(warnings) == 2 + + def test_allowlisted_at_baseline_passes(self, caps: Caps) -> None: + config = Config(caps=caps, baselines={"big.py": Baseline(lines=2000, bytes=80_000)}) + errors, warnings = evaluate(_stats(2000, 80_000), "big.py", config) + assert errors == [] + assert warnings == [] + + def test_allowlisted_growth_in_lines_fails(self, caps: Caps) -> None: + config = Config(caps=caps, baselines={"big.py": Baseline(lines=2000, bytes=80_000)}) + errors, _ = evaluate(_stats(2001, 80_000), "big.py", config) + assert len(errors) == 1 + assert "exceeds allowlist baseline" in errors[0] + + def test_allowlisted_growth_in_bytes_fails(self, caps: Caps) -> None: + config = Config(caps=caps, baselines={"big.py": Baseline(lines=2000, bytes=80_000)}) + errors, _ = evaluate(_stats(2000, 80_001), "big.py", config) + assert len(errors) == 1 + assert "exceeds allowlist baseline" in errors[0] + + def test_allowlisted_shrinkage_passes(self, caps: Caps) -> None: + """Cleanup that drops the file is fine even if still over hard cap.""" + config = Config(caps=caps, baselines={"big.py": Baseline(lines=2000, bytes=80_000)}) + errors, warnings = evaluate(_stats(1900, 79_000), "big.py", config) + assert errors == [] + assert warnings == [] + + def test_allowlisted_under_caps_passes_silently(self, caps: Caps) -> None: + """A file in the allowlist that's already shrunk under caps is silent. + + It should not produce a soft-cap warning for an allowlisted file. + """ + config = Config(caps=caps, baselines={"big.py": Baseline(lines=2000, bytes=80_000)}) + errors, warnings = evaluate(_stats(900, 30_000), "big.py", config) + assert errors == [] + assert warnings == [] + + +class TestIsTestFile: + def test_test_prefix(self) -> None: + assert is_test_file(Path("orchestrator/test_foo.py")) + + def test_test_suffix(self) -> None: + assert is_test_file(Path("orchestrator/foo_test.py")) + + def test_tests_subdir(self) -> None: + assert is_test_file(Path("orchestrator/tests/anything.py")) + + def test_pycache(self) -> None: + assert is_test_file(Path("orchestrator/__pycache__/foo.py")) + + def test_normal_source(self) -> None: + assert not is_test_file(Path("orchestrator/routes/pipelines.py")) + + +class TestMeasure: + def test_counts_lines_and_bytes(self, tmp_path: Path) -> None: + f = tmp_path / "x.py" + f.write_text("a\nb\nc\n") + s = measure(f) + assert s.lines == 3 + assert s.bytes == 6 + + def test_counts_no_trailing_newline(self, tmp_path: Path) -> None: + f = tmp_path / "x.py" + f.write_text("a\nb\nc") + s = measure(f) + assert s.lines == 3 + assert s.bytes == 5 + + def test_empty_file(self, tmp_path: Path) -> None: + f = tmp_path / "x.py" + f.write_text("") + s = measure(f) + assert s.lines == 0 + assert s.bytes == 0 + + +class TestIterSourceFiles: + def test_skips_tests_dir(self, tmp_path: Path) -> None: + (tmp_path / "orchestrator").mkdir() + (tmp_path / "orchestrator" / "tests").mkdir() + (tmp_path / "orchestrator" / "good.py").write_text("x = 1\n") + (tmp_path / "orchestrator" / "tests" / "test_a.py").write_text("x = 1\n") + (tmp_path / "orchestrator" / "test_inline.py").write_text("x = 1\n") + result = iter_source_files(tmp_path) + rels = sorted(p.relative_to(tmp_path).as_posix() for p in result) + assert rels == ["orchestrator/good.py"] + + def test_only_source_roots(self, tmp_path: Path) -> None: + (tmp_path / "orchestrator").mkdir() + (tmp_path / "orchestrator" / "x.py").write_text("x = 1\n") + (tmp_path / "docs").mkdir() + (tmp_path / "docs" / "ignored.py").write_text("x = 1\n") + result = iter_source_files(tmp_path) + rels = [p.relative_to(tmp_path).as_posix() for p in result] + assert rels == ["orchestrator/x.py"] + + def test_missing_root_is_fine(self, tmp_path: Path) -> None: + # No source roots present at all -- should return empty list. + assert iter_source_files(tmp_path) == [] From 78c201196888cc9b52976afb079b05df15b0f8e1 Mon Sep 17 00:00:00 2001 From: "egg-reviewer[bot]" <261018737+egg-reviewer[bot]@users.noreply.github.com> Date: Wed, 29 Apr 2026 17:27:57 +0000 Subject: [PATCH 2/5] Preserve issue: field on --update-allowlist Review feedback on #2250: Baseline only captured lines+bytes, so any --update-allowlist invocation silently dropped every entry's tracking issue link in one shot. Add an optional issue field to Baseline, read it on load, and emit it on write only when present so new entries don't get spurious null-issue keys. update_allowlist carries the issue forward from the existing entry when refreshing baselines. Adds round-trip tests covering load_config + write_allowlist that would have caught the data-loss bug. --- scripts/check-file-sizes.py | 32 +++++-- tests/scripts/test_check_file_sizes.py | 115 +++++++++++++++++++++++++ 2 files changed, 141 insertions(+), 6 deletions(-) diff --git a/scripts/check-file-sizes.py b/scripts/check-file-sizes.py index 47ee793e98..5abfa88086 100755 --- a/scripts/check-file-sizes.py +++ b/scripts/check-file-sizes.py @@ -66,6 +66,7 @@ class Caps: class Baseline: lines: int bytes: int + issue: str | None = None @dataclass(frozen=True) @@ -92,7 +93,11 @@ def load_config(path: Path = ALLOWLIST_PATH) -> Config: ) files_raw: dict[str, Any] = raw.get("files") or {} baselines = { - rel: Baseline(lines=int(entry["lines"]), bytes=int(entry["bytes"])) + rel: Baseline( + lines=int(entry["lines"]), + bytes=int(entry["bytes"]), + issue=(str(entry["issue"]) if entry.get("issue") is not None else None), + ) for rel, entry in files_raw.items() } return Config(caps=caps, baselines=baselines) @@ -205,6 +210,13 @@ def check_all(repo_root: Path = REPO_ROOT) -> tuple[list[str], list[str], list[s def write_allowlist(config: Config, baselines: dict[str, Baseline]) -> None: """Rewrite the allowlist file preserving caps + sorted baselines.""" + + def _entry(b: Baseline) -> dict[str, Any]: + out: dict[str, Any] = {"lines": b.lines, "bytes": b.bytes} + if b.issue is not None: + out["issue"] = b.issue + return out + payload: dict[str, Any] = { "caps": { "hard_lines": config.caps.hard_lines, @@ -212,22 +224,30 @@ def write_allowlist(config: Config, baselines: dict[str, Baseline]) -> None: "soft_lines": config.caps.soft_lines, "soft_bytes": config.caps.soft_bytes, }, - "files": { - rel: {"lines": b.lines, "bytes": b.bytes} for rel, b in sorted(baselines.items()) - }, + "files": {rel: _entry(b) for rel, b in sorted(baselines.items())}, } ALLOWLIST_PATH.write_text(yaml.safe_dump(payload, sort_keys=False)) def update_allowlist(repo_root: Path = REPO_ROOT) -> int: - """Refresh allowlist baselines from current file sizes.""" + """Refresh allowlist baselines from current file sizes. + + The ``issue:`` tracking field on each existing entry is carried forward; + losing it would drop the link between the file and its decomposition + follow-up. + """ config = load_config() new_baselines: dict[str, Baseline] = {} for path in iter_source_files(repo_root): rel = str(path.relative_to(repo_root)) stats = measure(path) if stats.lines > config.caps.hard_lines or stats.bytes > config.caps.hard_bytes: - new_baselines[rel] = Baseline(lines=stats.lines, bytes=stats.bytes) + existing = config.baselines.get(rel) + new_baselines[rel] = Baseline( + lines=stats.lines, + bytes=stats.bytes, + issue=existing.issue if existing is not None else None, + ) write_allowlist(config, new_baselines) print(f"Wrote {len(new_baselines)} entries to {ALLOWLIST_PATH.name}") return 0 diff --git a/tests/scripts/test_check_file_sizes.py b/tests/scripts/test_check_file_sizes.py index 389e62bdb5..d715ef12d4 100644 --- a/tests/scripts/test_check_file_sizes.py +++ b/tests/scripts/test_check_file_sizes.py @@ -4,9 +4,11 @@ import importlib.util import sys +import textwrap from pathlib import Path import pytest +import yaml _SCRIPT_PATH = Path(__file__).resolve().parents[2] / "scripts" / "check-file-sizes.py" _spec = importlib.util.spec_from_file_location("check_file_sizes", _SCRIPT_PATH) @@ -24,7 +26,9 @@ evaluate = _mod.evaluate is_test_file = _mod.is_test_file iter_source_files = _mod.iter_source_files +load_config = _mod.load_config measure = _mod.measure +write_allowlist = _mod.write_allowlist @pytest.fixture @@ -174,3 +178,114 @@ def test_only_source_roots(self, tmp_path: Path) -> None: def test_missing_root_is_fine(self, tmp_path: Path) -> None: # No source roots present at all -- should return empty list. assert iter_source_files(tmp_path) == [] + + +class TestYamlRoundTrip: + """Cover load_config + write_allowlist to guard against losing the + ``issue:`` tracking field on --update-allowlist.""" + + def _yaml(self, tmp_path: Path, body: str) -> Path: + p = tmp_path / "allowlist.yaml" + p.write_text(textwrap.dedent(body)) + return p + + def test_load_preserves_issue_field(self, tmp_path: Path) -> None: + path = self._yaml( + tmp_path, + """ + caps: + hard_lines: 1500 + hard_bytes: 100000 + soft_lines: 800 + soft_bytes: 60000 + files: + foo/bar.py: + lines: 2000 + bytes: 80000 + issue: "2248" + """, + ) + config = load_config(path) + assert config.baselines["foo/bar.py"].issue == "2248" + + def test_load_handles_missing_issue_field(self, tmp_path: Path) -> None: + path = self._yaml( + tmp_path, + """ + caps: + hard_lines: 1500 + hard_bytes: 100000 + soft_lines: 800 + soft_bytes: 60000 + files: + foo/bar.py: + lines: 2000 + bytes: 80000 + """, + ) + config = load_config(path) + assert config.baselines["foo/bar.py"].issue is None + + def test_load_handles_null_issue_field(self, tmp_path: Path) -> None: + path = self._yaml( + tmp_path, + """ + caps: + hard_lines: 1500 + hard_bytes: 100000 + soft_lines: 800 + soft_bytes: 60000 + files: + foo/bar.py: + lines: 2000 + bytes: 80000 + issue: null + """, + ) + config = load_config(path) + assert config.baselines["foo/bar.py"].issue is None + + def test_write_emits_issue_when_present( + self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path, caps: Caps + ) -> None: + out = tmp_path / "out.yaml" + monkeypatch.setattr(_mod, "ALLOWLIST_PATH", out) + config = Config(caps=caps, baselines={}) + baselines = {"foo/bar.py": Baseline(lines=2000, bytes=80_000, issue="2248")} + write_allowlist(config, baselines) + loaded = yaml.safe_load(out.read_text()) + assert loaded["files"]["foo/bar.py"] == { + "lines": 2000, + "bytes": 80_000, + "issue": "2248", + } + + def test_write_omits_issue_when_absent( + self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path, caps: Caps + ) -> None: + out = tmp_path / "out.yaml" + monkeypatch.setattr(_mod, "ALLOWLIST_PATH", out) + config = Config(caps=caps, baselines={}) + baselines = {"foo/bar.py": Baseline(lines=2000, bytes=80_000)} + write_allowlist(config, baselines) + loaded = yaml.safe_load(out.read_text()) + # No `issue:` key emitted when the baseline has none -- avoids + # littering new entries with `issue: null`. + assert loaded["files"]["foo/bar.py"] == {"lines": 2000, "bytes": 80_000} + assert "issue" not in loaded["files"]["foo/bar.py"] + + def test_round_trip_preserves_issue( + self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path, caps: Caps + ) -> None: + """Regression: --update-allowlist must not silently drop issue links.""" + out = tmp_path / "out.yaml" + monkeypatch.setattr(_mod, "ALLOWLIST_PATH", out) + config = Config(caps=caps, baselines={}) + original = { + "a.py": Baseline(lines=2000, bytes=80_000, issue="2248"), + "b.py": Baseline(lines=2500, bytes=90_000, issue="9999"), + "c.py": Baseline(lines=1800, bytes=70_000), # no issue + } + write_allowlist(config, original) + reloaded = load_config(out) + assert reloaded.baselines == original From 8196e288689cc21fb326f1fdaeae33ba50e84807 Mon Sep 17 00:00:00 2001 From: "egg-reviewer[bot]" <261018737+egg-reviewer[bot]@users.noreply.github.com> Date: Wed, 29 Apr 2026 17:44:10 +0000 Subject: [PATCH 3/5] Add update_allowlist carry-forward regression test Reviewer flagged that the one-line existing.issue if existing is not None else None carry-forward inside update_allowlist had no direct test coverage -- a refactor that dropped the lookup would only be caught by manual verification. Add a test that builds a tmp repo, pre-seeds the allowlist with issue="2248", runs update_allowlist, and asserts the reloaded entry retains the issue while line/byte counts are refreshed from the live file. Make load_config resolve ALLOWLIST_PATH at call time (instead of binding it at function-definition time) so monkeypatch.setattr(_mod, "ALLOWLIST_PATH", ...) actually flows through update_allowlist's internal load_config() call. Existing callers pass an explicit path or relied on the module global; both still work. --- scripts/check-file-sizes.py | 5 ++- tests/scripts/test_check_file_sizes.py | 44 ++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/scripts/check-file-sizes.py b/scripts/check-file-sizes.py index 5abfa88086..fe2ab3609f 100755 --- a/scripts/check-file-sizes.py +++ b/scripts/check-file-sizes.py @@ -82,7 +82,10 @@ class Config: baselines: dict[str, Baseline] -def load_config(path: Path = ALLOWLIST_PATH) -> Config: +def load_config(path: Path | None = None) -> Config: + # Resolve the default at call time so tests can monkey-patch ALLOWLIST_PATH. + if path is None: + path = ALLOWLIST_PATH raw = yaml.safe_load(path.read_text()) or {} caps_raw = raw.get("caps") or {} caps = Caps( diff --git a/tests/scripts/test_check_file_sizes.py b/tests/scripts/test_check_file_sizes.py index d715ef12d4..bb86845ee3 100644 --- a/tests/scripts/test_check_file_sizes.py +++ b/tests/scripts/test_check_file_sizes.py @@ -28,6 +28,7 @@ iter_source_files = _mod.iter_source_files load_config = _mod.load_config measure = _mod.measure +update_allowlist = _mod.update_allowlist write_allowlist = _mod.write_allowlist @@ -289,3 +290,46 @@ def test_round_trip_preserves_issue( write_allowlist(config, original) reloaded = load_config(out) assert reloaded.baselines == original + + def test_update_allowlist_carries_issue_forward( + self, + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], + ) -> None: + """update_allowlist must preserve the existing entry's issue link.""" + repo_root = tmp_path / "repo" + (repo_root / "orchestrator").mkdir(parents=True) + # Write a file that will trip the (small) hard-line cap below. + big = repo_root / "orchestrator" / "big.py" + big.write_text("x = 1\n" * 50) + + allowlist = tmp_path / "allowlist.yaml" + allowlist.write_text( + textwrap.dedent( + """ + caps: + hard_lines: 10 + hard_bytes: 1000000 + soft_lines: 5 + soft_bytes: 500000 + files: + orchestrator/big.py: + lines: 50 + bytes: 300 + issue: "2248" + """ + ) + ) + monkeypatch.setattr(_mod, "ALLOWLIST_PATH", allowlist) + + rc = update_allowlist(repo_root) + assert rc == 0 + capsys.readouterr() # discard the "Wrote N entries" print + + reloaded = load_config(allowlist) + entry = reloaded.baselines["orchestrator/big.py"] + assert entry.issue == "2248" + # And the line/byte counts were refreshed from the live file. + assert entry.lines == 50 + assert entry.bytes == len(big.read_bytes()) From e802cee99a137740c78c9f64db51fdabc4a5127c Mon Sep 17 00:00:00 2001 From: "egg-reviewer[bot]" <261018737+egg-reviewer[bot]@users.noreply.github.com> Date: Wed, 29 Apr 2026 18:04:16 +0000 Subject: [PATCH 4/5] Strengthen update_allowlist test to detect stale-baseline regressions Seed the carry-forward test with deliberately stale lines/bytes (1/1 instead of 50/300) so the post-update assertions only pass when update_allowlist actually re-measures from the live file, rather than silently preserving the seed values. --- tests/scripts/test_check_file_sizes.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/scripts/test_check_file_sizes.py b/tests/scripts/test_check_file_sizes.py index bb86845ee3..1dbd8f0d76 100644 --- a/tests/scripts/test_check_file_sizes.py +++ b/tests/scripts/test_check_file_sizes.py @@ -304,6 +304,9 @@ def test_update_allowlist_carries_issue_forward( big = repo_root / "orchestrator" / "big.py" big.write_text("x = 1\n" * 50) + # Seed deliberately stale line/byte values so the post-update + # assertion only passes when update_allowlist genuinely re-measures + # from the live file (rather than silently preserving the seed). allowlist = tmp_path / "allowlist.yaml" allowlist.write_text( textwrap.dedent( @@ -315,8 +318,8 @@ def test_update_allowlist_carries_issue_forward( soft_bytes: 500000 files: orchestrator/big.py: - lines: 50 - bytes: 300 + lines: 1 + bytes: 1 issue: "2248" """ ) @@ -330,6 +333,7 @@ def test_update_allowlist_carries_issue_forward( reloaded = load_config(allowlist) entry = reloaded.baselines["orchestrator/big.py"] assert entry.issue == "2248" - # And the line/byte counts were refreshed from the live file. + # And the line/byte counts were refreshed from the live file — + # not preserved from the stale seed values above. assert entry.lines == 50 assert entry.bytes == len(big.read_bytes()) From a8f9c2db041e12e474c8b918258d1a6e3574c6a6 Mon Sep 17 00:00:00 2001 From: "james-in-a-box[bot]" <246424927+james-in-a-box[bot]@users.noreply.github.com> Date: Wed, 29 Apr 2026 18:12:06 +0000 Subject: [PATCH 5/5] Fix file-size allowlist: update gateway.py baseline to match current main MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit gateway/gateway.py grew by 1 line (9753→9754) and 106 bytes (373015→373121) after commit 2fa7dc6 expanded the branch-switch error message. Update the allowlist baseline so the CI merge check passes. Author: egg --- scripts/file-size-allowlist.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/file-size-allowlist.yaml b/scripts/file-size-allowlist.yaml index a6c5d4eb9c..0c8158c911 100644 --- a/scripts/file-size-allowlist.yaml +++ b/scripts/file-size-allowlist.yaml @@ -24,8 +24,8 @@ files: bytes: 669950 issue: "2248" gateway/gateway.py: - lines: 9753 - bytes: 373015 + lines: 9754 + bytes: 373121 issue: "2248" sandbox/egg_lib/orch_cli.py: lines: 3512