From f44c4f9e3cb862fdec098445955562e04e06fef9 Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Fri, 20 Mar 2026 09:44:06 +0000 Subject: [PATCH] refactor: remove deprecated memory_window, harden wizard display --- nanobot/cli/commands.py | 26 +++++++++++++--------- nanobot/cli/onboard_wizard.py | 38 ++++++++++++++++---------------- nanobot/config/schema.py | 9 +------- tests/test_commands.py | 31 +++++--------------------- tests/test_config_migration.py | 12 +++------- tests/test_consolidate_offset.py | 2 +- 6 files changed, 44 insertions(+), 74 deletions(-) diff --git a/nanobot/cli/commands.py b/nanobot/cli/commands.py index de49668..9d3c78b 100644 --- a/nanobot/cli/commands.py +++ b/nanobot/cli/commands.py @@ -322,9 +322,6 @@ def onboard( console.print(f"[red]✗[/red] Error during configuration: {e}") console.print("[yellow]Please run 'nanobot onboard' again to complete setup.[/yellow]") raise typer.Exit(1) - else: - console.print("[dim]Config template now uses `maxTokens` + `contextWindowTokens`; `memoryWindow` is no longer a runtime setting.[/dim]") - _onboard_plugins(config_path) # Create workspace, preferring the configured workspace path. @@ -464,21 +461,30 @@ def _load_runtime_config(config: str | None = None, workspace: str | None = None console.print(f"[dim]Using config: {config_path}[/dim]") loaded = load_config(config_path) + _warn_deprecated_config_keys(config_path) if workspace: loaded.agents.defaults.workspace = workspace return loaded -def _print_deprecated_memory_window_notice(config: Config) -> None: - """Warn when running with old memoryWindow-only config.""" - if config.agents.defaults.should_warn_deprecated_memory_window: +def _warn_deprecated_config_keys(config_path: Path | None) -> None: + """Hint users to remove obsolete keys from their config file.""" + import json + from nanobot.config.loader import get_config_path + + path = config_path or get_config_path() + try: + raw = json.loads(path.read_text(encoding="utf-8")) + except Exception: + return + if "memoryWindow" in raw.get("agents", {}).get("defaults", {}): console.print( - "[yellow]Hint:[/yellow] Detected deprecated `memoryWindow` without " - "`contextWindowTokens`. `memoryWindow` is ignored; run " - "[cyan]nanobot onboard[/cyan] to refresh your config template." + "[dim]Hint: `memoryWindow` in your config is no longer used " + "and can be safely removed.[/dim]" ) + # ============================================================================ # Gateway / Server # ============================================================================ @@ -506,7 +512,6 @@ def gateway( logging.basicConfig(level=logging.DEBUG) config = _load_runtime_config(config, workspace) - _print_deprecated_memory_window_notice(config) port = port if port is not None else config.gateway.port console.print(f"{__logo__} Starting nanobot gateway version {__version__} on port {port}...") @@ -697,7 +702,6 @@ def agent( from nanobot.cron.service import CronService config = _load_runtime_config(config, workspace) - _print_deprecated_memory_window_notice(config) sync_workspace_templates(config.workspace_path) bus = MessageBus() diff --git a/nanobot/cli/onboard_wizard.py b/nanobot/cli/onboard_wizard.py index f661375..2537dcc 100644 --- a/nanobot/cli/onboard_wizard.py +++ b/nanobot/cli/onboard_wizard.py @@ -247,12 +247,20 @@ def _mask_value(value: str) -> str: def _format_value(value: Any, rich: bool = True, field_name: str = "") -> str: - """Format a value for display, masking sensitive fields.""" + """Single recursive entry point for safe value display. Handles any depth.""" if value is None or value == "" or value == {} or value == []: return "[dim]not set[/dim]" if rich else "[not set]" - if field_name and _is_sensitive_field(field_name) and isinstance(value, str): + if _is_sensitive_field(field_name) and isinstance(value, str): masked = _mask_value(value) return f"[dim]{masked}[/dim]" if rich else masked + if isinstance(value, BaseModel): + parts = [] + for fname, _finfo in type(value).model_fields.items(): + fval = getattr(value, fname, None) + formatted = _format_value(fval, rich=False, field_name=fname) + if formatted != "[not set]": + parts.append(f"{fname}={formatted}") + return ", ".join(parts) if parts else ("[dim]not set[/dim]" if rich else "[not set]") if isinstance(value, list): return ", ".join(str(v) for v in value) if isinstance(value, dict): @@ -543,6 +551,7 @@ def _configure_pydantic_model( return items + ["[Done]"] while True: + console.clear() _show_config_panel(display_name, working_model, fields) choices = get_choices() answer = _select_with_back("Select field to configure:", choices) @@ -688,7 +697,6 @@ def _configure_provider(config: Config, provider_name: str) -> None: def _configure_providers(config: Config) -> None: """Configure LLM providers.""" - _show_section_header("LLM Providers", "Select a provider to configure API key and endpoint") def get_provider_choices() -> list[str]: """Build provider choices with config status indicators.""" @@ -703,6 +711,8 @@ def _configure_providers(config: Config) -> None: while True: try: + console.clear() + _show_section_header("LLM Providers", "Select a provider to configure API key and endpoint") choices = get_provider_choices() answer = _select_with_back("Select provider:", choices) @@ -738,18 +748,9 @@ def _get_channel_info() -> dict[str, tuple[str, type[BaseModel]]]: for name, channel_cls in discover_all().items(): try: mod = importlib.import_module(f"nanobot.channels.{name}") - config_cls = next( - ( - attr - for attr in vars(mod).values() - if isinstance(attr, type) - and issubclass(attr, BaseModel) - and attr is not BaseModel - and attr.__name__.endswith("Config") - ), - None, - ) - if config_cls: + config_name = channel_cls.__name__.replace("Channel", "Config") + config_cls = getattr(mod, config_name, None) + if config_cls and isinstance(config_cls, type) and issubclass(config_cls, BaseModel): display_name = getattr(channel_cls, "display_name", name.capitalize()) result[name] = (display_name, config_cls) except Exception: @@ -795,13 +796,13 @@ def _configure_channel(config: Config, channel_name: str) -> None: def _configure_channels(config: Config) -> None: """Configure chat channels.""" - _show_section_header("Chat Channels", "Select a channel to configure connection settings") - channel_names = list(_get_channel_names().keys()) choices = channel_names + ["<- Back"] while True: try: + console.clear() + _show_section_header("Chat Channels", "Select a channel to configure connection settings") answer = _select_with_back("Select channel:", choices) if answer is _BACK_PRESSED or answer is None or answer == "<- Back": @@ -842,8 +843,6 @@ def _configure_general_settings(config: Config, section: str) -> None: if not meta: return display_name, subtitle, skip = meta - _show_section_header(section, subtitle) - model = _SETTINGS_GETTER[section](config) updated = _configure_pydantic_model(model, display_name, skip_fields=skip) if updated is not None: @@ -975,6 +974,7 @@ def run_onboard(initial_config: Config | None = None) -> OnboardResult: config = base_config.model_copy(deep=True) while True: + console.clear() _show_main_menu_header() try: diff --git a/nanobot/config/schema.py b/nanobot/config/schema.py index c067231..aa7e80d 100644 --- a/nanobot/config/schema.py +++ b/nanobot/config/schema.py @@ -38,14 +38,7 @@ class AgentDefaults(Base): context_window_tokens: int = 65_536 temperature: float = 0.1 max_tool_iterations: int = 40 - # Deprecated compatibility field: accepted from old configs but ignored at runtime. - memory_window: int | None = Field(default=None, exclude=True) - reasoning_effort: str | None = None # low / medium / high — enables LLM thinking mode - - @property - def should_warn_deprecated_memory_window(self) -> bool: - """Return True when old memoryWindow is present without contextWindowTokens.""" - return self.memory_window is not None and "context_window_tokens" not in self.model_fields_set + reasoning_effort: str | None = None # low / medium / high - enables LLM thinking mode class AgentsConfig(Base): diff --git a/tests/test_commands.py b/tests/test_commands.py index 08ed59e..6020856 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -452,14 +452,15 @@ def test_agent_workspace_override_wins_over_config_workspace(mock_agent_runtime, assert mock_agent_runtime["agent_loop_cls"].call_args.kwargs["workspace"] == workspace_path -def test_agent_warns_about_deprecated_memory_window(mock_agent_runtime): - mock_agent_runtime["config"].agents.defaults.memory_window = 100 +def test_agent_hints_about_deprecated_memory_window(mock_agent_runtime, tmp_path): + config_file = tmp_path / "config.json" + config_file.write_text(json.dumps({"agents": {"defaults": {"memoryWindow": 42}}})) - result = runner.invoke(app, ["agent", "-m", "hello"]) + result = runner.invoke(app, ["agent", "-m", "hello", "-c", str(config_file)]) assert result.exit_code == 0 assert "memoryWindow" in result.stdout - assert "contextWindowTokens" in result.stdout + assert "no longer used" in result.stdout def test_gateway_uses_workspace_from_config_by_default(monkeypatch, tmp_path: Path) -> None: @@ -523,28 +524,6 @@ def test_gateway_workspace_option_overrides_config(monkeypatch, tmp_path: Path) assert config.workspace_path == override -def test_gateway_warns_about_deprecated_memory_window(monkeypatch, tmp_path: Path) -> None: - config_file = tmp_path / "instance" / "config.json" - config_file.parent.mkdir(parents=True) - config_file.write_text("{}") - - config = Config() - config.agents.defaults.memory_window = 100 - - monkeypatch.setattr("nanobot.config.loader.set_config_path", lambda _path: None) - monkeypatch.setattr("nanobot.config.loader.load_config", lambda _path=None: config) - monkeypatch.setattr("nanobot.cli.commands.sync_workspace_templates", lambda _path: None) - monkeypatch.setattr( - "nanobot.cli.commands._make_provider", - lambda _config: (_ for _ in ()).throw(_StopGatewayError("stop")), - ) - - result = runner.invoke(app, ["gateway", "--config", str(config_file)]) - - assert isinstance(result.exception, _StopGatewayError) - assert "memoryWindow" in result.stdout - assert "contextWindowTokens" in result.stdout - def test_gateway_uses_config_directory_for_cron_store(monkeypatch, tmp_path: Path) -> None: config_file = tmp_path / "instance" / "config.json" config_file.parent.mkdir(parents=True) diff --git a/tests/test_config_migration.py b/tests/test_config_migration.py index 7728c26..c1c9510 100644 --- a/tests/test_config_migration.py +++ b/tests/test_config_migration.py @@ -3,7 +3,7 @@ import json from nanobot.config.loader import load_config, save_config -def test_load_config_keeps_max_tokens_and_warns_on_legacy_memory_window(tmp_path) -> None: +def test_load_config_keeps_max_tokens_and_ignores_legacy_memory_window(tmp_path) -> None: config_path = tmp_path / "config.json" config_path.write_text( json.dumps( @@ -23,7 +23,7 @@ def test_load_config_keeps_max_tokens_and_warns_on_legacy_memory_window(tmp_path assert config.agents.defaults.max_tokens == 1234 assert config.agents.defaults.context_window_tokens == 65_536 - assert config.agents.defaults.should_warn_deprecated_memory_window is True + assert not hasattr(config.agents.defaults, "memory_window") def test_save_config_writes_context_window_tokens_but_not_memory_window(tmp_path) -> None: @@ -52,7 +52,7 @@ def test_save_config_writes_context_window_tokens_but_not_memory_window(tmp_path assert "memoryWindow" not in defaults -def test_onboard_refresh_rewrites_legacy_config_template(tmp_path, monkeypatch) -> None: +def test_onboard_does_not_crash_with_legacy_memory_window(tmp_path, monkeypatch) -> None: config_path = tmp_path / "config.json" workspace = tmp_path / "workspace" config_path.write_text( @@ -78,12 +78,6 @@ def test_onboard_refresh_rewrites_legacy_config_template(tmp_path, monkeypatch) result = runner.invoke(app, ["onboard"], input="n\n") assert result.exit_code == 0 - assert "contextWindowTokens" in result.stdout - saved = json.loads(config_path.read_text(encoding="utf-8")) - defaults = saved["agents"]["defaults"] - assert defaults["maxTokens"] == 3333 - assert defaults["contextWindowTokens"] == 65_536 - assert "memoryWindow" not in defaults def test_onboard_refresh_backfills_missing_channel_fields(tmp_path, monkeypatch) -> None: diff --git a/tests/test_consolidate_offset.py b/tests/test_consolidate_offset.py index 21e1e78..4f2e8f1 100644 --- a/tests/test_consolidate_offset.py +++ b/tests/test_consolidate_offset.py @@ -182,7 +182,7 @@ class TestConsolidationTriggerConditions: """Test consolidation trigger conditions and logic.""" def test_consolidation_needed_when_messages_exceed_window(self): - """Test consolidation logic: should trigger when messages > memory_window.""" + """Test consolidation logic: should trigger when messages exceed the window.""" session = create_session_with_messages("test:trigger", 60) total_messages = len(session.messages)