From a978c8d614bf2d81d8395b075162fa8569428ce1 Mon Sep 17 00:00:00 2001 From: Ramesh Padmanabhaiah <22363102+codeforester@users.noreply.github.com> Date: Wed, 30 Sep 2026 19:48:53 +0530 Subject: [PATCH 1/3] fix: key lifecycle state by context --- lib/python/base_cli/_lifecycle_install.py | 11 ++++++---- tests/test_click_tree_attachment.py | 25 +++++++++++++++++++++++ 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/lib/python/base_cli/_lifecycle_install.py b/lib/python/base_cli/_lifecycle_install.py index 6e4b295..1f1bada 100644 --- a/lib/python/base_cli/_lifecycle_install.py +++ b/lib/python/base_cli/_lifecycle_install.py @@ -93,7 +93,10 @@ def _capture_lifecycle_option( ) -> Any: source = click_context.get_parameter_source(parameter.name) captures = click_context.meta.setdefault(_LIFECYCLE_CAPTURE_META_KEY, {}) - context_values = captures.setdefault(id(click_context), {}) + # Keep the context object itself as the key. ``id(context)`` values can be + # reused after Click releases a context, which could associate a later + # invocation with stale lifecycle values. + context_values = captures.setdefault(click_context, {}) context_values[key] = _RawLifecycleValue( value=value, source=source, @@ -538,10 +541,10 @@ def _resolve_lifecycle_values( {}, ) parent = getattr(click_context, "parent", None) - parent_resolution = resolution_map.get(id(parent)) if parent is not None else None + parent_resolution = resolution_map.get(parent) if parent is not None else None raw = dict(parent_resolution.raw) if isinstance(parent_resolution, _LifecycleResolution) else {} captures = click_context.meta.get(_LIFECYCLE_CAPTURE_META_KEY, {}) - context_captures = captures.get(id(click_context), {}) + context_captures = captures.get(click_context, {}) depth = _context_depth(click_context) for key, binding in bindings.items(): @@ -576,7 +579,7 @@ def _resolve_lifecycle_values( values=_normalize_lifecycle_values(click, raw), raw=raw, ) - resolution_map[id(click_context)] = resolution + resolution_map[click_context] = resolution click_context.meta[LIFECYCLE_META_KEY] = resolution.values return resolution diff --git a/tests/test_click_tree_attachment.py b/tests/test_click_tree_attachment.py index 2392fde..9b285cf 100644 --- a/tests/test_click_tree_attachment.py +++ b/tests/test_click_tree_attachment.py @@ -12,6 +12,7 @@ from unittest import mock import base_cli +from base_cli import _lifecycle_install from base_cli._runtime import RuntimeDirectoryError from base_cli.testing import invoke @@ -63,6 +64,30 @@ def count_cleanup() -> None: @unittest.skipUnless(importlib.util.find_spec("click"), "Click is not installed") class ClickTreeAttachmentTests(unittest.TestCase): + def test_lifecycle_metadata_is_keyed_by_context_identity(self) -> None: + class FakeContext: + def __init__(self) -> None: + self.meta: dict[object, Any] = {} + + def get_parameter_source(self, name: str) -> None: + del name + return None + + first = FakeContext() + second = FakeContext() + parameter = type("Parameter", (), {"name": "environment"})() + + _lifecycle_install._capture_lifecycle_option(first, parameter, "first", key="environment") + _lifecycle_install._capture_lifecycle_option(second, parameter, "second", key="environment") + + captures = first.meta[next(key for key in first.meta if key is not None)] + self.assertIn(first, captures) + self.assertNotIn(id(first), captures) + self.assertEqual(captures[first]["environment"].value, "first") + second_captures = second.meta[next(key for key in second.meta if key is not None)] + self.assertIn(second, second_captures) + self.assertEqual(second_captures[second]["environment"].value, "second") + def test_prebuilt_single_command_preserves_click_contract_and_lifecycle(self) -> None: import click From 42888166da5dc6387e49c8baa40f1326ed22344d Mon Sep 17 00:00:00 2001 From: Ramesh Padmanabhaiah <22363102+codeforester@users.noreply.github.com> Date: Thu, 1 Oct 2026 00:19:49 +0530 Subject: [PATCH 2/3] test: cover chained lifecycle context teardown --- lib/python/base_cli/_lifecycle_install.py | 4 ++ tests/test_click_tree_attachment.py | 75 +++++++++++++++++++++++ 2 files changed, 79 insertions(+) diff --git a/lib/python/base_cli/_lifecycle_install.py b/lib/python/base_cli/_lifecycle_install.py index 1f1bada..d4781fe 100644 --- a/lib/python/base_cli/_lifecycle_install.py +++ b/lib/python/base_cli/_lifecycle_install.py @@ -540,6 +540,10 @@ def _resolve_lifecycle_values( _LIFECYCLE_RESOLUTION_META_KEY, {}, ) + # These maps live in Click's invocation-shared metadata and retain one + # context-keyed entry per context until the root invocation closes. That + # bounded, per-invocation retention is deliberate: it prevents id reuse + # without retaining state across invocations. parent = getattr(click_context, "parent", None) parent_resolution = resolution_map.get(parent) if parent is not None else None raw = dict(parent_resolution.raw) if isinstance(parent_resolution, _LifecycleResolution) else {} diff --git a/tests/test_click_tree_attachment.py b/tests/test_click_tree_attachment.py index 9b285cf..432d4b2 100644 --- a/tests/test_click_tree_attachment.py +++ b/tests/test_click_tree_attachment.py @@ -88,6 +88,81 @@ def get_parameter_source(self, name: str) -> None: self.assertIn(second, second_captures) self.assertEqual(second_captures[second]["environment"].value, "second") + def test_chained_click_contexts_keep_lifecycle_values_through_teardown(self) -> None: + import click + + observed: list[tuple[str, str | None, int]] = [] + closed: list[str] = [] + + def capture_environment(click_context: Any, parameter: Any, value: Any) -> Any: + return _lifecycle_install._capture_lifecycle_option( + click_context, + parameter, + value, + key="environment", + ) + + @click.group(name="pipeline", chain=True) + def pipeline() -> None: + pass + + @pipeline.command(name="first") + @click.option("--environment", callback=capture_environment) + def first(environment: str | None) -> None: + del environment + click_context = click.get_current_context() + captures = click_context.meta[_lifecycle_install._LIFECYCLE_CAPTURE_META_KEY] + observed.append( + ( + "first", + captures[click_context]["environment"].value, + len(captures), + ) + ) + click_context.call_on_close(lambda: closed.append("first")) + + @pipeline.command(name="second") + @click.option("--environment", callback=capture_environment) + def second(environment: str | None) -> None: + del environment + click_context = click.get_current_context() + captures = click_context.meta[_lifecycle_install._LIFECYCLE_CAPTURE_META_KEY] + observed.append( + ( + "second", + captures[click_context]["environment"].value, + len(captures), + ) + ) + self.assertEqual(closed, ["first"]) + click_context.call_on_close(lambda: closed.append("second")) + + app = base_cli.App(name="pipeline", log_to_file=False) + app.attach(pipeline) + + with tempfile.TemporaryDirectory() as tmpdir: + result = invoke( + app, + [ + "first", + "--environment", + "first-env", + "second", + "--environment", + "second-env", + ], + home=Path(tmpdir), + ) + + self.assertEqual(result.exit_code, 0, result.output) + self.assertEqual( + [(name, value) for name, value, _capture_count in observed], + [("first", "first-env"), ("second", "second-env")], + ) + self.assertGreaterEqual(observed[0][2], 2) + self.assertEqual(observed[0][2], observed[1][2]) + self.assertEqual(closed, ["first", "second"]) + def test_prebuilt_single_command_preserves_click_contract_and_lifecycle(self) -> None: import click From 8bd1f9786cbe673d5d2fbcf95d99089462044dfd Mon Sep 17 00:00:00 2001 From: Ramesh Padmanabhaiah <22363102+codeforester@users.noreply.github.com> Date: Sat, 3 Oct 2026 00:54:28 +0530 Subject: [PATCH 3/3] ci: separate sustained persistence cost from hosted filesystem tails --- docs/performance.md | 12 +++++++++++- scripts/benchmark_runtime.py | 9 +++++++-- tests/test_benchmark_runtime.py | 12 ++++++++++++ 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/docs/performance.md b/docs/performance.md index 8195656..96e0797 100644 --- a/docs/performance.md +++ b/docs/performance.md @@ -63,7 +63,7 @@ scheduler outlier block a change. | Cold no-op invocation, including startup and dispatch | 2,000 ms | 2,000 ms | 4,000 ms | 4,000 ms | | Base-cli lifecycle increment over Click warm dispatch | 5 ms | 5 ms | 15 ms | 15 ms | | Warm invocation and non-persistence feature scenarios | 50 ms | 50 ms | 100 ms | 100 ms | -| File-persistence-enabled scenario | 50 ms | 50 ms | 250 ms | 50 ms | +| File-persistence-enabled scenario | 125 ms | 125 ms | 250 ms | 50 ms | An initial 31-sample local calibration on macOS (Python 3.14.6, Apple Silicon) measured approximately 101 ms for base-cli cold import, 0.56 ms for warm @@ -76,6 +76,16 @@ budget instead of weakening other warm-scenario gates. These measurements are CI calibration evidence, not adoption claims or release comparisons; review subsequent retained artifacts before tightening platform budgets. +October 2026 hosted recalibration separates sustained persistence cost from +filesystem tails on Unix/macOS: median must remain at most **50 ms** and p95 +at most **125 ms**. The previous 50 ms p95 cap repeatedly rejected otherwise +unchanged runtime code, including the validation-only PR. Observed pairs were +14.66/118.04 ms (Unix median/p95) and 24.93/61.93 and 26.12/87.37 ms (macOS). +Evidence: [Unix run](https://github.com/basefoundry/base-cli/actions/runs/37048785893) +and [macOS validation-only run](https://github.com/basefoundry/base-cli/actions/runs/37052368353). +A sustained slowdown over 50 ms still fails; p95 over 125 ms also fails. +Windows, WSL, parser, import, and non-persistence limits are unchanged. + Each report is versioned as `base-cli.benchmark` schema version 1 and contains the package version, source revision, UTC timestamp, platform profile, Python version/ABI, OS release, architecture, CPU count, sample count, medians, p95, diff --git a/scripts/benchmark_runtime.py b/scripts/benchmark_runtime.py index 24f3e78..f5ade65 100755 --- a/scripts/benchmark_runtime.py +++ b/scripts/benchmark_runtime.py @@ -48,8 +48,8 @@ "wsl": 100.0, } PERSISTENCE_ENABLED_P95_BUDGETS_MS = { - "unix": 50.0, - "macos": 50.0, + "unix": 125.0, + "macos": 125.0, "windows": 250.0, "wsl": 50.0, } @@ -331,6 +331,11 @@ def _check_results(results: dict[str, FrameworkMetrics]) -> list[str]: feature_budget = _feature_budget_for_platform(name, BENCHMARK_PLATFORM) if p95 is not None and p95 > feature_budget: failures.append(f"base-cli {name} p95 exceeded {feature_budget:.0f} ms") + if BENCHMARK_PLATFORM in {"unix", "macos"} and isinstance(features, dict): + persistence = features.get("persistence_enabled_ms", {}) + median = persistence.get("median") if isinstance(persistence, dict) else None + if not isinstance(median, (int, float)) or not 0 <= median <= 50.0: + failures.append("base-cli persistence_enabled_ms median is missing, invalid, or exceeded 50 ms") return failures diff --git a/tests/test_benchmark_runtime.py b/tests/test_benchmark_runtime.py index 8528e5e..7518e06 100644 --- a/tests/test_benchmark_runtime.py +++ b/tests/test_benchmark_runtime.py @@ -125,6 +125,18 @@ def test_windows_persistence_budget_rejects_material_regressions(self) -> None: self.assertTrue(any("persistence_enabled_ms p95 exceeded 250 ms" in failure for failure in failures)) + def test_persistence_budget_separates_sustained_cost_from_filesystem_tails(self) -> None: + for profile in ("unix", "macos"): + for median, p95, fails in ((26.0, 118.0, False), (51.0, 60.0, True), (26.0, 126.0, True)): + with self.subTest(profile=profile, median=median, p95=p95): + metrics = self._complete_results() + sample = self._summary(p95) + sample["median"] = median + metrics["base-cli"]["features"]["persistence_enabled_ms"] = sample + with mock.patch.object(benchmark_runtime, "BENCHMARK_PLATFORM", profile): + failures = benchmark_runtime._check_results(metrics) + self.assertEqual(any("persistence_enabled_ms" in failure for failure in failures), fails) + def test_github_summary_separates_lifecycle_overhead_from_parser(self) -> None: metrics = self._complete_results(lifecycle_p95=4.0, click_p95=1.5) report = {