From 510a84d86757636d956578a2353179c464f869e4 Mon Sep 17 00:00:00 2001 From: Ramesh Padmanabhaiah <22363102+codeforester@users.noreply.github.com> Date: Fri, 31 Jul 2026 07:45:36 -0700 Subject: [PATCH] refactor: default App to generic profile --- CHANGELOG.md | 5 +++++ README.md | 12 +++++++----- docs/consumer-profiles.md | 19 +++++++++---------- lib/python/base_cli/app.py | 5 ++++- tests/test_app_log_retention.py | 20 ++++++++++++-------- tests/test_app_run.py | 8 ++++++-- tests/test_app_runtime_errors.py | 6 +++++- tests/test_base_cli.py | 16 ++++++++++------ tests/test_context_workspace.py | 6 +++++- tests/test_history.py | 16 ++++++++++------ tests/test_profile.py | 7 +++++++ tests/test_testing.py | 10 +++++++--- tests/test_user_config_github.py | 6 +++++- 13 files changed, 92 insertions(+), 44 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9766ba8..effe82e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ and versions are tracked in the repo-root `VERSION` file. ## [Unreleased] +### Changed + +- Make `base_cli.App()` use the consumer-neutral profile by default; the + temporary Base compatibility profile is now explicit. + ### Added - Initialized the repository with the Base-managed repo baseline. diff --git a/README.md b/README.md index 20217e4..e09ce84 100644 --- a/README.md +++ b/README.md @@ -58,7 +58,8 @@ The package follows these rules: between applications: project discovery, configuration, runtime placement, and optional history persistence. -Standalone consumers should opt into the generic profile explicitly: +Standalone consumers use the generic profile by default. It can also be passed +explicitly when making the policy boundary visible: ```python app = base_cli.App( @@ -71,10 +72,11 @@ app = base_cli.App( The generic profile has no manifest filename convention, no product-owned configuration directory, and no implicit history writer. Applications can provide those policies through callbacks or build their own profile. The -temporary default, `CliProfile.legacy_base()`, preserves the historical -Base behavior for existing callers while Base migrates to an explicit adapter. -See [`docs/consumer-profiles.md`](docs/consumer-profiles.md) for the boundary -and migration plan. +temporary compatibility profile, `CliProfile.legacy_base()`, preserves the +historical Base behavior only for callers that opt into it explicitly while +Base completes its adapter extraction. See +[`docs/consumer-profiles.md`](docs/consumer-profiles.md) for the boundary and +migration plan. ## Public API diff --git a/docs/consumer-profiles.md b/docs/consumer-profiles.md index af8e12d..9635ae0 100644 --- a/docs/consumer-profiles.md +++ b/docs/consumer-profiles.md @@ -78,11 +78,11 @@ command aliases. ## Compatibility profile -For the migration period, `App()` without an explicit profile selects -`CliProfile.legacy_base()`. This preserves existing Base consumers while they -move their integration code out of the generic package. New standalone -consumers should pass `CliProfile.generic()` explicitly so their behavior does -not depend on the compatibility default. +`App()` uses `CliProfile.generic()` when no profile is supplied. This keeps the +standalone default consumer-neutral. During the migration, Base and other +existing integrations that still need these conventions can opt into +`CliProfile.legacy_base()` explicitly while they move their adapters out of the +generic package. The legacy profile contains the current Base conventions, including: @@ -108,13 +108,12 @@ The following behaviors should not be added to generic lifecycle modules: - product-specific command lists or history schema; - assumptions about a downstream repository's directory layout. -The next migration phases are: +The remaining migration phases are: -1. Change Base command engines to construct and pass an explicit legacy profile. -2. Move Base discovery, config, runtime, and history adapters into Base. -3. Generalize the remaining context/config types where their names still encode +1. Move Base discovery, config, runtime, and history adapters into Base. +2. Generalize the remaining context/config types where their names still encode Base concepts. -4. Remove the compatibility default and keep `base_cli` focused on the generic +3. Remove the compatibility profile and keep `base_cli` focused on the generic lifecycle. The package rename is deliberately separate from this refactor. Names can be diff --git a/lib/python/base_cli/app.py b/lib/python/base_cli/app.py index c0653b1..d912f21 100644 --- a/lib/python/base_cli/app.py +++ b/lib/python/base_cli/app.py @@ -59,7 +59,10 @@ def __init__( self.help = help self.log_to_file = log_to_file self.max_log_files = max_log_files - self.profile = profile or CliProfile.legacy_base() + # Standalone applications must not inherit a consumer's product + # conventions. Consumers with an existing integration should pass an + # explicit profile, such as Base's temporary legacy adapter. + self.profile = profile or CliProfile.generic() self._click_command = None self._command_func: Callable[..., Any] | None = None self._command_args: tuple[Any, ...] = () diff --git a/tests/test_app_log_retention.py b/tests/test_app_log_retention.py index aee4ac4..1bf9260 100644 --- a/tests/test_app_log_retention.py +++ b/tests/test_app_log_retention.py @@ -14,6 +14,10 @@ from base_cli.testing import invoke +def legacy_app(**kwargs: object) -> base_cli.App: + return base_cli.App(profile=base_cli.CliProfile.legacy_base(), **kwargs) + + def write_log_file(path: Path, mtime: int) -> None: path.parent.mkdir(parents=True, exist_ok=True) path.write_text("old log\n", encoding="utf-8") @@ -56,7 +60,7 @@ def test_ignores_stale_paths_in_retention_index(self) -> None: @unittest.skipUnless(importlib.util.find_spec("click"), "Click is not installed") def test_uses_retention_index_after_initial_discovery(self) -> None: - app = base_cli.App(name="retention-index", max_log_files=2) + app = legacy_app(name="retention-index", max_log_files=2) @app.command() def main(ctx: base_cli.Context) -> None: @@ -77,7 +81,7 @@ def main(ctx: base_cli.Context) -> None: @unittest.skipUnless(importlib.util.find_spec("click"), "Click is not installed") def test_prunes_oldest_default_logs(self) -> None: - app = base_cli.App(name="retention-demo", max_log_files=2) + app = legacy_app(name="retention-demo", max_log_files=2) seen = {} @app.command() @@ -104,7 +108,7 @@ def main(ctx: base_cli.Context) -> None: @unittest.skipUnless(importlib.util.find_spec("click"), "Click is not installed") def test_prunes_by_filename_when_mtimes_disagree(self) -> None: - app = base_cli.App(name="retention-filename", max_log_files=2) + app = legacy_app(name="retention-filename", max_log_files=2) seen = {} @app.command() @@ -131,7 +135,7 @@ def main(ctx: base_cli.Context) -> None: @unittest.skipUnless(importlib.util.find_spec("click"), "Click is not installed") def test_preserves_current_log_file(self) -> None: - app = base_cli.App(name="retention-current", max_log_files=1) + app = legacy_app(name="retention-current", max_log_files=1) seen = {} @app.command() @@ -158,7 +162,7 @@ def main(ctx: base_cli.Context) -> None: @unittest.skipUnless(importlib.util.find_spec("click"), "Click is not installed") def test_keeps_logs_when_count_is_within_limit(self) -> None: - app = base_cli.App(name="retention-within-limit", max_log_files=3) + app = legacy_app(name="retention-within-limit", max_log_files=3) seen = {} @app.command() @@ -185,7 +189,7 @@ def main(ctx: base_cli.Context) -> None: @unittest.skipUnless(importlib.util.find_spec("click"), "Click is not installed") def test_is_disabled_by_default(self) -> None: - app = base_cli.App(name="retention-unset") + app = legacy_app(name="retention-unset") seen = {} @app.command() @@ -212,7 +216,7 @@ def main(ctx: base_cli.Context) -> None: @unittest.skipUnless(importlib.util.find_spec("click"), "Click is not installed") def test_skips_no_durable_write_modes(self) -> None: - dry_run_app = base_cli.App(name="retention-dry-run", max_log_files=1) + dry_run_app = legacy_app(name="retention-dry-run", max_log_files=1) dry_seen = {} @dry_run_app.command() @@ -222,7 +226,7 @@ def dry_run_main(ctx: base_cli.Context, dry_run: bool) -> None: dry_seen["log_file"] = ctx.log_file ctx.log.info("dry retention") - no_file_app = base_cli.App( + no_file_app = legacy_app( name="retention-no-file", log_to_file=False, max_log_files=1, diff --git a/tests/test_app_run.py b/tests/test_app_run.py index b03522e..1b68167 100644 --- a/tests/test_app_run.py +++ b/tests/test_app_run.py @@ -13,10 +13,14 @@ from base_cli.config import user_config_path +def legacy_app(**kwargs: object) -> base_cli.App: + return base_cli.App(profile=base_cli.CliProfile.legacy_base(), **kwargs) + + class RunAppTests(unittest.TestCase): @unittest.skipUnless(importlib.util.find_spec("click"), "Click is not installed") def test_run_app_reports_config_errors_without_traceback(self) -> None: - app = base_cli.App(name="bad-config", log_to_file=False) + app = legacy_app(name="bad-config", log_to_file=False) seen = {} @app.command() @@ -122,7 +126,7 @@ def main(ctx: base_cli.Context, name: str) -> None: @unittest.skipUnless(importlib.util.find_spec("click"), "Click is not installed") def test_run_app_uses_delegated_display_command_for_usage_errors(self) -> None: - app = base_cli.App(name="internal-cli", log_to_file=False) + app = legacy_app(name="internal-cli", log_to_file=False) @app.command(context_settings={"help_option_names": ["-h", "--help"]}) def main(ctx: base_cli.Context) -> None: diff --git a/tests/test_app_runtime_errors.py b/tests/test_app_runtime_errors.py index da3af42..6cfcd2b 100644 --- a/tests/test_app_runtime_errors.py +++ b/tests/test_app_runtime_errors.py @@ -14,6 +14,10 @@ from base_cli.testing import invoke +def legacy_app(**kwargs: object) -> base_cli.App: + return base_cli.App(profile=base_cli.CliProfile.legacy_base(), **kwargs) + + class AppRuntimeErrorTests(unittest.TestCase): def test_missing_click_error_recommends_pip_install(self) -> None: app = base_cli.App(name="missing-click") @@ -39,7 +43,7 @@ def main(ctx: base_cli.Context) -> None: @unittest.skipUnless(importlib.util.find_spec("click"), "Click is not installed") def test_run_app_reports_unwritable_cache_root_without_traceback(self) -> None: - app = base_cli.App(name="cache-failure", version="0.1.0") + app = legacy_app(name="cache-failure", version="0.1.0") @app.command() def main(ctx: base_cli.Context) -> None: diff --git a/tests/test_base_cli.py b/tests/test_base_cli.py index 129ba27..601b3d2 100644 --- a/tests/test_base_cli.py +++ b/tests/test_base_cli.py @@ -33,6 +33,10 @@ def change_directory(path: Path): os.chdir(original) +def legacy_app(**kwargs: object) -> base_cli.App: + return base_cli.App(profile=base_cli.CliProfile.legacy_base(), **kwargs) + + class BaseCliTests(unittest.TestCase): @staticmethod def make_context(tmpdir: str) -> tuple[base_cli.Context, mock.Mock]: @@ -509,7 +513,7 @@ def main(ctx: base_cli.Context) -> None: @unittest.skipUnless(importlib.util.find_spec("click"), "Click is not installed") def test_context_exposes_populated_typed_user_config(self) -> None: - app = base_cli.App(name="typed-config-populated", log_to_file=False) + app = legacy_app(name="typed-config-populated", log_to_file=False) seen = {} @app.command() @@ -587,7 +591,7 @@ def second(ctx: base_cli.Context) -> None: @unittest.skipUnless(importlib.util.find_spec("click"), "Click is not installed") def test_app_runs_with_context_and_cleans_temp_dir(self) -> None: - app = base_cli.App(name="demo", version="0.1.0") + app = legacy_app(name="demo", version="0.1.0") seen = {} @app.command() @@ -774,7 +778,7 @@ def main(ctx: base_cli.Context) -> None: @unittest.skipUnless(importlib.util.find_spec("click"), "Click is not installed") def test_testing_invoke_uses_supplied_cwd_for_manifest_discovery(self) -> None: - app = base_cli.App(name="project-aware", log_to_file=False) + app = legacy_app(name="project-aware", log_to_file=False) seen = {} @app.command() @@ -855,7 +859,7 @@ def main(ctx: base_cli.Context) -> None: @unittest.skipUnless(importlib.util.find_spec("click"), "Click is not installed") def test_testing_invoke_defaults_base_cache_dir_under_home(self) -> None: - app = base_cli.App(name="cache-default") + app = legacy_app(name="cache-default") seen = {} @app.command() @@ -881,7 +885,7 @@ def main(ctx: base_cli.Context) -> None: @unittest.skipUnless(importlib.util.find_spec("click"), "Click is not installed") def test_testing_invoke_preserves_explicit_environment_overrides(self) -> None: - app = base_cli.App(name="cache-override") + app = legacy_app(name="cache-override") seen = {} @app.command() @@ -917,7 +921,7 @@ def main(ctx: base_cli.Context) -> None: @unittest.skipUnless(importlib.util.find_spec("click"), "Click is not installed") def test_standard_options_manifest_context_and_sensitive_redaction(self) -> None: - app = base_cli.App(name="secret-tool", version="0.1.0") + app = legacy_app(name="secret-tool", version="0.1.0") seen = {} @app.command() diff --git a/tests/test_context_workspace.py b/tests/test_context_workspace.py index 021e347..ef05c19 100644 --- a/tests/test_context_workspace.py +++ b/tests/test_context_workspace.py @@ -9,10 +9,14 @@ from base_cli.config import user_config_path +def legacy_app(**kwargs: object) -> base_cli.App: + return base_cli.App(profile=base_cli.CliProfile.legacy_base(), **kwargs) + + @unittest.skipUnless(importlib.util.find_spec("click"), "Click is not installed") class ContextWorkspaceRootTests(unittest.TestCase): def test_context_exposes_workspace_root_when_configured(self) -> None: - app = base_cli.App(name="workspace-root-configured", log_to_file=False) + app = legacy_app(name="workspace-root-configured", log_to_file=False) seen: dict[str, Path | None] = {} @app.command() diff --git a/tests/test_history.py b/tests/test_history.py index e8e2c7b..7db58f0 100644 --- a/tests/test_history.py +++ b/tests/test_history.py @@ -14,6 +14,10 @@ from base_cli import history as history_helpers +def legacy_app(**kwargs: object) -> base_cli.App: + return base_cli.App(profile=base_cli.CliProfile.legacy_base(), **kwargs) + + def read_history_records(cache_root: Path) -> list[dict]: history_path = cache_root / "base" / "history" / "runs.jsonl" return [json.loads(line) for line in history_path.read_text(encoding="utf-8").splitlines()] @@ -185,7 +189,7 @@ def test_write_primary_record_preserves_user_command_and_project_metadata(self) @unittest.skipUnless(importlib.util.find_spec("click"), "Click is not installed") def test_app_records_successful_command_history_with_redacted_metadata(self) -> None: - app = base_cli.App(name="history-demo", version="0.1.0") + app = legacy_app(name="history-demo", version="0.1.0") seen = {} @app.command() @@ -255,7 +259,7 @@ def main(ctx: base_cli.Context, endpoint: str, token: str) -> None: @unittest.skipUnless(importlib.util.find_spec("click"), "Click is not installed") def test_app_reuses_parent_run_and_does_not_record_internal_history(self) -> None: - app = base_cli.App(name="history-internal") + app = legacy_app(name="history-internal") seen = {} @@ -291,7 +295,7 @@ def main(ctx: base_cli.Context) -> None: @unittest.skipUnless(importlib.util.find_spec("click"), "Click is not installed") def test_project_owner_uses_checkout_scoped_run_bundle(self) -> None: - app = base_cli.App(name="project-native") + app = legacy_app(name="project-native") @app.command() def main(ctx: base_cli.Context) -> None: @@ -329,7 +333,7 @@ def main(ctx: base_cli.Context) -> None: @unittest.skipUnless(importlib.util.find_spec("click"), "Click is not installed") def test_run_app_uses_explicit_argv_for_history_and_log_metadata(self) -> None: - app = base_cli.App(name="history-explicit", version="0.1.0") + app = legacy_app(name="history-explicit", version="0.1.0") seen = {} current_endpoint = "https://current.example/path" stale_endpoint = "https://stale.invalid/path" @@ -380,7 +384,7 @@ def main(ctx: base_cli.Context, endpoint: str, token: str) -> None: @unittest.skipUnless(importlib.util.find_spec("click"), "Click is not installed") def test_app_records_failed_command_history(self) -> None: - app = base_cli.App(name="failing-history") + app = legacy_app(name="failing-history") @app.command() def main(ctx: base_cli.Context) -> int: @@ -405,7 +409,7 @@ def main(ctx: base_cli.Context) -> int: @unittest.skipUnless(importlib.util.find_spec("click"), "Click is not installed") def test_app_history_write_failures_do_not_fail_command(self) -> None: - app = base_cli.App(name="history-best-effort") + app = legacy_app(name="history-best-effort") @app.command() def main(ctx: base_cli.Context) -> None: diff --git a/tests/test_profile.py b/tests/test_profile.py index 70c773e..0d90dc4 100644 --- a/tests/test_profile.py +++ b/tests/test_profile.py @@ -56,6 +56,13 @@ def main(ctx: base_cli.Context) -> None: self.assertFalse((home / ".base.d").exists()) self.assertFalse((home / ".cache" / "base").exists()) + def test_app_defaults_to_generic_profile(self) -> None: + app = base_cli.App(name="plain-tool", log_to_file=False) + + self.assertIsNone(app.profile.history_writer) + self.assertIsNone(app.profile.display_command()) + self.assertEqual(app.profile.resolve_runtime("plain-tool", None).runtime_owner, "default") + def test_generic_profile_accepts_consumer_project_and_config_policies(self) -> None: seen: dict[str, object] = {} diff --git a/tests/test_testing.py b/tests/test_testing.py index fe8404d..d8a28c2 100644 --- a/tests/test_testing.py +++ b/tests/test_testing.py @@ -15,6 +15,10 @@ from base_cli.testing import invoke +def legacy_app(**kwargs: object) -> base_cli.App: + return base_cli.App(profile=base_cli.CliProfile.legacy_base(), **kwargs) + + class PackageExportTests(unittest.TestCase): def test_package_exports_testing_module_for_documented_access(self) -> None: env = os.environ.copy() @@ -51,7 +55,7 @@ def test_invoke_declares_click_result_return_type(self) -> None: self.assertIn("Result", str(return_annotation)) def test_invoke_writes_manifest_fixture_into_cwd(self) -> None: - app = base_cli.App(name="testing-manifest", log_to_file=False) + app = legacy_app(name="testing-manifest", log_to_file=False) seen: dict[str, Path | None] = {} @app.command() @@ -80,13 +84,13 @@ def main(ctx: base_cli.Context) -> None: self.assertEqual(seen["manifest_path"], manifest_path.resolve()) def test_invoke_rejects_manifest_without_cwd(self) -> None: - app = base_cli.App(name="testing-manifest-without-cwd", log_to_file=False) + app = legacy_app(name="testing-manifest-without-cwd", log_to_file=False) with self.assertRaisesRegex(ValueError, "manifest requires cwd"): invoke(app, [], manifest={"project": {"name": "demo"}}) def test_invoke_with_cwd_exposes_process_cwd_and_restores_it(self) -> None: - app = base_cli.App(name="testing-cwd-isolation", log_to_file=False) + app = legacy_app(name="testing-cwd-isolation", log_to_file=False) seen: dict[str, Path | None | str] = {} @app.command() diff --git a/tests/test_user_config_github.py b/tests/test_user_config_github.py index d993ce1..a785de0 100644 --- a/tests/test_user_config_github.py +++ b/tests/test_user_config_github.py @@ -9,6 +9,10 @@ from base_cli.config import read_user_config, user_config_path +def legacy_app(**kwargs: object) -> base_cli.App: + return base_cli.App(profile=base_cli.CliProfile.legacy_base(), **kwargs) + + class GithubUserConfigTests(unittest.TestCase): def test_read_user_config_defaults_github_settings_to_none(self) -> None: with tempfile.TemporaryDirectory() as tmpdir: @@ -70,7 +74,7 @@ def test_read_user_config_rejects_invalid_github_clone_protocol(self) -> None: @unittest.skipUnless(importlib.util.find_spec("click"), "Click is not installed") def test_context_exposes_github_typed_user_config(self) -> None: - app = base_cli.App(name="typed-config-github", log_to_file=False) + app = legacy_app(name="typed-config-github", log_to_file=False) seen = {} @app.command()