test: add pytest suite + fix the CI gates that were failing on main (#4)

* chore: format sweep + Python 3.8-3.11 f-string compat fix

Two separate hygiene fixes bundled because they both block the new
test suite from running green in CI:

1. **black + isort sweep across 17 source files.** The repo was
   never run through `black` after the formatter became a CI gate
   in `.github/workflows/test.yml`. Pure whitespace / import-order
   changes; no logic touched.

2. **`tools/data_tools._generate_create_table_sql` syntax fix.**
   Line 188 used `{',\n    '.join(column_definitions)}` inside an
   f-string. Backslashes in f-string expression parts are a
   SyntaxError on Python 3.8 - 3.11 (only legal from 3.12 onward),
   and `pyproject.toml` declares `requires-python = ">=3.8"` plus
   the CI matrix runs 3.8-3.12. The module is unimportable on every
   pre-3.12 Python; CI never caught it because nothing imported the
   module. Fixed by binding the separator to a name (`sep = ",\n  "`)
   so the f-string expression is backslash-free. Behaviour
   identical.

After this commit, `black --check`, `isort --check-only`, and
`flake8 . --select=E9,F63,F7,F82` all pass cleanly. The next commit
adds the test suite that surfaced both gaps.

* test: add pytest suite for config, data_tools, and CLI

Bootstraps clickr's first real test suite. The CI workflow at
`.github/workflows/test.yml` already invokes `pytest tests/`, but
the directory did not exist - so every "passing" green check on
main was a no-op. This commit makes the green meaningful: 52 tests
across three files, all running in <1s on Python 3.9.

`tests/conftest.py` - shared fixtures. The autouse `_isolate_env`
fixture strips `CLICKHOUSE_*` and `OPENROUTER_*` from `os.environ`
and points `$HOME` at a `tmp_path` so the loader's probe for
`~/.config/proto/proto-config.json` never finds the developer's
real config. Without this, tests pass locally but fail in CI (or
vice versa) depending on whose machine they run on.

`tests/test_settings.py` (28 tests) - covers `ClickHouseConfig`
defaults and validation, plus every input source `load_config`
merges (file, env vars, CLI args, the legacy `clickhouse_*` key
mapping from the onboarding flow), and the precedence between
them (CLI > env > file). Also covers the silent
`provider != "local"` coercion (so an old config doesn't break
boot) and the `create_sample_config` round-trip.

`tests/test_data_tools.py` (15 tests) - exercises every dtype
branch of `_generate_create_table_sql` (UInt64 / Int64 /
Float64 / Bool / DateTime / String fallback for mixed-object
columns) and the structural invariants (IF NOT EXISTS, MergeTree
engine, ORDER BY tuple(), backtick quoting for reserved-word
columns, multi-column composition, empty-DataFrame degenerate
case). The function does not touch `self.client`, so the fixture
passes `None`.

`tests/test_cli.py` (9 tests) - Typer `CliRunner` smoke tests:
top-level `--help`, that documented subcommands appear in help
(catches silent renames), every subcommand has a working `--help`,
`version` returns 0, and unknown commands return non-zero. Does
not exercise the LLM provider or ClickHouse client - those need
real services.

Methodology choices worth flagging:

- No mocks for ClickHouse or the LLM. Mocked tests for an HTTP
  client tend to lock in the mock's view of reality and rot when
  the real service moves. The pure-data-validation tests cover
  what is actually testable without a server.
- Tests use the public API (`load_config`, `ClickHouseConfig`,
  `_generate_create_table_sql`) rather than asserting against
  internal helpers. A refactor of internals should not need to
  touch this file.
- Each test class groups behaviour, not function-under-test, so a
  reader can scan the test names for the contract instead of
  reading the code.

* fix(packaging): use legacy license-table syntax so install works on Python 3.8

`license = "MIT"` is the new PEP 639 SPDX-string form, only understood
by setuptools >= 77 (released Feb 2025). The CI matrix includes
Python 3.8, whose bundled setuptools predates that — so
`pip install -e ".[dev]"` fails on the ubuntu-latest 3.8 job with:

    ValueError: invalid pyproject.toml config: `project.license`.
    configuration error: `project.license` must be valid exactly by
    one definition (2 matches found):
        - keys: 'file': {type: string}
        - keys: 'text': {type: string}

`license = { text = "MIT" }` is the pre-PEP-639 table form, understood
by every setuptools version that supports pyproject.toml, so it works
across the full 3.8 - 3.12 matrix without forcing a setuptools-upgrade
step into the CI install. Same metadata reaches PyPI either way
(`License: MIT` classifier).

* chore: drop Python 3.8 (EOL Oct 2024) from CI matrix and metadata

The latest `clickhouse-connect` (a transitive dep) uses PEP 585
generic-subscript syntax (`list[IPv6Address]`) without bumping its
own `requires-python` past 3.8 — so on Python 3.8 pip resolves to a
version of clickhouse-connect that fails to import with
`TypeError: 'type' object is not subscriptable`. The Python 3.8
test job was failing before this PR for that reason; the matrix
ran for years on luck (no test ever imported clickhouse-connect).

Python 3.8 reached end-of-life on 2024-10-07. Dropping it across:
- `.github/workflows/test.yml` matrix: 3.8 → 3.9 floor
- `pyproject.toml`: `requires-python`, classifier, mypy
  `python_version`, black `target-version`
- `setup.py`: `python_requires`, classifier

Cuts the matrix from 15 jobs to 12 and matches the practical reality
of the dep tree. Anyone genuinely on 3.8 can still install older
clickr versions from PyPI; new 3.9+ becomes the supported floor.

* fix(packaging): read README.md with explicit utf-8 encoding for Windows

`Path.read_text()` without `encoding=` falls back to
`locale.getpreferredencoding()` — UTF-8 on macOS/Linux, but cp1252
on Windows. README.md contains the ERP•AI brand mark (`•`) and
em-dashes, which fail to decode as cp1252 and crash the install:

    UnicodeDecodeError: 'charmap' codec can't decode byte 0x9d in
    position 480: character maps to <undefined>

Pinning `encoding="utf-8"` makes the install reliable across the
matrix. The same bug pattern bites every Python project that does
`open("README.md").read()` without encoding; would not surface
until someone runs CI (or installs) on Windows.

* test(conftest): point USERPROFILE + HOMEDRIVE/HOMEPATH at tmp_path for Windows

`Path.home()` reads HOME on POSIX but USERPROFILE on Windows (with
HOMEDRIVE + HOMEPATH as a fallback). The autouse fixture only set
HOME, so on Windows runners the loader's `~/.config/proto/...`
probe escaped the sandbox and resolved to the real runner's home —
where there is no proto-config.json — and
test_default_config_in_xdg_home_is_picked_up failed with
`assert 'localhost' == 'xdg.example.com'`.

Setting all three env vars (HOME, USERPROFILE, HOMEDRIVE+HOMEPATH)
makes the fixture portable across the full ubuntu/macos/windows
matrix without forking the test or skipping on Windows.

Author: protosphinx

.github/workflows/test.yml

@@ -12,7 +12,7 @@ jobs:
     strategy:
       matrix:
         os: [ubuntu-latest, macos-latest, windows-latest]
-        python-version: [3.8, 3.9, '3.10', '3.11', '3.12']
+        python-version: ['3.9', '3.10', '3.11', '3.12']
 
     steps:
     - uses: actions/checkout@v4

agent/__init__.py

@@ -1,3 +1,3 @@
 from .clickhouse_agent import ClickHouseAgent
 
-__all__ = ['ClickHouseAgent']
+__all__ = ["ClickHouseAgent"]

agent/clickhouse_agent.py

@@ -1 +1 @@
-# Config package
+# Config package

config/__init__.py

@@ -7,16 +7,17 @@
 from typing import Optional
 
 import typer
+from dotenv import load_dotenv
 from pydantic import BaseModel, Field
 from rich.console import Console
-from rich.prompt import Prompt, Confirm
-from dotenv import load_dotenv
+from rich.prompt import Confirm, Prompt
 
 console = Console()
 
+
 class ClickHouseConfig(BaseModel):
     """ClickHouse connection configuration"""
-    
+
     host: str = Field(default="localhost", description="ClickHouse host")
     port: int = Field(default=8123, description="ClickHouse HTTP port")
     username: str = Field(default="default", description="ClickHouse username")
@@ -26,37 +27,50 @@ class ClickHouseConfig(BaseModel):
 
     # Provider selection
     provider: str = Field(default="local", description="LLM provider: local only")
-    
+
     # Local LLM (llama.cpp / llamafile / llama-cpp-python server) configuration
-    local_llm_base_url: str = Field(default="http://127.0.0.1:8000/v1", description="Local LLM server base URL")
-    local_llm_model: str = Field(default="qwen3-1.7b", description="Local LLM model name")
-    
+    local_llm_base_url: str = Field(
+        default="http://127.0.0.1:8000/v1", description="Local LLM server base URL"
+    )
+    local_llm_model: str = Field(
+        default="qwen3-1.7b", description="Local LLM model name"
+    )
+
     # Legacy OpenRouter configuration (kept for backward compatibility)
     openrouter_api_key: str = Field(default="", description="Legacy OpenRouter API key")
-    openrouter_model: str = Field(default="openai/gpt-4o-mini", description="Legacy OpenRouter model")
-    openrouter_provider_only: str = Field(default="openai", description="Legacy OpenRouter provider preference")
-    openrouter_data_collection: str = Field(default="deny", description="Legacy OpenRouter data collection setting")
-    
+    openrouter_model: str = Field(
+        default="openai/gpt-4o-mini", description="Legacy OpenRouter model"
+    )
+    openrouter_provider_only: str = Field(
+        default="openai", description="Legacy OpenRouter provider preference"
+    )
+    openrouter_data_collection: str = Field(
+        default="deny", description="Legacy OpenRouter data collection setting"
+    )
+
     # Agent configuration
-    max_tool_calls: int = Field(default=35, description="Maximum tool calls per conversation")
+    max_tool_calls: int = Field(
+        default=35, description="Maximum tool calls per conversation"
+    )
     temperature: float = Field(default=0.1, description="LLM temperature")
     max_tokens: int = Field(default=4000, description="Maximum tokens per response")
 
+
 def load_config(
     config_file: Optional[Path] = None,
     host: Optional[str] = None,
     port: Optional[int] = None,
     username: Optional[str] = None,
     password: Optional[str] = None,
-    database: Optional[str] = None
+    database: Optional[str] = None,
 ) -> ClickHouseConfig:
     """Load configuration from file and command line arguments"""
-    
+
     # Load environment variables from .env file
     load_dotenv()
-    
+
     config_data = {}
-    
+
     # Determine default config file if not provided
     default_config_file = Path.home() / ".config" / "proto" / "proto-config.json"
     legacy_config_file = Path("proto-config.json")
@@ -71,6 +85,7 @@ def load_config(
     # Load from config file if found
     if candidate_config and candidate_config.exists():
         import json
+
         with open(candidate_config) as f:
             config_data = json.load(f)
 
@@ -86,27 +101,31 @@ def load_config(
     for old_key, new_key in key_mapping.items():
         if old_key in config_data and new_key not in config_data:
             config_data[new_key] = config_data[old_key]
-    
+
     # Load from environment variables
-    config_data.update({
-        k.lower().replace("clickhouse_", ""): v 
-        for k, v in os.environ.items() 
-        if k.startswith("CLICKHOUSE_")
-    })
-    
+    config_data.update(
+        {
+            k.lower().replace("clickhouse_", ""): v
+            for k, v in os.environ.items()
+            if k.startswith("CLICKHOUSE_")
+        }
+    )
+
     # OpenRouter configuration from environment
     if "OPENROUTER_API_KEY" in os.environ:
         config_data["openrouter_api_key"] = os.environ["OPENROUTER_API_KEY"]
-    
+
     if "OPENROUTER_MODEL" in os.environ:
         config_data["openrouter_model"] = os.environ["OPENROUTER_MODEL"]
-    
+
     if "OPENROUTER_PROVIDER_ONLY" in os.environ:
         config_data["openrouter_provider_only"] = os.environ["OPENROUTER_PROVIDER_ONLY"]
-    
+
     if "OPENROUTER_DATA_COLLECTION" in os.environ:
-        config_data["openrouter_data_collection"] = os.environ["OPENROUTER_DATA_COLLECTION"]
-    
+        config_data["openrouter_data_collection"] = os.environ[
+            "OPENROUTER_DATA_COLLECTION"
+        ]
+
     # Override with command line arguments
     if host:
         config_data["host"] = host
@@ -118,21 +137,22 @@ def load_config(
         config_data["password"] = password
     if database:
         config_data["database"] = database
-    
+
     config = ClickHouseConfig(**config_data)
-    
+
     # No interactive configuration needed for local provider
     if config.provider != "local":
         console.print("[yellow]⚠️  Only local provider is supported[/yellow]")
         console.print("[blue]ℹ️  Switching to local provider automatically[/blue]")
         config.provider = "local"
-    
+
     return config
 
+
 def save_env_config(config: ClickHouseConfig):
     """Save configuration to .env file"""
     env_path = Path(".env")
-    
+
     env_content = f"""# ClickHouse Configuration
 CLICKHOUSE_HOST={config.host}
 CLICKHOUSE_PORT={config.port}
@@ -147,12 +167,13 @@ def save_env_config(config: ClickHouseConfig):
 OPENROUTER_PROVIDER_ONLY={config.openrouter_provider_only}
 OPENROUTER_DATA_COLLECTION={config.openrouter_data_collection}
 """
-    
+
     with open(env_path, "w") as f:
         f.write(env_content)
-    
+
     console.print(f"[green]✓[/green] Configuration saved to {env_path}")
 
+
 def create_sample_config():
     """Create a sample configuration file"""
     config_data = {
@@ -168,13 +189,14 @@ def create_sample_config():
         "openrouter_data_collection": "deny",
         "max_tool_calls": 35,
         "temperature": 0.1,
-        "max_tokens": 4000
+        "max_tokens": 4000,
     }
-    
+
     config_path = Path("proto-config.json")
-    
+
     import json
+
     with open(config_path, "w") as f:
         json.dump(config_data, f, indent=2)
-    
-    console.print(f"[green]✓[/green] Sample configuration created at {config_path}")
+
+    console.print(f"[green]✓[/green] Sample configuration created at {config_path}")