From 5c43b3d5a194a3d0629c5db16e15bd2ff3fb7462 Mon Sep 17 00:00:00 2001 From: Joe Hartshorn <8881940+j-hartshorn@users.noreply.github.com> Date: Thu, 21 May 2026 09:58:29 +0100 Subject: [PATCH 01/10] docs: spec local-only format and lint Plan the change to make `sqlmesh format` and `sqlmesh lint` runnable without state-sync credentials or any external connection, so they work in CI jobs that have no warehouse access. Captures the work that led to spec.md: three research docs covering how `format` runs, what every CLI invocation does at bootstrap, and the contribution workflow; spec.md itself; and spec_notes.md with the full CLI command audit, the `dag` Selector wart, mechanism and naming alternatives, and the verification methodology used to confirm `format` and `lint` are clean. No code changes yet. Coding-Agent: pi Model: anthropic/claude-opus-4-7 Signed-off-by: Joe Hartshorn <8881940+j-hartshorn@users.noreply.github.com> --- .../01_format_command.md | 166 +++++++++++++ .../02_cli_bootstrap.md | 169 +++++++++++++ .../03_contributing.md | 230 ++++++++++++++++++ docs/2026-05-21_local-only-format/spec.md | 79 ++++++ .../spec_notes.md | 101 ++++++++ 5 files changed, 745 insertions(+) create mode 100644 docs/2026-05-21_local-only-format/01_format_command.md create mode 100644 docs/2026-05-21_local-only-format/02_cli_bootstrap.md create mode 100644 docs/2026-05-21_local-only-format/03_contributing.md create mode 100644 docs/2026-05-21_local-only-format/spec.md create mode 100644 docs/2026-05-21_local-only-format/spec_notes.md diff --git a/docs/2026-05-21_local-only-format/01_format_command.md b/docs/2026-05-21_local-only-format/01_format_command.md new file mode 100644 index 0000000000..1c5eda3a9b --- /dev/null +++ b/docs/2026-05-21_local-only-format/01_format_command.md @@ -0,0 +1,166 @@ +# SQLMesh Format CLI Command Reference + +This document provides a technical overview and architectural reference for how the `sqlmesh format` command is defined, processed, and executed within the SQLMesh codebase. + +## 1. CLI Entry Point & Click Command Definition +The `format` command is defined as a Click CLI command in the file **`sqlmesh/cli/main.py`** at lines **343–380**: + +```python +@cli.command("format") +@click.argument("paths", nargs=-1) +@click.option( + "-t", + "--transpile", + type=str, + help="Transpile project models to the specified dialect.", +) +@click.option( + "--check", + is_flag=True, + help="Whether or not to check formatting (but not actually format anything).", + default=None, +) +@click.option( + "--rewrite-casts/--no-rewrite-casts", + is_flag=True, + help="Rewrite casts to use the :: syntax.", + default=None, +) +@click.option( + "--append-newline", + is_flag=True, + help="Include a newline at the end of each file.", + default=None, +) +@opt.format_options +@click.pass_context +@error_handler +@cli_analytics +def format( + ctx: click.Context, paths: t.Optional[t.Tuple[str, ...]] = None, **kwargs: t.Any +) -> None: + """Format all SQL models and audits.""" + if not ctx.obj.format(**{k: v for k, v in kwargs.items() if v is not None}, paths=paths): + ctx.exit(1) +``` + +The command takes an optional positional argument `paths` to limit formatting to specific files or directories, and routes all keyword arguments down to `Context.format(...)`. + +--- + +## 2. The Execution Call Chain +When a user executes `sqlmesh format` in the terminal, the execution traverses the following call chain: + +1. **`sqlmesh/cli/main.py` (Line 343)**: The Click CLI parses options/arguments and invokes the `format(...)` command handler. +2. **`sqlmesh/cli/main.py` (Line 377)**: The CLI command delegates execution to the core context format method by calling `ctx.obj.format(...)`. Here, `ctx.obj` is an instance of the `Context` class. +3. **`sqlmesh/core/context.py` (Line 1220)**: `Context.format(...)` performs the orchestration: + - Filters down models and audits from the loaded context (`self._models.values()` and `self._audits.values()`) that are physical `.sql` files on disk (and match specified `paths` filters, if any). + - Skips targets where `target.formatting is False` (configured in model/audit metadata). + - Iterates through the files, opening each one to read its content as `before` (Line 1247). + - Calls `self._format(target, before, ...)` to generate the newly formatted content as `after`. +4. **`sqlmesh/core/context.py` (Line 1280)**: `Context._format(...)` parses and delegates rendering: + - Calls `parse(before, default_dialect=self.config_for_node(target).dialect)` to tokenize and parse the SQL file into a list of SQLGlot AST expressions (defined in `sqlmesh/core/dialect.py` at line 907). + - If transpilation is requested via `--transpile`, it updates the model's/audit's metadata dialect in-place within the first AST expression (the `MODEL` or `AUDIT` metadata block) (Lines 1290–1294). + - Resolves formatting configuration settings from the node's path config: `format_config = self.config_for_node(target).format` (Line 1296). + - Calls `format_model_expressions(expressions, ...)` to format the parsed expressions. +5. **`sqlmesh/core/dialect.py` (Line 754)**: `format_model_expressions(...)` performs the pretty-printing: + - If `rewrite_casts` is set (or configured by default), runs a recursive AST transformer (`cast_to_colon`) to rewrite `exp.Cast` AST nodes as double-colon cast nodes (`DColonCast` / `::`), unless they have custom dialect arguments (Lines 771–796). + - Executes `.sql(pretty=True, dialect=dialect, **kwargs)` on each AST expression to generate pretty-printed SQL string representations, forwarding general SQLGlot generator options. + - Joins multiple statements together with double newlines and semicolons (`";\n\n"`). +6. **`sqlmesh/core/context.py` (Lines 1259–1262)**: + - If the `--check` flag is active, compares `before` and `after`, and registers mismatched paths in `unformatted_file_paths` without modifying the disk. + - Otherwise, seeks back to `0`, writes `after` to the file, and truncates. + +--- + +## 3. Command Options & Configuration +The formatting CLI command accepts the following CLI arguments and flags: + +### Primary CLI Flags: +* **`paths`** (Positional, `nargs=-1`): Limits formatting check or update to specified file or directory paths. +* **`-t`, `--transpile TEXT`**: Transpiles models/audits to a specific target dialect (e.g., `snowflake`, `bigquery`), writing both formatted SQL and updated metadata back to the source files. +* **`--check`**: Evaluates formatting without modifying files. Exits with code `1` if any file needs reformatting. +* **`--rewrite-casts / --no-rewrite-casts`**: Toggles rewriting standard SQL `CAST(x AS TYPE)` syntax to the shorter `x::TYPE` syntax. +* **`--append-newline`**: Ensures a trailing newline is appended to the end of every formatted file. + +### Common SQLGlot Generator Flags (defined under `@opt.format_options` decorator in `sqlmesh/cli/options.py:60`): +* **`--normalize`**: Normalizes all unquoted identifiers to lowercase. +* **`--pad INTEGER`**: Determines padding size (spaces) used in formatting. +* **`--indent INTEGER`**: Determines indentation size (spaces) used in formatting. +* **`--normalize-functions TEXT`**: Formats function names to a specific case (`'upper'` or `'lower'`). +* **`--leading-comma`**: Formats SELECT list commas as leading instead of trailing. +* **`--max-text-width INTEGER`**: Sets maximum line character width before wrapping. + +### Configuration via Config File / Environment variables: +Options can be set in the main config file under `format` (mapped to `FormatConfig` in `sqlmesh/core/config/format.py`), or as model defaults (`ModelDefaultsConfig(formatting=False)` in `sqlmesh/core/config/model.py`), or through environment variables (e.g., `SQLMESH__FORMAT__LEADING_COMMA=true`). + +--- + +## 4. File Discovery & File Types Touched +The `format` command is strictly designed to discover and modify **SQL models and audits** (.sql files only). + +* **Discovery**: SQLMesh loads model definitions via its loader system. The standard loader `SqlMeshLoader` (`sqlmesh/core/loader.py:481`) globs SQL files recursively in the project's `models/` (`c.MODELS`) and `audits/` directories. +* **Filtering**: In `Context.format()`, targets are filtered using the list comprehension: + ```python + filtered_targets = [ + target + for target in chain(self._models.values(), self._audits.values()) + if target._path is not None + and target._path.suffix == ".sql" + and (not paths or any(target._path.samefile(p) for p in paths)) + ] + ``` +* **Excluded File Types**: The command does **not** touch Python models (`.py`), YAML model definitions (`.yaml`, `.yml`), macros, configurations, or schemas. + +--- + +## 5. Under-the-Hood AST Round-Trip +The formatting implementation performs a full **round-trip parsing** of the SQL source code: +1. **Source to AST**: The raw source string is parsed using `sqlmesh.core.dialect.parse`, which returns a list of SQLGlot AST node structures (`exp.Expr`). +2. **AST Mutation**: + - Updates meta-expressions (the `MODEL` or `AUDIT` block) to adjust dialect settings if transpilation is requested. + - Converts `Cast` AST nodes into `DColonCast` AST nodes for cast normalization. +3. **AST to Target SQL**: SQLGlot's `.sql(...)` generator produces the final formatted output from the modified AST using the configured dialect generator rules. +4. **Statement Stitching**: Statements are stitched back together using double newlines and semicolons. + +Because it is an AST-based round-trip, comments that are detached from AST nodes may be relocated or discarded, and invalid SQL syntax anywhere within a formatted file will result in parsing failures. + +--- + +## 6. Project Context & External Warehouse Dependencies +Running `sqlmesh format` **requires loading the full project context and connecting to the warehouse/state sync backend**. It is not a pure local formatting command. + +### Evidence: +1. **No Context Skip**: In `sqlmesh/cli/main.py` lines 25–43, `format` is **not** included in `SKIP_LOAD_COMMANDS` or `SKIP_CONTEXT_COMMANDS`. Therefore, `Context` is instantiated on command invocation with `load=True`. +2. **Full Project Loading**: Instantiating the context executes `context.load()`, which parses **every** script, model, and audit in the project recursively. A single syntax error in an unrelated model will block the formatting command from starting. +3. **Warehouse Connection/State Sync Hit**: + - Inside `Context.load()` (**`sqlmesh/core/context.py` at lines 677–689**), SQLMesh checks the production environment definition from state sync: + ```python + prod = self.state_reader.get_environment(c.PROD) + ``` + - Resolving `self.state_reader` forces the evaluation of the `self.state_sync` property (**`sqlmesh/core/context.py` at line 621**). + - If a state sync backend is uninitialized, the context evaluates `_new_state_sync()` (**line 2944**) which calls `_scheduler.create_state_sync(self)` to establish a connection to the configured database. + - Under database-backed state storage (e.g., Snowflake, BigQuery, Databricks), SQLMesh makes network/database connections to query state tables (and may run schema migrations if `schema_version == 0`). + - Consequently, running `sqlmesh format` requires valid warehouse credentials and an active database connection. + +--- + +## 7. Existing Format Tests +The following files cover format command behavior: + +* **`tests/core/test_format.py`**: + * `test_format_files`: Builds a temporary directory with multiple SQL models, audits, and inline audits. Asserts that formatting check detects unformatted files, verifies transpiling to BigQuery (rewriting both dialect metadata and query-specific syntax successfully), and validates formatting of nested/inline audits. + * `test_ignore_formating_files`: Validates that configuring `formatting false` in `MODEL` metadata, `AUDIT` metadata, or as a global configuration model default successfully skips formatting for those files. +* **`tests/cli/test_cli.py`**: + * `test_format_leading_comma_default`: Runs the CLI `sqlmesh format` command using a Click runner. Ensures environment variables (e.g., `SQLMESH__FORMAT__LEADING_COMMA`) are respected, and that explicit CLI flags (such as `--leading-comma`) override any environment variable configs. +* **`tests/core/test_dialect.py`**: + * `test_format_model_expressions`, `test_macro_format`, `test_format_body_macros`: Test that AST structures, custom macro syntaxes (e.g., `@WITH`, `@EACH`), comments, and lists are formatted into correct SQL syntax. +* **`tests/core/test_model.py` / `tests/core/test_audit.py`**: + * `test_formatting_flag_serde` / `test_audit_formatting_flag_serde`: Verify the `formatting` boolean field is excluded from JSON serializations but safely preserved across Pydantic models. + +--- + +## 8. Contributor Quirks & Non-Obvious Behavior +* **Unrelated Errors Block Execution**: Because the entire context is loaded on startup, any syntax/validation error in **any** file in the project will cause `sqlmesh format` to crash immediately—even if the user only specified a single valid file via the `paths` parameter. +* **In-place Metadata Modification**: Passing the `--transpile` flag will physically rewrite the `dialect` property within the model's SQL block `MODEL (...)` in the source file on disk. This is a mutating write that changes the dialect SQLMesh uses to load that model in all subsequent executions. +* **Credentials/Connection Dependency**: Due to loading state definitions during context initialization, local formatting can fail if database credentials are invalid or if there is no active internet connection to a remote data warehouse. diff --git a/docs/2026-05-21_local-only-format/02_cli_bootstrap.md b/docs/2026-05-21_local-only-format/02_cli_bootstrap.md new file mode 100644 index 0000000000..d8581fd055 --- /dev/null +++ b/docs/2026-05-21_local-only-format/02_cli_bootstrap.md @@ -0,0 +1,169 @@ +# Research: CLI Bootstrap and Lifecycle + +## Scope +This reference document investigates the general setup, initialization flow, and bootstrap sequence that execute upon invoking the `sqlmesh` or `sqlmesh_cicd` command-line interfaces. It covers CLI entry points, global options, configuration loading, Context instantiation, database and state sync connection lifecycles, and external touchpoints (such as telemetry, disk access, and external APIs). + +Specifically, this document analyzes the behavior of the `sqlmesh format` command to clarify what is always, sometimes, or never performed during its execution, focusing on whether a live database or state sync connection is required. + +## Summary +* **Entry Points:** CLI tools are defined as console scripts in `pyproject.toml`, mapping `sqlmesh` to `sqlmesh.cli.main:cli` and `sqlmesh_cicd` to `sqlmesh.cicd.bot:bot`. +* **Global CLI Parsing:** Top-level global options (such as paths, configs, gateways, and custom `.env` files) are parsed at the click group level before subcommands execute. +* **Configuration Discovery:** Configuration files (`config.py`, `config.yaml`, `sqlmesh.yaml`) are discovered from project paths and user folders (`~/.sqlmesh/`), combined with environment variables starting with `SQLMESH__`. +* **Eager vs. Lazy Loading:** By default, subcommands initialize the `Context` with `load=True`, eagerly parsing local model files. Certain commands skip context construction (`init`, `ui`) or set `load=False` (`clean`, `run`, `migrate`). +* **Lazy DB Connections:** `EngineAdapter` connections are inherently lazy; the database client is initialized via a connection factory and only connects when a query or cursor is requested. +* **State Sync Eagerness:** For `load=True` commands, the bootstrap process queries the state sync backend for the schema version and production environment, eagerly resolving the lazy database connection. +* **Implicit Telemetry:** An anonymized telemetry collector runs in a daemon thread on every invocation, flushing run analytics to Tobiko Cloud on command completion unless disabled via environment variables. +* **`sqlmesh format` Connection Requirement:** Because `format` runs with a fully loaded Context (`load=True`), it **always** attempts to establish a live connection to the state sync backend database. It will throw an exception and fail if the state database is unreachable. + +--- + +## Findings + +### 1. CLI Entry Points & Top-Level Click Groups + +The CLI entry points are defined under `[project.scripts]` in `pyproject.toml:112-116`: +* `sqlmesh = "sqlmesh.cli.main:cli"` +* `sqlmesh_cicd = "sqlmesh.cicd.bot:bot"` + +#### The `sqlmesh` Command Group +The main `sqlmesh` command-line interface is governed by a Click group defined at `sqlmesh/cli/main.py:55`. The entry point function is `cli(...)` at `sqlmesh/cli/main.py:94`. + +On invocation, `cli(...)` handles global environment setup and console configuration: +1. **Logging & Console Setup:** Configures Python logging and Rich-based console output formatting (`sqlmesh/cli/main.py:112-116`). +2. **Context Skip Audits:** If the subcommand is in `SKIP_CONTEXT_COMMANDS = ("init", "ui")` and exactly one path is provided, it immediately sets `ctx.obj` to the absolute path of the project and returns (`sqlmesh/cli/main.py:120-123`), bypassing configuration loading and Context creation entirely. +3. **Config Loading:** Discovers and loads configuration files for the project path(s) (`sqlmesh/cli/main.py:127`). +4. **Log Retention Cleanup:** Deletes older log files in the specified log directory that exceed the maximum log retention limit configured via `log_limit` (`sqlmesh/cli/main.py:128-130`). +5. **Context Construction:** Instantiates the global `Context` (`sqlmesh/cli/main.py:132-137`). + +#### The `sqlmesh_cicd` Command Group +The CI/CD bot is governed by a Click group defined at `sqlmesh/cicd/bot.py:14`. Its entry function is `bot(...)`. It receives project path and configuration arguments, sets up logging to stdout, and stores CLI options in `ctx.obj` (`sqlmesh/cicd/bot.py:25-28`) before delegating to subcommands like `github` (`sqlmesh/integrations/github/cicd/command.py:21`). + +### 2. Global CLI Options and Flags +Global flags are defined as decorators on the Click group inside `sqlmesh/cli/main.py:56-93`: +* `--paths` / `-p` (`sqlmesh/cli/options.py:8`): Multiple path strings pointing to SQLMesh projects. Defaults to `[os.getcwd()]`. +* `--config` (`sqlmesh/cli/options.py:16`): The name of the Python configuration object to load if `config.py` is used. +* `--gateway` (env: `SQLMESH_GATEWAY`): Specifies the gateway configuration to utilize. +* `--ignore-warnings` (env: `SQLMESH_IGNORE_WARNINGS`): Suppresses execution warnings. +* `--debug`: Enables detailed logging and displays full tracebacks on failures. +* `--log-to-stdout`: Prints application logs to stdout. +* `--log-file-dir`: Specifies the folder to write logs to. Defaults to `.logs/` inside the project. +* `--dotenv` (env: `SQLMESH_DOTENV_PATH`): Specifies a custom `.env` file to load. + +### 3. Project Configuration Loading & Precedence +SQLMesh configuration loading is orchestrated by `load_configs(...)` in `sqlmesh/core/config/loader.py:29`. The loading process obeys a strict sequence: + +1. **Dotenv Loading:** If a custom `.env` file is specified via the `--dotenv` CLI option, it is loaded. Otherwise, the loader searches for a `.env` file directly under each discovered absolute project path and loads it using `python-dotenv` with `override=True` (`sqlmesh/core/config/loader.py:44-51`). +2. **Personal Overrides:** The loader searches the user's personal settings directory at `~/.sqlmesh/` for configuration files matching `YAML_CONFIG_FILENAMES` (defined as `config.yml`, `config.yaml`, `sqlmesh.yml`, `sqlmesh.yaml` in `sqlmesh/core/config/common.py:15`). If a personal configuration exists, it parses it and loads environment variables declared under the `env_vars` key (`sqlmesh/core/config/loader.py:58-63`). +3. **Project Configuration Resolution:** For each project path, the configuration is resolved via `load_config_from_paths(...)` (`sqlmesh/core/config/loader.py:76`): + * **YAML Configs:** Scans project directories for files matching `ALL_CONFIG_FILENAMES` (`config.py`, `config.yml`, `config.yaml`, `sqlmesh.yml`, `sqlmesh.yaml`). If YAML configurations are found, they are parsed via `load_config_from_yaml` (`sqlmesh/core/config/loader.py:116`). + * **Python Module Configs:** If `config.py` exists, it is dynamically imported and evaluated via `load_config_from_python_module`, matching the specified configuration variable name (`sqlmesh/core/config/loader.py:121`). +4. **Environment Variables (`SQLMESH__`):** Environment variables are read via `load_config_from_env()` (`sqlmesh/core/config/loader.py:251`). Any environment variable prefixed with `SQLMESH__` (case-insensitive) is split by double underscores `__`. These segments are parsed and nested into a dictionary which overrides the loaded file-based configurations (e.g., `SQLMESH__DEFAULT_GATEWAY=dev` is parsed to `{"default_gateway": "dev"}`). + +### 4. Context Instantiation & The `load` Flag +When the `Context` is instantiated (`sqlmesh/core/context.py:376`), it executes the initialization of its parent class `GenericContext`. The bootstrap flow inside `GenericContext.__init__` performs the following work: + +1. **Configuration Merging:** Aggregates and loads configurations for all specified project paths. +2. **DAG & Cache Setup:** Creates the DAG holder, model caches, metadata indexes, and registers notification targets. +3. **Analytics Opt-out:** Checks if `disable_anonymized_analytics` is configured in the project settings, disabling the telemetry collector if set (`sqlmesh/core/context.py:423-424`). +4. **Gateway & Scheduler Mapping:** Identifies the selected gateway (defaulting to `default_gateway_name` or overriden via the `--gateway` CLI option) and builds the Scheduler configuration (`sqlmesh/core/context.py:427-430`). +5. **Project Parsing and State Check:** If `load=True`, the initialization calls `self.load()` (`sqlmesh/core/context.py:472`). + +#### The Eager Loading Flow (`self.load()`) +When `load` is `True` (the default), `self.load()` performs heavy, eager cross-cutting operations (`sqlmesh/core/context.py:629`): +1. **Local Project Load:** Invokes the registered loaders (e.g., `Loader` or custom converters) to parse and load all local Python/SQL models, audits, macros, and requirements from disk. +2. **State Sync Interaction:** It accesses `self.state_reader` on line 678 to retrieve the production environment state: + ```python + prod = self.state_reader.get_environment(c.PROD) + ``` + This retrieval triggers the `state_sync` property on `GenericContext` (`sqlmesh/core/context.py:609-620`), which boots the state sync backend. + +#### State Sync Boot & Version Validation +Retrieving `self.state_sync` initializes the state sync engine adapter and triggers version verification (`sqlmesh/core/context.py:612-615`): +```python +if self._state_sync.get_versions(validate=False).schema_version == 0: + self.console.log_status_update("Initializing new project state...") + self._state_sync.migrate() +self._state_sync.get_versions() +``` +* `_new_state_sync()` is called, which creates the state sync engine adapter. +* `get_versions(validate=False)` actively queries the database state schema's versions table. +* If the versions table does not exist or has a schema version of `0`, the state sync eagerly runs migrations (`migrate()`) to set up standard tables (`_snapshots`, `_environments`, `_versions`, `_intervals`). +* `get_versions()` is called a second time to validate that the remote schema version is fully compatible with the current running version of SQLMesh. + +Because `self.load()` queries the state sync backend, **any command initializing a Context with `load=True` eagerly resolves a database connection to query the remote state database on bootstrap.** + +### 5. CLI Command Lightweight Paths +Different commands use lightweight or bypassed paths depending on whether they require full project files or database access. + +| Command(s) | Category | Bypasses Context? | Bypasses Eager `load()`? | Description / Cites | +| :--- | :--- | :--- | :--- | :--- | +| `init`, `ui` | `SKIP_CONTEXT_COMMANDS` | **Yes** | **Yes** | Bypasses all config discovery and Context creation. `ctx.obj` is assigned the absolute directory path string (`sqlmesh/cli/main.py:43`). | +| `clean`, `create_external_models`, `destroy`, `environments`, `invalidate`, `janitor`, `migrate`, `rollback`, `run`, `table_name` | `SKIP_LOAD_COMMANDS` | **No** | **Yes** | `load` is set to `False` (`sqlmesh/cli/main.py:31-42`). Instantiates `Context` but skips running `self.load()` on initialization. Avoids loading model files and querying state sync during context creation (though the subcommands may query the state sync during execution). | +| `format`, `render`, `plan`, `diff`, `evaluate`, etc. | Standard Commands | **No** | **No** | Requires a fully loaded context. Eagerly reads all local model files, builds the model DAG, and connects to the state sync backend on boot. | + +### 6. Database and State Sync Connection Lifecycles +Connection handling in SQLMesh is designed to be highly lazy, preventing unnecessary connections until queries must run. However, the boot sequence forces this laziness to resolve immediately for standard commands. + +#### Lazy Connection Construction +1. **Engine Adapter Init:** The `EngineAdapter` is created by `create_engine_adapter()` inside `sqlmesh/core/config/connection.py:167`. Instead of receiving a live connection object, it receives `self._connection_factory_with_kwargs`—a callable factory (`sqlmesh/core/config/connection.py:155`). +2. **Connection Pool Wrapping:** Inside the `EngineAdapter` constructor (`sqlmesh/core/engine_adapter/base.py:126`), the connection factory is wrapped inside a lazy `ConnectionPool` subclass (`SingletonConnectionPool`, `ThreadLocalConnectionPool`, or `ThreadLocalSharedConnectionPool`) created via `create_connection_pool` (`sqlmesh/utils/connection_pool.py:342`). +3. **Deferred Execution:** The connection pool does not execute the connection factory callable until a cursor is explicitly requested (e.g. calling `self._connection_pool.get_cursor()` inside `cursor` property in `sqlmesh/core/engine_adapter/base.py:194`). + +#### State Sync Resolution +The state sync backend is initialized via `EngineAdapterStateSync` at `sqlmesh/core/state_sync/db/facade.py:74`. It holds its own lazy engine adapter. +However, because the `state_sync` property on `Context` eagerly executes `get_versions(validate=False)` on boot, it performs a SQL lookup (`sqlmesh/core/state_sync/db/version.py:61`): +```python +if not self.engine_adapter.table_exists(self.versions_table): + return no_version +``` +This forces `self.engine_adapter` to fetch a cursor, invoking the lazy connection factory. Consequently, **the state sync database connection is eagerly established during CLI boot for all standard commands.** + +### 7. Implicit External Touchpoints +On any standard CLI invocation, SQLMesh touches the local filesystem, environment, and remote services: +* **Filesystem (Project):** Reads `.env` files, parses model/audit/macro files, and writes log files under `.logs/`. Cleans up excess log files using `remove_excess_logs()` (`sqlmesh/__init__.py:171`). +* **Filesystem (Global):** Searches `~/.sqlmesh/` for personal configurations. +* **Implicit Telemetry:** On import of `sqlmesh.core.analytics`, a global telemetry `collector` is initialized. If `SQLMESH__DISABLE_ANONYMIZED_ANALYTICS` is not `"true"`, it launches a background thread with an `AsyncEventDispatcher` (`sqlmesh/core/analytics/__init__.py:29`). The command-run telemetry events are queued and, upon command completion (`atexit`), flushed via a network HTTP POST request to `https://analytics.tobikodata.com/v1/sqlmesh/` (`sqlmesh/core/analytics/dispatcher.py:19`). +* **Implicit Cloud Authentication:** No global, implicit authentication flows are run on general CLI boot. However, if the state sync or project config is configured with an interactive OAuth database (such as MotherDuck without a token, or Snowflake SAML), the lazy-connection resolution on boot will trigger browser-based interactive authentication prompts (`sqlmesh/core/config/connection.py:292`). +* **CI/CD Bot Network Touchpoints:** When invoking `sqlmesh_cicd github`, `GithubController.__init__` eagerly queries the GitHub API over the network via PyGithub to retrieve repository, issue, and pull request data, along with PR review approvals (`sqlmesh/integrations/github/cicd/controller.py:317-329`). + +--- + +## Behavior of `sqlmesh format` + +The table below breaks down what is performed by the `sqlmesh format` command, detailing the underlying mechanisms. + +| Step | Status | Mechanism / Cite | +| :--- | :--- | :--- | +| **Discover & Load Config** | **Always** | Parses `.env` files, searches `~/.sqlmesh/`, loads YAML/Python project configurations, and merges environment overrides (`sqlmesh/core/config/loader.py:29`). | +| **Telemetry Dispatch** | **Always** | Telemetry is initialized, queuing CLI command-run details and attempting to flush to `https://analytics.tobikodata.com/v1/` on completion (`sqlmesh/core/analytics/__init__.py`). | +| **Initialize Context** | **Always** | Instantiates a full `Context` object with `load=True` (`sqlmesh/cli/main.py:132`). | +| **Parse Local Models** | **Always** | Reads and parses all local SQL/Python model files and audits inside the project directory to construct the metadata DAG (`sqlmesh/core/context.py:629`). | +| **Database Connection** | **Always** | Connects to the **state sync database** backend to query `get_versions` and retrieve the `prod` environment configuration (`sqlmesh/core/context.py:609`, `sqlmesh/core/state_sync/db/version.py:61`). | +| **Interactive Login** | **Sometimes** | Prompts for browser authentication if the state sync backend utilizes an engine requiring interactive login (e.g. MotherDuck, BigQuery/Snowflake OAuth) and credentials are not pre-supplied (`sqlmesh/core/config/connection.py:292`). | +| **Metadata Migration** | **Sometimes** | Performs auto-migrations on the state database schema if the remote state schema version is `0` (`sqlmesh/core/context.py:613`). | +| **Data Warehouse Connection** | **Never** | Does not connect to or query the target data warehouse connection (if separate from the state connection) for executing model queries or performing DDL commands. | +| **Run Unit Tests** | **Never** | Bypasses testing hooks and does not evaluate any defined unit tests. | +| **Mutate Physical Tables** | **Never** | Does not perform DDL operations or alter physical user tables in the warehouse. | + +### Why `sqlmesh format` Requires a Live State Connection +A standard reader might assume that code formatting is a pure text-transformation utility that should operate offline on individual files. However, in SQLMesh, `sqlmesh format` does not bypass the standard context bootstrap. + +Because `format` requires the context to be initialized with `load=True`, the initialization flow calls `self.load()`. During this process, `self.load()` merges local files with environment metadata stored in the state sync database. To do this, it invokes `self.state_reader.get_environment(c.PROD)`. This forces SQLMesh to instantiate the state sync backend, verify metadata schema versions, and issue queries to the state database. + +As a result, **`sqlmesh format` cannot run in offline/isolated environments where the state sync database backend is unreachable.** If the database is down, offline, or behind a VPN, the command will crash on bootstrap prior to formatting any files. + +--- + +## Key References +* `pyproject.toml:112-116` — Configuration of standard CLI console script entry points. +* `sqlmesh/cli/main.py:94` — Main Click `cli(...)` entry point function. +* `sqlmesh/cli/main.py:31` & `43` — Definitions of bypassed subcommands (`SKIP_LOAD_COMMANDS` and `SKIP_CONTEXT_COMMANDS`). +* `sqlmesh/core/config/loader.py:29` — Main configuration loader `load_configs(...)`. +* `sqlmesh/core/context.py:376` — Context constructor `GenericContext.__init__` handling configuration mapping. +* `sqlmesh/core/context.py:609-620` — `state_sync` property logic executing eager version check and auto-migrations. +* `sqlmesh/core/config/scheduler.py:67` — Eager engine adapter construction and state sync instantiation. +* `sqlmesh/core/config/connection.py:167` — Dynamic creation of lazy engine adapters. +* `sqlmesh/utils/connection_pool.py:342` — Thread-safe connection pool delaying connection factory invocation. +* `sqlmesh/core/state_sync/db/version.py:61` — `get_versions()` executing version table queries on the database. +* `sqlmesh/core/analytics/__init__.py:29` — Background telemetry dispatcher initialization and flush routines. +* `sqlmesh/integrations/github/cicd/controller.py:286` — `GithubController` initializing and querying GitHub API resources. diff --git a/docs/2026-05-21_local-only-format/03_contributing.md b/docs/2026-05-21_local-only-format/03_contributing.md new file mode 100644 index 0000000000..cd444ed7cd --- /dev/null +++ b/docs/2026-05-21_local-only-format/03_contributing.md @@ -0,0 +1,230 @@ +# Contributing to SQLMesh + +This reference guide details the standards, local environments, testing frameworks, and submission workflows required when contributing to SQLMesh. Since you have already forked the repository and started a development branch (`fix/local-only-format`), this document focuses on what happens from this point forward. + +--- + +## 1. Core Contribution Principles + +SQLMesh is a Linux Foundation project and is licensed under the **Apache License 2.0**. Understanding community expectations and legal baselines helps avoid friction during your pull request (PR). + +### Community Policies +* **Code of Conduct (`CODE_OF_CONDUCT.md`)**: SQLMesh adheres to the [LF Projects Code of Conduct](https://lfprojects.org/policies/code-of-conduct/). All community participants are expected to uphold these standards. +* **General Expectations (`CONTRIBUTING.md`)**: Outlines project roles (Contributors, Maintainers, and the Technical Steering Committee), file licensing expectations, and general contribution workflow steps. + +### Legal Safeguards +* **Developer Certificate of Origin (DCO)**: All commits must be signed off to certify that you have the right to submit the code under the project's license. Your commit messages must include a `Signed-off-by` line. + * *How to commit with sign-off:* Use the `-s` flag when committing: + ```bash + git commit -s -m "Your commit message" + ``` + * *How to sign off existing commits:* + * Amending the latest commit: `git commit --amend -s` + * Rebasing multiple commits: `git rebase HEAD~N --signoff` +* **License Headers**: Any new file introduced to the codebase must begin with the SPDX license header: + ```python + # SPDX-License-Identifier: Apache-2.0 + ``` + +--- + +## 2. Developer Environment Setup + +The primary configuration for SQLMesh is defined in the root `pyproject.toml` and orchestrated using the project's `Makefile`. + +### Prerequisites +* **Python**: `>= 3.9` and `< 3.13` (Python 3.13+ is not yet supported). +* **Java**: OpenJDK `>= 11` (required for certain engine tests such as Spark). +* **Docker & Docker Compose V2**: Required for running containerized database engines. + +### Environment Setup Commands +Always work within a virtual environment. Use these commands (detailed in `docs/development.md` and `Makefile`): + +1. **Create and Activate Virtual Environment**: + ```bash + python -m venv .venv + source .venv/bin/activate # On Windows: .venv\Scripts\activate + ``` +2. **Install Development Dependencies**: + ```bash + make install-dev + ``` + This targets the `dev` extra in `pyproject.toml` (`[project.optional-dependencies]`) along with web, slack, dlt, and lsp extras: + ```bash + pip install -e ".[dev,web,slack,dlt,lsp]" ./examples/custom_materializations + ``` +3. **Setup Pre-Commit Hooks**: + ```bash + make install-pre-commit + ``` + This registers the pre-commit configuration in `.pre-commit-config.yaml` with git. + +--- + +## 3. Local Verification (Run Before You Push) + +To maintain a clean main branch, contributors must verify code style, static typing, and basic test suites locally before pushing their branch. + +### Linting and Formatting +SQLMesh uses **Ruff** for linting/formatting and **MyPy** for static type checks, automated via `pre-commit` hooks. + +* **Style Verification**: Run the style command before every push: + ```bash + make style + ``` + This executes `pre-commit run --all-files`, which runs: + * `ruff check --force-exclude --fix --ignore E721 --ignore E741` on python files. + * `ruff format --force-exclude --line-length 100` for PEP 8 compliance. + * `mypy` for static typing (configured in `pyproject.toml` to enforce `disallow_untyped_defs = true` on the core library, with relaxed rules for tests and migrations). + * `valid migrations` using `tooling/validating_migration_numbers.sh` to ensure migration sequences are consecutive and have no gaps or overlaps. +* **Python-Only Style**: If you only touched python files and want to skip node/frontend linters, run: + ```bash + make py-style + ``` + +--- + +## 4. Testing Framework and Conventions + +The SQLMesh test suite lives in the `tests/` directory. Tests are categorised and run using `pytest` markers (defined in `pyproject.toml`). + +### Organization of Tests +* **Unit & Integration Tests**: Organized within subdirectories under `tests/` matching core modules (e.g., `tests/core/`, `tests/dbt/`, `tests/web/`, etc.). +* **Engine-Specific Tests**: Integration tests that verify SQLMesh's behavior against physical databases. + +### Pytest Markers +Pytest markers control execution speed and environment requirements. Key markers include: +* `fast`: Tests that execute rapidly without database interactions (default when no markers are supplied). +* `slow`: Tests involving local DB file/memory interactions (such as DuckDB). +* `docker`: Tests that require running localized Docker containers. +* `remote`: Tests that interact with cloud database endpoints. +* `isolated` / `dialect_isolated` / `registry_isolation`: Tests that must be run sequentially or separately to avoid shared state contamination. + +### Running Tests Locally +Depending on the scope of your changes, use the corresponding Makefile target: + +* **Fast Suite**: Run unit and quick tests: + ```bash + make fast-test + ``` + *Maps to: `pytest -n auto -m "fast and not cicdonly" ...` followed by isolated test suites.* +* **Slow Suite**: Run all local unit and integration tests (including slow ones): + ```bash + make slow-test + ``` + *Maps to: `pytest -n auto -m "(fast or slow) and not cicdonly" ...` followed by isolated test suites.* +* **Doctests**: Verify examples embedded in docstrings: + ```bash + make doc-test + ``` +* **Engine-Specific Unit Tests**: + * **DuckDB**: Run unit tests targeting DuckDB adapter: + ```bash + make duckdb-test + ``` + * **Dockerized Engines** (Postgres, MySQL, MSSQL, ClickHouse, Trino, Spark, RisingWave): + To run these, you must bring the engine up in Docker first, run the tests, and optionally tear down: + ```bash + # Bring up Postgres in Docker and run tests + make engine-postgres-up + make postgres-test + make engine-postgres-down + ``` + *Note: Individual engine targets automatically execute standard Docker Compose files located under `tests/core/engine_adapter/integration/docker/`.* + * **Cloud Engines** (Snowflake, BigQuery, Databricks, Redshift, Athena, Fabric, GCP-Postgres): + These require passing corresponding credentials via environment variables (e.g., `SNOWFLAKE_ACCOUNT`, `BIGQUERY_KEYFILE`, etc.) and executing the respective targets (e.g., `make snowflake-test`). + +--- + +## 5. Agent & AI-Assisted Workflow + +SQLMesh contains explicit expectations for contributors using AI assistants or agents (detailed in `CLAUDE.md`). The project relies on a structured, Test-Driven Development (TDD) cycle. + +### The Agent Loop +When implementing features or bug fixes, you should navigate the code in a systematic loop: +1. **Understand**: Explore the codebase, review existing design patterns, and read relevant GitHub issues. +2. **TDD (Failing Test First)**: *Always begin by writing a failing test (or tests)* that reproduces the bug or asserts the expected behavior of the new feature before editing the source files. +3. **Implement**: Make the smallest surgical change that solves the issue. Avoid speculative coding. +4. **Code Review**: Hand off your implementation to an automated review or use a code-reviewer agent to identify edge cases, test coverage gaps, and style compliance. +5. **Iterate**: Fix any review items, verify the tests pass, and proceed. +6. **Document**: Write or update user-facing documentation for any user-visible behaviors. + +--- + +## 6. Commit and PR Workflow + +### Commit Conventions +Based on the repository's git history, commits follow a semantic naming convention. Maintainers merge PRs using squash-commits, so clean commit messages are highly valued. +* **Prefixes**: Use semantic prefixes to describe the nature of your change: + * `fix:` or `Fix:` for bug fixes. + * `feat:` or `Feat:` for new features. + * `chore:` or `Chore:` for dependency bumps, configuration, or structural updates. + * `docs:` or `(docs):` for documentation edits. + * Add a `!` prefix (e.g., `Chore!:`) for breaking changes. +* **Engine Scopes**: If the change is engine-specific, denote the engine in parentheses: + * `Fix (databricks): use shared connection pool...` +* **PR Reference**: Keep commit titles clean. When merging, the PR number is appended (e.g., `(#5801)`). +* *Mandatory Requirement:* Every commit must contain the DCO sign-off (`Signed-off-by`). + +### Submission Process +1. **Branch Target**: All pull requests must be opened against the `main` branch of `sqlmesh/sqlmesh`. +2. **PR Template**: Fill out `.github/pull_request_template.md` completely. It requires: + * **Description**: A clear summary of the changes. + * **Test Plan**: Specific steps on how you tested the changes. + * **Checklist**: Check boxes certifying you ran `make style`, added tests, verified `make fast-test`, and signed off all commits. +3. **Reviewers & CODEOWNERS**: There is no explicit `CODEOWNERS` file in the repository. Instead, community maintainers (such as Alexander Butler `z3z1ma`, Toby Mao `tobymao`, or Yuki Kakegawa `StuffbyYuki`) manually review and assign reviewers. + +### Required CI Checks (GitHub Actions) +The PR workflow (`.github/workflows/pr.yaml`) runs an extensive validation suite: +* **`doc-tests`**: Runs `make doc-test` to verify docstring examples. +* **`style-and-cicd-tests`**: Runs `make py-style`, `make benchmark-ci`, and `make cicd-test` across a Python version matrix (`3.9`, `3.10`, `3.11`, `3.12`, `3.13`) on Ubuntu. +* **`cicd-tests-windows`**: Runs `make fast-test` on Windows. +* **`migration-test`**: Validates migrations by running integration tests under database schema upgrades using `./.github/scripts/test_migration.sh`. +* **`ui-test`**: Runs frontend unit and Playwright e2e tests for the web UI. +* **`engine-tests-docker`**: Executes test suites for containerized engines (`duckdb`, `postgres`, `mysql`, `mssql`, `trino`, `spark`, `clickhouse`, `risingwave`). +* **`test-dbt-versions`**: Validates compatibility across multiple dbt versions (`1.3` through `1.10`) using `make dbt-fast-test`. + +--- + +## 7. Documentation Contributions + +If your changes alter user-visible APIs, configurations, or behaviors, you must update the documentation (written in connected prose following `docs/HOWTO.md`). + +### Structure and Location +* Documentation markdown files live in the `docs/` folder. +* Configuration guides are located under `docs/guides/configuration.md`. +* Integration-specific documents are located under `docs/integrations/`. + +### Previewing Documentation +1. **Install Docs Dependencies**: + ```bash + make install-doc + ``` + *If version conflicts occur, run the fallback command:* + ```bash + pip install mkdocs mkdocs-include-markdown-plugin mkdocs-material mkdocs-material-extensions mkdocs-glightbox pdoc + ``` +2. **Serve Locally**: + ```bash + make docs-serve + ``` + This launches a local MkDocs development server at `http://127.0.0.1:8000/` that hot-reloads as you edit markdown files. + +### Style and Tone Guidelines +* **Cognitive Load**: SQLMesh is powerful and complex; focus on minimizing cognitive load for readers. +* **Instructional Voice**: Use the second-person voice for instructions ("Add an audit...", "You can partition..."). +* **Narrative Voice**: Use the first-person plural for walk-throughs or narratives ("First, we create a new environment..."). +* **Tone**: Prefer active voice over passive voice, and keep sentences short. +* **Review Flow**: For larger doc additions, the Technical Writer/Editor (Trey) may perform a full review and editing pass by opening a PR directly *against your branch* before team approval. + +--- + +## 8. Summary Checklist for Contributors + +Before pushing and submitting your PR, ensure you can tick off all items on this list: +- [ ] No new files are missing the Apache 2.0 license header. +- [ ] All commits are signed off (`git commit -s`). +- [ ] Code is formatted and typed (`make style`). +- [ ] No gaps or overlaps exist in any newly added migration numbers. +- [ ] The local fast test suite passes (`make fast-test`). +- [ ] The `.github/pull_request_template.md` is fully completed. diff --git a/docs/2026-05-21_local-only-format/spec.md b/docs/2026-05-21_local-only-format/spec.md new file mode 100644 index 0000000000..32ef466e4e --- /dev/null +++ b/docs/2026-05-21_local-only-format/spec.md @@ -0,0 +1,79 @@ +# Spec: local-only `format` and `lint` + +## Context + +`sqlmesh format` is a pure text transformation — it parses each `.sql` model and audit file with SQLGlot, optionally rewrites casts or transpiles the dialect, and writes the pretty-printed result back to disk. `sqlmesh lint` is the same shape: it walks loaded model ASTs through linter rules and reports violations. Neither needs a database. But today, both run an authenticated state-sync query before doing any of their actual work. + +The reason is wiring, not intent. Neither command is listed in `SKIP_LOAD_COMMANDS` or `SKIP_CONTEXT_COMMANDS` (`sqlmesh/cli/main.py:31,43`), so the CLI builds a `Context` with `load=True`. `Context.load()` then runs a state-merge block (`sqlmesh/core/context.py:678-697`) that calls `self.state_reader.get_environment(c.PROD)` to pull remote snapshots and environment statements into the local model registry. Touching `state_reader` resolves the lazy `state_sync` property, which connects to the state database and runs `get_versions` (`sqlmesh/core/context.py:609-620`). If state lives in Postgres, Snowflake, BigQuery, or any other shared backend, that's an authenticated network call. In CI, where credentials are typically not provisioned for formatting or lint jobs, this turns a purely local check into a connection-required step. + +Both commands' actual code paths are clean: `Context.format()` (`context.py:1220`) uses `self._models`, `self._audits`, `target._path`, `target.formatting`, `target.dialect`, and `self.config_for_node()` — nothing else. `Context.lint_models()` (`context.py:3210`) iterates over `self._models.values()` and runs each through `Linter.lint_model`; the four places the built-in rules touch `self.context` (`linter/rules/builtin.py:140,161,185,247`) all read from local in-memory data (`models_with_tests`, `get_model`, `extract_references_from_query`, `self.context.path`). Neither command, nor its helpers, ever references `state_sync`, `state_reader`, `engine_adapter`, or `snapshots`. The state connection both commands open today is dead weight from `Context.load()`'s remote-snapshot merge. + +## Goals + +- `sqlmesh format` and `sqlmesh lint` run end-to-end without opening any network connection to the state-sync backend, any other database, or any external service, and without requiring any authentication. +- Neither command requires state credentials, warehouse credentials, or any other auth material to be present in the environment or config. A CI job with zero secrets provisioned can run both successfully against a real project. +- The fix is one focused mechanism — a single new `Context` parameter and a single new CLI tuple — not a per-command refactor. +- Existing behavior of every other CLI command is unchanged. No regression in `plan`, `run`, `render`, `diff`, etc. +- The new behavior is covered by tests that fail without the fix (e.g., constructing a `Context` against a project whose state connection points at an unreachable database, and asserting `format` and `lint` still succeed). + +## Non-goals + +- Making `dag` local-only. It's a plausible candidate but its `--select-model` path constructs a `Selector` (`context.py:2947`) that evaluates `self.state_reader` at construction time, even though `Selector.expand_model_selections` never uses it. Fixing that cleanly would require deferring state-reader resolution inside `Selector` and reviewing every other call site of `_new_selector`. We're not doing it here. +- Making `render`, `rewrite`, `create_test`, `test`, or any other CLI command local-only. They have real state or warehouse dependencies in their actual code paths, not just in `Context.load()`. +- Refactoring `Context.load()` beyond gating the existing state-merge block. The block's logic is unchanged — only whether it runs. +- Suppressing anonymized analytics or any other implicit network activity on CLI invocation. That's governed by the existing `disable_anonymized_analytics` setting and is orthogonal to this change. +- Adding an env-var or config knob to restore the old behavior for `format`/`lint`. The spec commits to the new behavior, consistent with how `SKIP_LOAD_COMMANDS` and `SKIP_CONTEXT_COMMANDS` are handled today (`cli/main.py:31,43`). +- Changing the public Python API beyond the one new `Context(load_state=...)` parameter. No new method, no new module, no rename. + +## Key discoveries + +- `format` and `lint` are wired through `Context` with `load=True`. The CLI's two existing escape tuples don't cover them: neither command appears in `SKIP_LOAD_COMMANDS` or `SKIP_CONTEXT_COMMANDS` (`sqlmesh/cli/main.py:31,43`). +- The state-sync connection both commands open is opened exclusively inside `Context.load()`'s remote-snapshot merge block (`sqlmesh/core/context.py:678-697`). Two calls touch state: `self.state_reader.get_environment(c.PROD)` at line 678, and the snapshot/environment-statement merge gated by `if any(self._projects):` at lines 681-697. +- Reading `self.state_reader` is the trigger. The property returns `self.state_sync` (`context.py:621`), which lazily instantiates the state backend, runs `get_versions(validate=False)`, optionally migrates, and runs `get_versions()` again (`context.py:609-619`). All of that is one authenticated round trip to the configured state database. +- The engine adapter is genuinely lazy. `create_engine_adapter()` wraps the connection in a `ConnectionPool` that doesn't invoke its factory until a cursor is requested (`sqlmesh/utils/connection_pool.py:342`, `sqlmesh/core/engine_adapter/base.py:194`). Neither `Context.format()` nor `Context.lint_models()` ever calls a method that requests a cursor, so no warehouse connection is opened by these commands' own code paths — only by `load()`'s state-merge block. +- `Context.format()` (`context.py:1220`) uses `self._models`, `self._audits`, `target._path`, `target.formatting`, `target.dialect`, and `self.config_for_node()`. Verified by reading the method end to end. +- `Context.lint_models()` (`context.py:3210`) iterates `self._models.values()` and delegates to per-project `Linter.lint_model`. The four places the built-in rules access `self.context` (`linter/rules/builtin.py:140,161,185,247`) are all reads of local in-memory data: `models_with_tests`, `get_model`, `extract_references_from_query`, and `self.context.path`. No rule references `state_sync`, `state_reader`, `engine_adapter`, or `snapshots` (verified by `rg` across `sqlmesh/core/linter/`). +- `update_model_schemas` in the `update_schemas=True` branch of `load()` (`context.py:723`) is local-only — it constructs a `MappingSchema` against `self._models` and uses `OptimizedQueryCache` on the local filesystem. No engine adapter involvement. +- The repo already has the right shape for the change. `cli/main.py` has two precedents — `SKIP_LOAD_COMMANDS` and `SKIP_CONTEXT_COMMANDS` — for marking subcommands that need lighter Context construction, and both flow through to `Context.__init__` via plain boolean parameters. Adding a third tuple with the same shape matches the existing pattern. + +## Proposed approach + +Add a third boolean parameter, `load_state: bool = True`, to `GenericContext.__init__`. Store it on the instance. In `Context.load()`, guard the existing remote-snapshot merge block (`context.py:678-697`) on this flag — when `load_state` is `False`, skip the two `self.state_reader.get_environment(c.PROD)` calls and the snapshot/environment-statement merge between them. The flag is meaningful only when `load=True`; when `load=False`, `load()` isn't called and the flag is inert. Document that in the docstring. + +In `sqlmesh/cli/main.py`, add a new constant alongside the existing two: + +```python +LOCAL_ONLY_COMMANDS = ("format", "lint") +``` + +In `cli(...)`, after the existing `SKIP_CONTEXT_COMMANDS` and `SKIP_LOAD_COMMANDS` checks, set `load_state = False` when `ctx.invoked_subcommand in LOCAL_ONLY_COMMANDS`. Pass it through to the `Context(...)` constructor. The existing `load=True` path is unchanged for these commands — models are still parsed, schemas still updated, audits still loaded — only the state-merge step is skipped. + +No other code changes. `Context.format()`, `Context.lint_models()`, the linter rules, and `update_model_schemas` are already local-only; this spec just removes the unnecessary state-merge that `load()` runs before they get a chance to execute. + +Tests: add a regression test that constructs a `Context` against a project whose state connection points at an unreachable database (e.g., a Postgres `ConnectionConfig` pointing at `localhost:1` or a similar guaranteed-fail target), with `load_state=False`, and asserts that `format` and `lint` complete successfully. Also add a CLI-level test that invokes `sqlmesh format` and `sqlmesh lint` against the same project and asserts neither opens a state connection — likely by patching `EngineAdapterStateSync.get_versions` and asserting it's never called. + +## Key decisions & tradeoffs + +- **`load_state: bool = True` over an enum.** Matches the existing `load: bool = True` shape on `Context`. The nonsensical combination `load=False, load_state=True` is inert (load() never runs), so an enum would add abstraction without preventing a real bug. +- **New `LOCAL_ONLY_COMMANDS` tuple over reusing `SKIP_LOAD_COMMANDS`.** `format` and `lint` *do* need models loaded; they just don't need state merged in. Reusing `SKIP_LOAD_COMMANDS` would require a parallel "models-only loader" and duplicate `load()`'s parsing logic. A third tuple is one line and reads exactly as the existing pattern. +- **Gate the state-merge block, don't extract it.** Pulling lines 678-697 of `context.py` into a private method would be cleaner in isolation but would touch every caller of `load()` and obscure the diff. A single `if self._load_state and any(self._projects):` guard keeps the diff small and the change reviewable. +- **No escape hatch.** Consistent with how `SKIP_LOAD_COMMANDS` and `SKIP_CONTEXT_COMMANDS` are handled. The state-merge work is unused by `format`/`lint`, so there's no scenario where restoring it would be correct behavior for these commands. +- **`Context(load_state=...)` is part of the public API.** It's a Python-level parameter on `GenericContext`, so programmatic users of SQLMesh can construct a local-only Context too. Cost is zero (one kwarg, one docstring line) and the symmetry with `load` is worth it. +- **`load_state=False` is set by the CLI, not inferred at runtime.** We could try to detect at runtime whether a command will touch state, but that's a much larger change. The CLI knows statically which commands are local-only; a constant tuple is the right place for that knowledge. + +## Risks / unknowns + +- **Test fixtures that assume state is queried during load.** Existing tests that build a `Context` with `load=True` may rely on remote snapshots being merged in. Our change defaults `load_state=True`, so these tests should be unaffected — but if any test runs `format` or `lint` through the CLI runner against a project that depends on cross-project snapshot merging for the formatted/linted models to resolve, that test may need a fixture adjustment. Likely none exist, but worth confirming during implementation. +- **`update_model_schemas` cache state.** `load(update_schemas=True)` runs after the state-merge block. When the block is skipped, the local `MappingSchema` won't contain remote snapshot columns. For `format` and `lint` this is fine — neither needs resolved column types from remote projects — but if a linter rule is ever added later that relies on cross-project column resolution, it would silently produce different results under `load_state=False`. Note in the linter rule contract, but no code change needed today. +- **CLI test isolation.** Asserting "no state connection was opened" is easier to assert by patching than by mocking the network. The cleanest seam is `EngineAdapterStateSync.get_versions` — patching it to raise should be enough; we then assert `format`/`lint` succeed and the patched method is never called. If that seam turns out to be wrong (e.g., a different method gets called first), we'll adjust during implementation. +- **Multi-project monorepos.** The state-merge block's purpose is to import remote snapshots for *other* projects when the current load covers only a subset (`context.py:681-697`, gated on `any(self._projects)`). For `format`/`lint`, this means a model that depends on a model defined in a sibling project not currently being loaded won't have its upstream resolved. That's already the behavior for any model `format`/`lint` touches — they operate per-file, not on rendered downstream queries — so there should be no observable difference. Flagging in case a reviewer raises it. + +## Dependencies + +- Research docs: + - `docs/2026-05-21_local-only-format/01_format_command.md` — how `format` works end to end. + - `docs/2026-05-21_local-only-format/02_cli_bootstrap.md` — what runs on any CLI invocation, where state is touched. + - `docs/2026-05-21_local-only-format/03_contributing.md` — DCO sign-off, `make style`, `make fast-test`, PR template, CI matrix. + - `docs/2026-05-21_local-only-format/spec_notes.md` — discovery and planning context that didn't fit in the spec: full CLI command audit, why `dag` was dropped, mechanism alternatives considered, verification methodology. +- No external library or service dependencies. Pure internal change. +- No other in-flight specs or branches to coordinate with — the working branch `fix/local-only-format` is fresh off `main`. diff --git a/docs/2026-05-21_local-only-format/spec_notes.md b/docs/2026-05-21_local-only-format/spec_notes.md new file mode 100644 index 0000000000..e1d51dedff --- /dev/null +++ b/docs/2026-05-21_local-only-format/spec_notes.md @@ -0,0 +1,101 @@ +# Spec notes + +Context captured during the discovery and planning conversation that didn't make it into `spec.md`. The spec is the source of truth for what we're building; these are the things a reader would benefit from if the conversation were lost — the audit table, the design alternatives we rejected, the latent issues we noticed, and the verification methodology that backed the spec's confidence. + +## CLI command survey + +When the user asked "are there other commands that should be local-only?", I worked through every `@cli.command` in `sqlmesh/cli/main.py` and classified each by whether it touches state and whether it touches the warehouse. The table below captures the full audit. Only `format` and `lint` made it into this spec; everything else is either already handled (existing skip tuples), genuinely needs state/warehouse, or has an awkward edge case that ruled it out. + +| Command | Current Context mode | Touches state? | Touches warehouse? | Local-only candidate? | +|---|---|---|---|---| +| `init` | `SKIP_CONTEXT_COMMANDS` | No | No | Already covered | +| `ui` | `SKIP_CONTEXT_COMMANDS` | No | No | Already covered | +| `clean` | `SKIP_LOAD_COMMANDS` | No (clears caches) | No | Already effectively local | +| `create_external_models` | `SKIP_LOAD_COMMANDS` | No | **Yes** (queries DB for schemas) | No | +| `destroy` | `SKIP_LOAD_COMMANDS` | Yes | Yes | No | +| `environments` | `SKIP_LOAD_COMMANDS` | Yes | No | No | +| `invalidate` | `SKIP_LOAD_COMMANDS` | Yes | No | No | +| `janitor` | `SKIP_LOAD_COMMANDS` | Yes | Yes | No | +| `migrate` | `SKIP_LOAD_COMMANDS` | Yes | No | No | +| `rollback` | `SKIP_LOAD_COMMANDS` | Yes | No | No | +| `run` | `SKIP_LOAD_COMMANDS` | Yes | Yes | No | +| `table_name` | `SKIP_LOAD_COMMANDS` | Yes (looks up env) | No | No | +| **`format`** | full load | No | No | **Yes** — this spec | +| **`lint`** | full load | No | Possibly via rules — verified no | **Yes** — this spec | +| `dag` | full load | No without `--select-model`; yes with | No | Edge case — see below | +| `render` | full load | Yes (`self.snapshots`, deployability index) | Yes (seed engine adapter) | No | +| `evaluate` | full load | Yes | Yes | No | +| `diff` | full load | Yes (env diff) | No | No | +| `plan` | full load | Yes | Yes | No | +| `create_test` | full load | No | Yes (executes input queries) | No | +| `test` | full load | No | Local test engine | No (uses warehouse-like adapter) | +| `audit` | full load | Yes | Yes | No | +| `check_intervals` | full load | Yes | No | No | +| `fetchdf` | full load | Yes | Yes | No | +| `info` | full load | Optional via `--skip-connection` | Optional | Already has skip | +| `table_diff` | full load | Yes | Yes | No | +| `rewrite` | full load | Unverified — not investigated deeply | Unverified | Out of scope | +| `dlt_refresh` | full load | No (writes models from DLT) | No | Plausible but not investigated | + +`rewrite` and `dlt_refresh` are the two commands where the audit is shallow. They were filed under "out of scope" rather than "verified not a candidate". If someone picks up local-only work later, those are the first ones to check. + +## Why `dag` was dropped + +`dag` looked like a clean third candidate at first. `Context.render_dag` (`context.py:2170`) just writes HTML of `self.get_dag(select_models)`, and the linter rules and engine adapter aren't referenced anywhere in that path. But tracing through `Context.get_dag` (`context.py:2121`) surfaced a latent issue: + +When `select_models` is provided, `get_dag` calls `self._new_selector().expand_model_selections(...)`. `Context._new_selector()` (`context.py:2947`) constructs a `Selector` with `self.state_reader` as its first argument. Reading `self.state_reader` evaluates the lazy property (`context.py:621`), which evaluates `self.state_sync` (`context.py:609-619`), which boots the state backend, runs `get_versions(validate=False)`, optionally migrates, and runs `get_versions()` again. **The act of constructing the Selector opens a state connection**, even though `Selector.expand_model_selections` (`selector.py:191+`) never actually uses the state reader — only `_load_env_models` (`selector.py:153`) does, and `get_dag` never reaches that path. + +Three options were considered: + +1. **Drop `dag`** — what we chose. Tightest spec. `dag` is lower-traffic than `format`/`lint` in CI. +2. **Include `dag` partially** — fix the no-`--select-model` path only, leave `dag --select-model X` broken offline. Would be a subtle, surprising behavior split for the same command. Rejected. +3. **Include `dag` fully** — also defer `state_reader` evaluation inside `Selector`. Would touch every other call site of `_new_selector` (used by `plan`, `diff`, `run`, etc., via state methods). Wider blast radius, departs from "minimal change". Rejected. + +The Selector wart is real but out of scope for this PR. + +## Mechanism alternatives considered + +Before landing on the `load_state` kwarg, three mechanisms were on the table: + +1. **`load_state` kwarg on Context, new `LOCAL_ONLY_COMMANDS` tuple in CLI** — chosen. Mirrors the existing `load: bool = True` shape and the existing `SKIP_*_COMMANDS` tuples. One kwarg, one tuple, two if-guards. +2. **Extract the state-merge block into a private `_merge_remote_state()` method, call selectively** — cleaner separation of concerns inside `Context`, but `load()` is called from `Context.__init__`, so the CLI can't directly influence whether the new method runs. We'd still need a kwarg on `__init__`, which means the same surface area plus an extracted method. No net win. +3. **Add `format`/`lint` to `SKIP_LOAD_COMMANDS` and have those commands load models themselves** — reuses existing infrastructure but requires a new `_load_models_only()` method that duplicates much of `load()`. Bigger surface area for bugs, and doesn't match how the other `SKIP_LOAD` commands work (they genuinely don't need models). + +## Naming alternatives for `load_state` + +The chosen name `load_state` mirrors the existing `load: bool = True`. Other names considered: + +- `state` — too vague; conflicts with `state_sync` / `state_reader` properties and the `state` subcommand group. +- `require_state` — frames it as a requirement rather than an action; less symmetric with `load`. +- `offline` — describes user intent ("offline mode") but inverts polarity vs every other boolean in this area, and is broader than what we're doing (we still hit the filesystem and load models). + +The user also asked whether an enum (`LoadMode.NONE` / `LoadMode.LOCAL` / `LoadMode.FULL`) would be better, since `load=False, load_state=True` is a nonsensical combination. The answer was no: that combination is inert (when `load=False`, `load()` is never called, so `load_state` has no effect), and the repo's existing pattern in this area is plain booleans (`load=True`, the two `SKIP_*` tuples). An enum would stand out as a new abstraction without preventing a real bug. + +## Verification methodology + +To verify a command is local-only after the fix, the procedure is: + +1. Start from the `@cli.command` in `sqlmesh/cli/main.py`. Identify the `Context` method(s) it calls. +2. Read those methods in full. Recursively follow every function/method call into helpers. +3. Look for any read of: `self.state_sync`, `self.state_reader`, `self._state_sync`, `self.engine_adapter`, `self._get_engine_adapter`, `self.connection_config`, `self.snapshots`. These are the seams that boot a connection. Also look for direct calls to `.get_environment`, `.get_versions`, `.get_snapshots`, or other state-sync API methods on any object. +4. Pay attention to **property side effects**, not just method calls. `self.state_reader` is a property that boots the state backend on access — just *passing* it as an argument to another constructor triggers the connection (this is how `dag --select-model` gets caught). +5. For linter rules specifically, check what's reachable through `self.context` from rule classes. The `Rule` base class (`linter/rule.py:75`) stores `self.context = context`, so any attribute read on `self.context` from inside a rule is a code path to audit. +6. Cross-check with existing tests: do they construct a full `Context` with a live DB, or do they get away with a DuckDB stub? Tests that pass without real credentials are evidence about what's actually required at runtime — though note that DuckDB-backed state still hides the boot sequence behind a local file, so test passage isn't proof of "no state access". + +A subtle trap: when a researcher reports "command X is NOT CLEAN", check whether they're describing current behavior (before the fix) or post-fix behavior. The researcher's first verification of `lint`/`dag` reported them as "NOT CLEAN" because today they're wired through `load=True` and thus hit state during `Context.load()` — true but not the question being asked. The actual question is whether the command's *own* code path (everything downstream of `load()`) reaches for state. For `lint`, it doesn't. For `dag`, it does (via Selector). That distinction is what determines whether the spec's mechanism is sufficient. + +## Test seam + +The spec proposes patching `EngineAdapterStateSync.get_versions` as the assertion seam. The reasoning: `get_versions` is the first state-touching method called when the lazy `state_sync` property resolves (`context.py:613`). Patching it to raise turns any accidental state access into a loud failure that the test can detect. If the implementer finds that another method gets called first in practice (e.g., during state-sync construction itself), they should adjust to patch whichever method actually fires first — the principle is "any state access raises", not specifically "`get_versions` raises". + +A lighter alternative: configure the project's state connection to point at an unreachable host (`localhost:1`, or a Postgres config with an obviously-wrong port). This exercises the real connection code and produces a meaningful error if the fix regresses. Both flavors of test have value; the spec leaves room for the implementer to pick. + +## `update_model_schemas` is local + +The `update_schemas=True` branch of `load()` (`context.py:723`) calls `update_model_schemas(self.dag, models=self._models, cache_dir=self.cache_dir)`. This was specifically verified: `sqlmesh/core/model/schema.py:22` constructs a `MappingSchema` against the local models dict and uses `OptimizedQueryCache` (a local-filesystem cache). It never references `self.engine_adapter` or any state API. So `format` and `lint` get fully-resolved local schemas without paying for any connection. + +If anyone later writes a linter rule that needs cross-project column resolution (a model imported from a sibling project that's only present in remote state), that rule would silently produce different results under `load_state=False`. Not a concern today — no such rule exists — but worth flagging if rule authors ever start reaching for remote schemas. + +## What the researcher got right and where it misled + +The research docs in this folder (`01_format_command.md`, `02_cli_bootstrap.md`, `03_contributing.md`) are accurate as references for the current behavior of the codebase. The first round of `lint`/`dag` verification was *also* accurate as a description of current behavior, but its verdict ("NOT CLEAN") was framed against today's wiring rather than against the proposed fix. When delegating verification work to a researcher subagent in this kind of "are these commands ready for X change?" conversation, it's worth explicitly framing the question as "given that we'll make change Y, would the command still touch state?" rather than just "does the command touch state?" From 15bf65bb8a8a061ed6387fe3472eda48d51db2be Mon Sep 17 00:00:00 2001 From: Joe Hartshorn <8881940+j-hartshorn@users.noreply.github.com> Date: Thu, 21 May 2026 10:28:22 +0100 Subject: [PATCH 02/10] docs: plan local-only format and lint Break the spec into two TDD tasks for a single PR: Task 1 - add `load_state: bool = True` to `GenericContext` and gate the state-merge block in `Context.load()` on it, with fast unit tests in `tests/core/test_format.py` and `tests/core/linter/test_builtin.py` that construct a Context against an unreachable Postgres state config. Task 2 - introduce `LOCAL_ONLY_COMMANDS = ("format", "lint")` in `sqlmesh/cli/main.py` and pass `load_state=False` for those commands, with three slow CLI tests in `tests/cli/test_cli.py` covering format, lint, and a guard-rail confirming `plan` still loads state. Plan was reviewer-checked; line citations corrected, test specifications tightened, commit-message examples aligned with repo conventions. Coding-Agent: pi Model: anthropic/claude-opus-4-7 Signed-off-by: Joe Hartshorn <8881940+j-hartshorn@users.noreply.github.com> --- docs/2026-05-21_local-only-format/plan.md | 148 ++++++++++++++++++++++ 1 file changed, 148 insertions(+) create mode 100644 docs/2026-05-21_local-only-format/plan.md diff --git a/docs/2026-05-21_local-only-format/plan.md b/docs/2026-05-21_local-only-format/plan.md new file mode 100644 index 0000000000..3f689e3de3 --- /dev/null +++ b/docs/2026-05-21_local-only-format/plan.md @@ -0,0 +1,148 @@ +# Plan: local-only `format` and `lint` + +## Goal + +Make `sqlmesh format` and `sqlmesh lint` runnable with no state-sync credentials and no reachable database, by adding a `load_state: bool = True` parameter to `GenericContext` that gates the remote-snapshot merge block in `Context.load()`, and a `LOCAL_ONLY_COMMANDS = ("format", "lint")` tuple in `cli/main.py` that flips it to `False` for those two commands. Everything else stays as it is. + +## Overall validation + +The PR is done when all of these are true: + +- A `Context` constructed against a project whose state connection points at an unreachable host completes both `context.format()` and `context.lint_models()` without raising when `load_state=False` is passed. +- `sqlmesh format` and `sqlmesh lint` invoked through the CLI runner against a project whose `config.yaml` declares an unreachable state connection complete with `exit_code == 0`. `EngineAdapterStateSync.get_versions` is never called during the run (verified by `mocker.patch` with `assert_not_called()`). +- A guard-rail CLI test confirms `sqlmesh plan` against the same project *does* call `EngineAdapterStateSync.get_versions` — the new tuple didn't accidentally cover commands it shouldn't. +- `make py-style` is clean. Ruff, mypy, and the migration-sequence check all pass. +- `make fast-test` is green (covers the new Context-level tests). +- The targeted CLI test passes when run directly (`pytest tests/cli/test_cli.py::test_format_runs_without_state -v` and the same for lint). `make slow-test` is green overall. +- Every commit on the branch carries a `Signed-off-by:` trailer per the DCO bot. `git log --format='%(trailers:key=Signed-off-by)' main..HEAD` shows one for each commit. + +## Key discoveries & assumptions + +**Facts** (each verified during exploration): + +- The state-merge block in `Context.load()` is two consecutive `if any(self._projects):` guards at `sqlmesh/core/context.py:677-699`. The first loads environment statements from prod (lines 677-683); the second populates the local `uncached` set and merges remote snapshots into `self._models` / `self._standalone_audits` (lines 685-699). Both touch state via `self.state_reader.get_environment(c.PROD)`. +- `GenericContext.__init__` lives at `sqlmesh/core/context.py:376` with the existing `load: bool = True` parameter at line 385. `self.load()` is called at the very end of `__init__` if `load` is true (line 473). +- The CLI top-level group `cli(...)` lives at `sqlmesh/cli/main.py:94`. The `SKIP_CONTEXT_COMMANDS` and `SKIP_LOAD_COMMANDS` tuples are at `cli/main.py:31` and `cli/main.py:43`. The Context construction call is at `cli/main.py:132-137`. +- Existing format tests live at `tests/core/test_format.py` (no module-level mark — runs under `fast`). They construct a `Context` directly with `Config()` and `tmp_path` (e.g. `test_format_files` at line 14). +- Existing linter tests live at `tests/core/linter/test_builtin.py` (also no module-level mark — fast). They use the `copy_to_temp_path` fixture from `tests/conftest.py:565` against `examples/sushi`. +- CLI runner tests live at `tests/cli/test_cli.py`. The module is marked `pytestmark = pytest.mark.slow` at line 23. The runner fixture is at line 31, returning `CliRunner(env={"COLUMNS": "80"})`. `create_example_project` helper is at line 36. +- The `Linter` is built per-project in `Context.load()` at `context.py:670-674` before the state-merge block. Linters don't depend on the state-merge having run. +- `format` and `lint` Click handlers are at `sqlmesh/cli/main.py:343-380` and `sqlmesh/cli/main.py:1168-1185` respectively. Neither needs argument changes. + +**Assumptions** (flagged for the implementer to confirm during work): + +- A Postgres connection config pointing at an unreachable host (e.g. `localhost:1`) is sufficient to make state access fail loudly without requiring `psycopg2` to actually attempt a real connection — the failure should happen at `get_versions` cursor-fetch time, not at config validation time. If config validation rejects the host, fall back to patching `EngineAdapterStateSync.get_versions` to raise. +- No existing test currently exercises the "format/lint succeed despite broken state" path. Verified by `rg 'format.*state\|lint.*state' tests/` returning no relevant matches, but the implementer should confirm before adding the regression. + +## Existing patterns & conventions to follow + +**Code patterns:** + +- **CLI command-class tuples.** `SKIP_LOAD_COMMANDS` and `SKIP_CONTEXT_COMMANDS` at `sqlmesh/cli/main.py:31,43` are plain module-level string tuples; the `cli(...)` group reads them inside `if ctx.invoked_subcommand in ...` checks. The new `LOCAL_ONLY_COMMANDS` follows the same shape and is checked in the same block (`cli/main.py:120-123`). +- **Boolean construction parameters.** `GenericContext.__init__` (`context.py:376-389`) uses plain `bool` parameters with `True`/`False` defaults — e.g. `load: bool = True`. Add `load_state: bool = True` in the same style, with a matching docstring entry in the class docstring at `context.py:362-373`. +- **Private instance storage.** Constructor flags are stored as private attributes prefixed with `_` (e.g. `self._loaded`, `self._loaders`, `self._models`). Store the new flag as `self._load_state`. +- **Guard composition.** Add to an existing `if`-guard with `and` rather than wrapping the block in a new outer `if`. Matches the style at `context.py:677` and keeps the diff minimal. + +**Test patterns:** + +- **Fast Context-level tests** construct a `Context` from a `tmp_path` plus a `Config(...)` literal (`tests/core/test_format.py:14-42`). No fixtures beyond `tmp_path` and `mocker` are needed. Tests are top-level `def test_*` functions, not classes. +- **Linter tests** use the `copy_to_temp_path` fixture (`tests/conftest.py:565`) and patch the project's `config.py` text to enable rules (`tests/core/linter/test_builtin.py:7-46`). The same fixture can be reused for the local-only lint test by writing a config with an unreachable state connection. +- **CLI tests** use the session-scoped `runner` fixture and the `create_example_project` helper (`tests/cli/test_cli.py:31-60`). The module-level `pytestmark = pytest.mark.slow` (line 23) inherits to every test added there. +- **Mocker assertions.** `tests/cli/test_cli.py` already uses `MagicMock` patterns. For "method never called", `mocker.patch(...)` followed by `mock.assert_not_called()` is the idiomatic shape in this repo (`rg "assert_not_called" tests/` shows ~20 hits). + +**Contribution conventions:** + +- **DCO sign-off.** Every commit needs `Signed-off-by:` — produced by `git commit -s`. The bot blocks merge otherwise. The docs commit on this branch already carries one. +- **Commit messages.** Recent commits use lowercase semantic prefixes (`fix:`, `chore:`, `docs:`) with imperative subject lines — see `git log --oneline -20` for the pattern. Combined with the global AGENTS.md format: subject ≤50 chars, body wrapped at 72 explaining why, trailers `Coding-Agent: pi` / `Model: ` / `Signed-off-by: ...`. +- **Pre-commit style.** `make py-style` runs ruff (check + format, line length 100), mypy, and the migration-sequence check. Run before each commit. If only Python changed, `make py-style` is faster than `make style` (which also runs prettier/eslint). +- **Test selection.** `make fast-test` runs the fast suite plus isolated groups. CLI tests in `tests/cli/test_cli.py` are slow-marked at the module level and require `make slow-test` for the full suite, but the targeted CLI test can be invoked directly via `pytest tests/cli/test_cli.py::` during development. +- **No new files unless necessary.** Per `03_contributing.md`, new Python files need the `# SPDX-License-Identifier: Apache-2.0` header. This plan adds no new source files — only modifies existing ones — so no header work needed. + +## Out of scope + +- **`dag` and any other CLI command.** Even though the `load_state` mechanism would make some of `dag` local-only, its `--select-model` path constructs a `Selector` that evaluates `self.state_reader` at construction time (`context.py:2947`). Fixing that requires touching `Selector` and every other `_new_selector` call site — separate change, not in this PR. +- **Refactoring `Context.load()` structure.** No extraction of the state-merge block into a private method, no rearrangement of the load sequence. The change is `and self._load_state` added to two existing guards. +- **Suppressing analytics or any other implicit network activity.** Anonymized analytics still fire on `format` and `lint` invocations after this change. That's governed by the existing `disable_anonymized_analytics` config knob and is orthogonal. +- **Tests for "command X still hits state."** No new test asserts that `plan`/`diff`/`run`/etc. continue to touch state. The default `load_state=True` preserves their behavior; the existing test suite already covers them. +- **Documentation updates.** The user-facing behavior change is "CI now works without state credentials." Discoverable from the PR description and release notes. No `docs/` page change in this PR. +- **Backporting or version-gating.** This is a forward-only change on `main`. No release-branch backport, no `@deprecated` shim, no feature flag. + +## Tasks + +Two tasks, two commits, one PR. + +--- + +### Task 1: Add `load_state` to Context and gate the state-merge block + +**Purpose:** Introduce the mechanism that lets a caller construct a `Context` that loads models but never touches the state backend. + +**Files:** +- Modify: `sqlmesh/core/context.py` +- Modify: `tests/core/test_format.py` +- Modify: `tests/core/linter/test_builtin.py` + +**Test cases** (red first, then implementation): + +`tests/core/test_format.py` (fast): +- *`test_format_without_state_load`*: Construct a `Context` with `paths=tmp_path`, a `Config` whose state connection points at `localhost:1` (Postgres), and `load_state=False`. Place one `.sql` model file under `models/` containing SQL that is already in canonical formatted form (so `--check` succeeds for content reasons — the test is about state access, not about formatting decisions). Call `context.format(check=True)` and assert it returns `True` without raising. Assert the file's contents on disk are unchanged. + +`tests/core/linter/test_builtin.py` (fast): +- *`test_lint_without_state_load`*: Using `copy_to_temp_path("examples/sushi")`, rewrite the sushi `config.py` to enable the linter (existing pattern at `test_builtin.py:21-40`) AND set a Postgres state connection at `localhost:1`. Construct `Context(paths=[sushi_path], load_state=False)`. Call `context.lint_models(raise_on_error=False)` and assert it returns a list (i.e., the linter ran end-to-end) without raising. + +**Implementation outline:** + +In `sqlmesh/core/context.py`: +- Extend the `GenericContext` class docstring args block at lines 356-368 with a one-line entry for the new parameter: a brief description noting it gates the remote-state merge inside `load()` and is only meaningful when `load=True`. +- Add `load_state: bool = True` to `GenericContext.__init__` after the existing `load: bool = True` at line 385. +- Store as `self._load_state` on the instance during `__init__`, alongside the other private attributes (`self._loaded`, `self._loaders`, etc., around lines 396-415). +- In `Context.load()`, tighten the two `if any(self._projects):` guards at lines 677 and 685 by ANDing `self._load_state` into each predicate. No other changes to `load()` body. `update_schemas`, `Linter.from_rules`, and the `analytics.collector.on_project_loaded` call are unaffected. + +**Verification:** +- Automated: `pytest tests/core/test_format.py::test_format_without_state_load tests/core/linter/test_builtin.py::test_lint_without_state_load -v` — new tests pass. +- Automated: `pytest tests/core/test_format.py tests/core/linter/ -v` — existing tests in these files still pass. +- Automated: `make py-style` — ruff, mypy, migration-sequence all clean. +- Manual: read the diff of `context.py` and confirm the only non-test changes are: one docstring line, one constructor parameter, one attribute assignment, two `and` insertions. + +**Commit:** `git commit -s` with subject `feat: add load_state flag to Context` (37 chars). Trailers: `Coding-Agent: pi`, `Model: `, `Signed-off-by: ...`. + +--- + +### Task 2: Wire `LOCAL_ONLY_COMMANDS` in the CLI + +**Purpose:** Make `sqlmesh format` and `sqlmesh lint` construct their `Context` with `load_state=False`, completing the user-facing behavior. + +**Files:** +- Modify: `sqlmesh/cli/main.py` +- Modify: `tests/cli/test_cli.py` + +**Test cases** (red first, then implementation): + +`tests/cli/test_cli.py` (slow, via module-level `pytestmark`): + +All three tests share a setup (factor into a local helper or repeat inline): +1. `create_example_project(tmp_path, template=ProjectTemplate.EMPTY)` to scaffold a real project. +2. Overwrite the generated `config.yaml` so the `gateways.local` block includes a `state_connection` of type `postgres` pointing at `host: localhost, port: 1`. The DuckDB warehouse connection stays as it is. This makes the project *configuration* declare an unreachable state, exercising the full config-loading path — not just the runtime patch. +3. Place one already-formatted `.sql` model under `models/` so `format --check` has no formatting reason to fail. +4. `mocker.patch("sqlmesh.core.state_sync.db.facade.EngineAdapterStateSync.get_versions", side_effect=RuntimeError("state should not be accessed"))` belt-and-suspenders: if the unreachable host somehow gets bypassed, the patch still catches it. + +- *`test_format_runs_without_state`*: Run setup. Invoke `runner.invoke(cli, ["--paths", str(tmp_path), "format", "--check"])`. Assert `result.exit_code == 0`. Assert the patched `get_versions` mock has `assert_not_called()`. +- *`test_lint_runs_without_state`*: Run setup. Invoke `runner.invoke(cli, ["--paths", str(tmp_path), "lint"])`. Assert `result.exit_code == 0` and the patched mock was not called. +- *`test_plan_still_loads_state`* (required guard-rail): Run setup, but patch `get_versions` to return a stub `Versions` object rather than raise, so `plan` can proceed past the version check. Invoke `runner.invoke(cli, ["--paths", str(tmp_path), "plan"], input="n\n")` to cancel at the prompt. Assert the patched method *was* called at least once. Confirms `LOCAL_ONLY_COMMANDS` didn't accidentally include `plan`. + +**Implementation outline:** + +In `sqlmesh/cli/main.py`: +- Add a new module-level constant `LOCAL_ONLY_COMMANDS = ("format", "lint")` immediately after `SKIP_CONTEXT_COMMANDS` (line 43), matching the surrounding tuple style. +- Inside `cli(...)` (lines 117-123), mirror the existing `load = True; if ... in SKIP_LOAD_COMMANDS: load = False` pattern to introduce a `load_state` local that defaults to `True` and flips to `False` when `ctx.invoked_subcommand in LOCAL_ONLY_COMMANDS`. +- Pass `load_state=load_state` as an additional keyword argument in the `Context(...)` call at lines 132-137. + +**Verification:** +- Automated: `pytest tests/cli/test_cli.py::test_format_runs_without_state tests/cli/test_cli.py::test_lint_runs_without_state tests/cli/test_cli.py::test_plan_still_loads_state -v` — all three new tests pass. +- Automated: `pytest tests/cli/test_cli.py::test_format_leading_comma_default -v` — existing format CLI test still green. +- Automated: `make py-style` — clean. +- Automated: `make fast-test` — full fast suite green. Confirms Task 1 didn't regress and Task 2 didn't disturb fast tests. +- Manual: from inside the repo, run `sqlmesh --paths examples/sushi format --check` and `sqlmesh --paths examples/sushi lint` against a real sushi project. They should succeed. (Sushi uses DuckDB so this won't *prove* offline behavior, but it sanity-checks the wiring.) +- Manual: read the diff of `cli/main.py` and confirm the only changes are: one new tuple, one new variable, one `if`-check, one `load_state=` kwarg in the `Context(...)` call. + +**Commit:** `git commit -s` with subject `cli: run format and lint without state` (38 chars). Trailers: `Coding-Agent: pi`, `Model: `, `Signed-off-by: ...`. From 82e04876fe53713e75b6a2a6552be229f2d8670e Mon Sep 17 00:00:00 2001 From: Joe Hartshorn <8881940+j-hartshorn@users.noreply.github.com> Date: Thu, 21 May 2026 10:58:05 +0100 Subject: [PATCH 03/10] feat: add load_state flag to Context Add `load_state: bool = True` to `GenericContext.__init__` and AND the new flag into the two `if any(self._projects):` guards in `Context.load()`. With `load_state=False`, no remote-snapshot merge runs, so `state_reader` is never read and the state-sync backend is never instantiated. The default preserves today's behavior for every existing caller. CLI wiring lands in the next commit; this one just introduces the mechanism and proves it. Tests patch `EngineAdapterStateSync.get_versions` to raise and assert it is never called when `load_state=False`. They set a non-empty `project` on the test config so `any(self._projects)` is truthy and the gate actually exercises `self._load_state` (with the default empty project, the outer `any(...)` short-circuits and the test is vacuous). Verified non-vacuous by flipping `load_state=True` and confirming both tests fail with the patched RuntimeError. The plan originally proposed an unreachable Postgres state connection; `psycopg2` is not installed in the dev env so `PostgresConnectionConfig` validates at construction time and the config can't be built. Implementation uses the plan's documented patch-based fallback instead. Plan updated to reflect the change. Coding-Agent: pi Model: anthropic/claude-sonnet-4-6 Signed-off-by: Joe Hartshorn <8881940+j-hartshorn@users.noreply.github.com> --- docs/2026-05-21_local-only-format/plan.md | 11 +++++---- sqlmesh/core/context.py | 7 ++++-- tests/core/linter/test_builtin.py | 29 +++++++++++++++++++++++ tests/core/test_format.py | 18 ++++++++++++++ 4 files changed, 58 insertions(+), 7 deletions(-) diff --git a/docs/2026-05-21_local-only-format/plan.md b/docs/2026-05-21_local-only-format/plan.md index 3f689e3de3..f1b5852a20 100644 --- a/docs/2026-05-21_local-only-format/plan.md +++ b/docs/2026-05-21_local-only-format/plan.md @@ -29,10 +29,11 @@ The PR is done when all of these are true: - The `Linter` is built per-project in `Context.load()` at `context.py:670-674` before the state-merge block. Linters don't depend on the state-merge having run. - `format` and `lint` Click handlers are at `sqlmesh/cli/main.py:343-380` and `sqlmesh/cli/main.py:1168-1185` respectively. Neither needs argument changes. -**Assumptions** (flagged for the implementer to confirm during work): +**Decisions resolved during implementation:** -- A Postgres connection config pointing at an unreachable host (e.g. `localhost:1`) is sufficient to make state access fail loudly without requiring `psycopg2` to actually attempt a real connection — the failure should happen at `get_versions` cursor-fetch time, not at config validation time. If config validation rejects the host, fall back to patching `EngineAdapterStateSync.get_versions` to raise. -- No existing test currently exercises the "format/lint succeed despite broken state" path. Verified by `rg 'format.*state\|lint.*state' tests/` returning no relevant matches, but the implementer should confirm before adding the regression. +- The plan originally suggested configuring an unreachable Postgres state connection. This doesn't work in the dev environment because `psycopg2` is not installed; `PostgresConnectionConfig` runs `_get_engine_import_validator("psycopg2", ...)` at config-validation time (`sqlmesh/core/config/connection.py:1424`) and raises before our code path is reached. Implementation uses the plan's documented fallback: patch `sqlmesh.core.state_sync.db.facade.EngineAdapterStateSync.get_versions` with `side_effect=RuntimeError(...)` and assert it is never called. +- The state-merge blocks are guarded by `if any(self._projects):`, where `self._projects = {config.project for config in self.configs.values()}`. `Config.project` defaults to `""` (`sqlmesh/core/config/root.py:142`), and `any({""})` is `False` — so a `Config()` literal short-circuits the guard before the new `self._load_state` term is evaluated, making the test vacuous. The Context-level tests therefore set a non-empty `project` (`Config(project="local_only")` in the format test; an inline rewrite of sushi's `config.py` to add `project="sushi"` in the linter test) to ensure the guard is actually exercised. +- No existing test exercised the "format/lint succeed despite broken state" path (verified by `rg 'format.*state\|lint.*state' tests/`). ## Existing patterns & conventions to follow @@ -85,10 +86,10 @@ Two tasks, two commits, one PR. **Test cases** (red first, then implementation): `tests/core/test_format.py` (fast): -- *`test_format_without_state_load`*: Construct a `Context` with `paths=tmp_path`, a `Config` whose state connection points at `localhost:1` (Postgres), and `load_state=False`. Place one `.sql` model file under `models/` containing SQL that is already in canonical formatted form (so `--check` succeeds for content reasons — the test is about state access, not about formatting decisions). Call `context.format(check=True)` and assert it returns `True` without raising. Assert the file's contents on disk are unchanged. +- *`test_format_without_state_load`*: Patch `EngineAdapterStateSync.get_versions` to raise `RuntimeError`. Place one `.sql` model under `models/`. Construct `Context(paths=tmp_path, config=Config(project="local_only"), load_state=False)` (non-empty `project` so `any(self._projects)` is truthy and the gate is exercised). Call `context.format(check=True)`. Assert the patched mock has `assert_not_called()`. Verified non-vacuous by flipping to `load_state=True` and confirming the test fails with the patched `RuntimeError`. `tests/core/linter/test_builtin.py` (fast): -- *`test_lint_without_state_load`*: Using `copy_to_temp_path("examples/sushi")`, rewrite the sushi `config.py` to enable the linter (existing pattern at `test_builtin.py:21-40`) AND set a Postgres state connection at `localhost:1`. Construct `Context(paths=[sushi_path], load_state=False)`. Call `context.lint_models(raise_on_error=False)` and assert it returns a list (i.e., the linter ran end-to-end) without raising. +- *`test_lint_without_state_load`*: Using `copy_to_temp_path("examples/sushi")`, rewrite the sushi `config.py` to add `project="sushi"` to the `Config(...)` call (so `any(self._projects)` is truthy). Patch `EngineAdapterStateSync.get_versions` to raise. Construct `Context(paths=[sushi_path], load_state=False)`. Call `context.lint_models(raise_on_error=False)`. Assert the patched mock has `assert_not_called()`. Verified non-vacuous the same way as the format test. **Implementation outline:** diff --git a/sqlmesh/core/context.py b/sqlmesh/core/context.py index 2861adbeda..46f50ae82d 100644 --- a/sqlmesh/core/context.py +++ b/sqlmesh/core/context.py @@ -363,6 +363,7 @@ class GenericContext(BaseContext, t.Generic[C]): connection as it appears in configuration will be used. concurrent_tasks: The maximum number of tasks that can use the connection concurrently. load: Whether or not to automatically load all models and macros (default True). + load_state: Whether to merge remote state into the local project during load (default True). Only meaningful when load=True. console: The rich instance used for printing out CLI command results. users: A list of users to make known to SQLMesh. """ @@ -383,6 +384,7 @@ def __init__( concurrent_tasks: t.Optional[int] = None, loader: t.Optional[t.Type[Loader]] = None, load: bool = True, + load_state: bool = True, users: t.Optional[t.List[User]] = None, config_loader_kwargs: t.Optional[t.Dict[str, t.Any]] = None, selector: t.Optional[t.Type[Selector]] = None, @@ -413,6 +415,7 @@ def __init__( self._engine_adapter: t.Optional[EngineAdapter] = None self._linters: t.Dict[str, Linter] = {} self._loaded: bool = False + self._load_state: bool = load_state self._selector_cls = selector or NativeSelector self.path, self.config = t.cast(t.Tuple[Path, C], next(iter(self.configs.items()))) @@ -674,7 +677,7 @@ def load(self, update_schemas: bool = True) -> GenericContext[C]: ) # Load environment statements from state for projects not in current load - if any(self._projects): + if self._load_state and any(self._projects): prod = self.state_reader.get_environment(c.PROD) if prod: existing_statements = self.state_reader.get_environment_statements(c.PROD) @@ -684,7 +687,7 @@ def load(self, update_schemas: bool = True) -> GenericContext[C]: uncached = set() - if any(self._projects): + if self._load_state and any(self._projects): prod = self.state_reader.get_environment(c.PROD) if prod: diff --git a/tests/core/linter/test_builtin.py b/tests/core/linter/test_builtin.py index 0ff91470ff..42ae5f9278 100644 --- a/tests/core/linter/test_builtin.py +++ b/tests/core/linter/test_builtin.py @@ -232,3 +232,32 @@ def test_no_missing_unit_tests(tmp_path, copy_to_temp_path): assert len(model_violations) == 0, ( f"Model {model_name} should not have a violation since it has a test" ) + + +def test_lint_without_state_load(tmp_path, copy_to_temp_path, mocker) -> None: + """`lint_models` with `load_state=False` runs end-to-end without touching state sync.""" + sushi_paths = copy_to_temp_path("examples/sushi") + sushi_path = sushi_paths[0] + + with open(sushi_path / "config.py", "r") as f: + read_file = f.read() + + # Set a non-empty project name so `any(self._projects)` is truthy and the + # state-merge guard in `Context.load()` actually exercises `self._load_state`. + read_file = read_file.replace( + "config = Config(\n gateways=", + 'config = Config(\n project="sushi",\n gateways=', + ) + assert 'project="sushi"' in read_file + + with open(sushi_path / "config.py", "w") as f: + f.writelines(read_file) + + mock = mocker.patch( + "sqlmesh.core.state_sync.db.facade.EngineAdapterStateSync.get_versions", + side_effect=RuntimeError("state should not be accessed"), + ) + + context = Context(paths=[sushi_path], load_state=False) + context.lint_models(raise_on_error=False) + mock.assert_not_called() diff --git a/tests/core/test_format.py b/tests/core/test_format.py index 7d544eadf0..6881992621 100644 --- a/tests/core/test_format.py +++ b/tests/core/test_format.py @@ -144,3 +144,21 @@ def test_ignore_formating_files(tmp_path: pathlib.Path): model3.read_text(encoding="utf-8") == "MODEL (\n name this.model3,\n dialect 'duckdb',\n formatting TRUE\n);\n\nSELECT\n 1 AS col" ) + + +def test_format_without_state_load(tmp_path: pathlib.Path, mocker: MockerFixture): + """`format` with `load_state=False` runs end-to-end without touching state sync.""" + mock = mocker.patch( + "sqlmesh.core.state_sync.db.facade.EngineAdapterStateSync.get_versions", + side_effect=RuntimeError("state should not be accessed"), + ) + + create_temp_file( + tmp_path, + pathlib.Path("models/example.sql"), + "MODEL(name local.example, dialect 'duckdb'); SELECT 1 AS col", + ) + + context = Context(paths=tmp_path, config=Config(project="local_only"), load_state=False) + context.format(check=True) + mock.assert_not_called() From addfc193464ad695db9686c08000b02903cdd32b Mon Sep 17 00:00:00 2001 From: Joe Hartshorn <8881940+j-hartshorn@users.noreply.github.com> Date: Thu, 21 May 2026 11:11:22 +0100 Subject: [PATCH 04/10] cli: run format and lint without state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add `LOCAL_ONLY_COMMANDS = ("format", "lint")` and pass `load_state=False` to the `Context(...)` constructor for those subcommands. Mirrors the existing `SKIP_LOAD_COMMANDS` shape; one new tuple, one new local, one new kwarg in the Context call. With Task 1's gate in place, `format` and `lint` now run end to end with no state-sync connection, no warehouse connection, and no credentials. A CI job with zero secrets can run them against a real project. Three new slow CLI tests share a setup helper that scaffolds an empty project, sets a non-empty `project` name (so `any(self._projects)` gates on `self._load_state` rather than short-circuiting on the empty string default), patches `EngineAdapterStateSync.get_versions` to raise, and writes one model file. The format and lint tests assert `exit_code == 0` and that the patch was never called. The guard-rail test spies on `Context.__init__` and asserts every call passed `load_state=True` for `plan` — stronger than asserting state was touched later, since `plan` would touch state regardless of `load_state` via `context.plan(...)`. Plan updated to reflect the spy-based assertion and the patch-throughout test mechanism. Coding-Agent: pi Model: google/gemini-3.5-flash Signed-off-by: Joe Hartshorn <8881940+j-hartshorn@users.noreply.github.com> --- docs/2026-05-21_local-only-format/plan.md | 14 +++---- sqlmesh/cli/main.py | 5 +++ tests/cli/test_cli.py | 47 +++++++++++++++++++++++ 3 files changed, 59 insertions(+), 7 deletions(-) diff --git a/docs/2026-05-21_local-only-format/plan.md b/docs/2026-05-21_local-only-format/plan.md index f1b5852a20..1197643833 100644 --- a/docs/2026-05-21_local-only-format/plan.md +++ b/docs/2026-05-21_local-only-format/plan.md @@ -121,15 +121,15 @@ In `sqlmesh/core/context.py`: `tests/cli/test_cli.py` (slow, via module-level `pytestmark`): -All three tests share a setup (factor into a local helper or repeat inline): +All three tests share a setup factored into `_setup_local_only_project(tmp_path, mocker)`: 1. `create_example_project(tmp_path, template=ProjectTemplate.EMPTY)` to scaffold a real project. -2. Overwrite the generated `config.yaml` so the `gateways.local` block includes a `state_connection` of type `postgres` pointing at `host: localhost, port: 1`. The DuckDB warehouse connection stays as it is. This makes the project *configuration* declare an unreachable state, exercising the full config-loading path — not just the runtime patch. -3. Place one already-formatted `.sql` model under `models/` so `format --check` has no formatting reason to fail. -4. `mocker.patch("sqlmesh.core.state_sync.db.facade.EngineAdapterStateSync.get_versions", side_effect=RuntimeError("state should not be accessed"))` belt-and-suspenders: if the unreachable host somehow gets bypassed, the patch still catches it. +2. Prepend `project: cli_test\n\n` to the generated `config.yaml` so `Config.project` is non-empty and `any(self._projects)` is truthy. Without this, the existing outer guard short-circuits regardless of `self._load_state` and the tests are vacuous. +3. Write one `.sql` model under `models/` so the CLI doesn't short-circuit with "no models found". +4. `mocker.patch("sqlmesh.core.state_sync.db.facade.EngineAdapterStateSync.get_versions", side_effect=RuntimeError("state should not be accessed"))`. (No `state_connection: postgres` block in `config.yaml` — same `psycopg2` problem as Task 1; YAML validation through Pydantic fires the import validator. The patch is the only mechanism.) -- *`test_format_runs_without_state`*: Run setup. Invoke `runner.invoke(cli, ["--paths", str(tmp_path), "format", "--check"])`. Assert `result.exit_code == 0`. Assert the patched `get_versions` mock has `assert_not_called()`. -- *`test_lint_runs_without_state`*: Run setup. Invoke `runner.invoke(cli, ["--paths", str(tmp_path), "lint"])`. Assert `result.exit_code == 0` and the patched mock was not called. -- *`test_plan_still_loads_state`* (required guard-rail): Run setup, but patch `get_versions` to return a stub `Versions` object rather than raise, so `plan` can proceed past the version check. Invoke `runner.invoke(cli, ["--paths", str(tmp_path), "plan"], input="n\n")` to cancel at the prompt. Assert the patched method *was* called at least once. Confirms `LOCAL_ONLY_COMMANDS` didn't accidentally include `plan`. +- *`test_format_runs_without_state`*: Run setup. Invoke `runner.invoke(cli, ["--paths", str(tmp_path), "format"])` (no `--check` — lets format write in place, exit_code 0 whenever the call returned). Assert `result.exit_code == 0` and `mock.assert_not_called()`. +- *`test_lint_runs_without_state`*: Run setup. Invoke `runner.invoke(cli, ["--paths", str(tmp_path), "lint"])`. Assert `result.exit_code == 0` and `mock.assert_not_called()`. +- *`test_plan_still_loads_state`* (required guard-rail): Run setup. *Additionally* spy on `Context.__init__` via `mocker.spy(Context, "__init__")`. Invoke `runner.invoke(cli, ["--paths", str(tmp_path), "plan"], input="n\n")`. Assert that the spy was called and that every call's `load_state` kwarg was `True` (defaults to `True` when omitted). This is stronger than asserting `mock.called` on `get_versions` — a regression where someone added `"plan"` to `LOCAL_ONLY_COMMANDS` would still hit state later via `context.plan(...)`, so `get_versions.called` alone wouldn't catch it. The spy proves the Context constructor itself was called with `load_state=True` for `plan`. Verified by temporarily appending `"plan"` to `LOCAL_ONLY_COMMANDS`; the spy-based assertion fails. **Implementation outline:** diff --git a/sqlmesh/cli/main.py b/sqlmesh/cli/main.py index ec5acbea59..fea627bdd0 100644 --- a/sqlmesh/cli/main.py +++ b/sqlmesh/cli/main.py @@ -41,6 +41,7 @@ "table_name", ) SKIP_CONTEXT_COMMANDS = ("init", "ui") +LOCAL_ONLY_COMMANDS = ("format", "lint") def _sqlmesh_version() -> str: @@ -115,6 +116,7 @@ def cli( configure_console(ignore_warnings=ignore_warnings) load = True + load_state = True if len(paths) == 1: path = os.path.abspath(paths[0]) @@ -123,6 +125,8 @@ def cli( return if ctx.invoked_subcommand in SKIP_LOAD_COMMANDS: load = False + if ctx.invoked_subcommand in LOCAL_ONLY_COMMANDS: + load_state = False configs = load_configs(config, Context.CONFIG_TYPE, paths, dotenv_path=dotenv) log_limit = list(configs.values())[0].log_limit @@ -135,6 +139,7 @@ def cli( config=configs, gateway=gateway, load=load, + load_state=load_state, ) except Exception: if debug: diff --git a/tests/cli/test_cli.py b/tests/cli/test_cli.py index 5e0737e1b6..2e9e5f5d32 100644 --- a/tests/cli/test_cli.py +++ b/tests/cli/test_cli.py @@ -2237,3 +2237,50 @@ def test_format_leading_comma_default(runner: CliRunner, tmp_path: Path): assert result.exit_code == 0 finally: del os.environ["SQLMESH__FORMAT__LEADING_COMMA"] + + +def _setup_local_only_project(tmp_path, mocker): + """Scaffold a project with a non-empty `project` name and patch state to raise.""" + create_example_project(tmp_path, template=ProjectTemplate.EMPTY) + config_path = tmp_path / "config.yaml" + existing = config_path.read_text(encoding="utf-8") + config_path.write_text("project: cli_test\n\n" + existing, encoding="utf-8") + + (tmp_path / "models" / "example.sql").write_text( + "MODEL(name local.example, dialect 'duckdb'); SELECT 1 AS col\n", + encoding="utf-8", + ) + + return mocker.patch( + "sqlmesh.core.state_sync.db.facade.EngineAdapterStateSync.get_versions", + side_effect=RuntimeError("state should not be accessed"), + ) + + +def test_format_runs_without_state(runner: CliRunner, tmp_path: Path, mocker): + mock = _setup_local_only_project(tmp_path, mocker) + result = runner.invoke(cli, ["--paths", str(tmp_path), "format"]) + assert result.exit_code == 0, f"Format failed: {result.output}\nException: {result.exception}" + mock.assert_not_called() + + +def test_lint_runs_without_state(runner: CliRunner, tmp_path: Path, mocker): + mock = _setup_local_only_project(tmp_path, mocker) + result = runner.invoke(cli, ["--paths", str(tmp_path), "lint"]) + assert result.exit_code == 0, f"Lint failed: {result.output}\nException: {result.exception}" + mock.assert_not_called() + + +def test_plan_still_loads_state(runner: CliRunner, tmp_path: Path, mocker): + """Guard-rail: confirm `plan` is not in LOCAL_ONLY_COMMANDS by checking + the Context constructor received load_state=True for it.""" + _setup_local_only_project(tmp_path, mocker) + init_spy = mocker.spy(Context, "__init__") + + runner.invoke(cli, ["--paths", str(tmp_path), "plan"], input="n\n") + + assert init_spy.called, "Context was never constructed" + load_state_values = [call.kwargs.get("load_state", True) for call in init_spy.call_args_list] + assert all(load_state_values), ( + f"Context was constructed with load_state=False for `plan`: {load_state_values}" + ) From ea5978547ca64c3efda46a97c7427245f6d82019 Mon Sep 17 00:00:00 2001 From: Joe Hartshorn <8881940+j-hartshorn@users.noreply.github.com> Date: Thu, 21 May 2026 11:28:30 +0100 Subject: [PATCH 05/10] fix: address whole-PR review findings Three issues caught in the final review against the spec: * Multi-path CLI invocations were still loading state. The LOCAL_ONLY_COMMANDS check was inside `if len(paths) == 1:`, mirroring the existing `SKIP_LOAD_COMMANDS` shape. But unlike load-skipping, local-only behaviour is independent of project count - multi-project monorepo users running `sqlmesh --paths a --paths b format` should not hit state either. Lifted the check out of the conditional. Verified empirically: multi-path format against an unreachable Postgres state now exits 0. * `GenericContext.__init__` had `load_state` inserted between `load` and `users`, shifting positional arguments for any caller outside this repo passing `users`, `config_loader_kwargs`, or `selector` positionally. Moved `load_state` to the end of the signature. In-repo callers (cli/main.py, magics.py, github controller) all use kwargs so they are unaffected. * The lint test was running with the linter disabled, so no built-in rule actually executed under `load_state=False`. Added the existing pattern's `LinterConfig(enabled=False, ...)` -> `enabled=True, rules=["nomissingexternalmodels"]` rewrite so the test exercises a real rule's code path while state is patched to raise. Plan updated to reflect the final signature placement and the lifted CLI check. Coding-Agent: pi Model: anthropic/claude-opus-4-7 Signed-off-by: Joe Hartshorn <8881940+j-hartshorn@users.noreply.github.com> --- docs/2026-05-21_local-only-format/plan.md | 4 ++-- sqlmesh/cli/main.py | 4 +--- sqlmesh/core/context.py | 2 +- tests/core/linter/test_builtin.py | 19 ++++++++++++++++++- 4 files changed, 22 insertions(+), 7 deletions(-) diff --git a/docs/2026-05-21_local-only-format/plan.md b/docs/2026-05-21_local-only-format/plan.md index 1197643833..8034ae58a1 100644 --- a/docs/2026-05-21_local-only-format/plan.md +++ b/docs/2026-05-21_local-only-format/plan.md @@ -95,7 +95,7 @@ Two tasks, two commits, one PR. In `sqlmesh/core/context.py`: - Extend the `GenericContext` class docstring args block at lines 356-368 with a one-line entry for the new parameter: a brief description noting it gates the remote-state merge inside `load()` and is only meaningful when `load=True`. -- Add `load_state: bool = True` to `GenericContext.__init__` after the existing `load: bool = True` at line 385. +- Add `load_state: bool = True` to `GenericContext.__init__` at the *end* of the parameter list (after `selector`). Placing it last avoids shifting any existing positional arguments for callers outside this repo who may pass `users`, `config_loader_kwargs`, or `selector` positionally. - Store as `self._load_state` on the instance during `__init__`, alongside the other private attributes (`self._loaded`, `self._loaders`, etc., around lines 396-415). - In `Context.load()`, tighten the two `if any(self._projects):` guards at lines 677 and 685 by ANDing `self._load_state` into each predicate. No other changes to `load()` body. `update_schemas`, `Linter.from_rules`, and the `analytics.collector.on_project_loaded` call are unaffected. @@ -135,7 +135,7 @@ All three tests share a setup factored into `_setup_local_only_project(tmp_path, In `sqlmesh/cli/main.py`: - Add a new module-level constant `LOCAL_ONLY_COMMANDS = ("format", "lint")` immediately after `SKIP_CONTEXT_COMMANDS` (line 43), matching the surrounding tuple style. -- Inside `cli(...)` (lines 117-123), mirror the existing `load = True; if ... in SKIP_LOAD_COMMANDS: load = False` pattern to introduce a `load_state` local that defaults to `True` and flips to `False` when `ctx.invoked_subcommand in LOCAL_ONLY_COMMANDS`. +- Inside `cli(...)` (around line 117), compute `load_state = ctx.invoked_subcommand not in LOCAL_ONLY_COMMANDS` *outside* the `if len(paths) == 1:` block. Unlike `SKIP_LOAD_COMMANDS` (whose `load = False` toggle is inside the single-path conditional), `load_state` must apply regardless of how many `--paths` were provided — multi-project monorepo invocations of `format`/`lint` need to be local-only too. - Pass `load_state=load_state` as an additional keyword argument in the `Context(...)` call at lines 132-137. **Verification:** diff --git a/sqlmesh/cli/main.py b/sqlmesh/cli/main.py index fea627bdd0..2f1962dabe 100644 --- a/sqlmesh/cli/main.py +++ b/sqlmesh/cli/main.py @@ -116,7 +116,7 @@ def cli( configure_console(ignore_warnings=ignore_warnings) load = True - load_state = True + load_state = ctx.invoked_subcommand not in LOCAL_ONLY_COMMANDS if len(paths) == 1: path = os.path.abspath(paths[0]) @@ -125,8 +125,6 @@ def cli( return if ctx.invoked_subcommand in SKIP_LOAD_COMMANDS: load = False - if ctx.invoked_subcommand in LOCAL_ONLY_COMMANDS: - load_state = False configs = load_configs(config, Context.CONFIG_TYPE, paths, dotenv_path=dotenv) log_limit = list(configs.values())[0].log_limit diff --git a/sqlmesh/core/context.py b/sqlmesh/core/context.py index 46f50ae82d..54ef50ae66 100644 --- a/sqlmesh/core/context.py +++ b/sqlmesh/core/context.py @@ -384,10 +384,10 @@ def __init__( concurrent_tasks: t.Optional[int] = None, loader: t.Optional[t.Type[Loader]] = None, load: bool = True, - load_state: bool = True, users: t.Optional[t.List[User]] = None, config_loader_kwargs: t.Optional[t.Dict[str, t.Any]] = None, selector: t.Optional[t.Type[Selector]] = None, + load_state: bool = True, ): self.configs = ( config diff --git a/tests/core/linter/test_builtin.py b/tests/core/linter/test_builtin.py index 42ae5f9278..68d67e7714 100644 --- a/tests/core/linter/test_builtin.py +++ b/tests/core/linter/test_builtin.py @@ -235,7 +235,7 @@ def test_no_missing_unit_tests(tmp_path, copy_to_temp_path): def test_lint_without_state_load(tmp_path, copy_to_temp_path, mocker) -> None: - """`lint_models` with `load_state=False` runs end-to-end without touching state sync.""" + """`lint_models` with `load_state=False` runs built-in rules without touching state sync.""" sushi_paths = copy_to_temp_path("examples/sushi") sushi_path = sushi_paths[0] @@ -250,6 +250,23 @@ def test_lint_without_state_load(tmp_path, copy_to_temp_path, mocker) -> None: ) assert 'project="sushi"' in read_file + # Enable the linter with one built-in rule so `lint_models` actually executes + # a rule under `load_state=False`, not just the empty-rule-set path. + before = """ linter=LinterConfig( + enabled=False, + rules=[ + "ambiguousorinvalidcolumn", + "invalidselectstarexpansion", + "noselectstar", + "nomissingaudits", + "nomissingowner", + "nomissingexternalmodels", + ], + ),""" + after = 'linter=LinterConfig(enabled=True, rules=["nomissingexternalmodels"]),' + read_file = read_file.replace(before, after) + assert after in read_file + with open(sushi_path / "config.py", "w") as f: f.writelines(read_file) From 95d2f9d11f7d7248092025ad78bd67ea20b7f638 Mon Sep 17 00:00:00 2001 From: Joe Hartshorn <8881940+j-hartshorn@users.noreply.github.com> Date: Thu, 21 May 2026 12:34:32 +0100 Subject: [PATCH 06/10] polish: address fresh-eyes review Three parallel whole-PR reviews surfaced no blockers but several genuine resilience and clarity improvements. Applied: * Added a CLI regression test covering the realistic CI scenario: a config.yaml declaring a Postgres state connection whose credentials come from unset env vars. Proves end-to-end that no secrets are needed for format. Skipped when psycopg2 isn't importable. Non-vacuity verified: temporarily forcing load_state=True makes the test fail with the patched RuntimeError. * Hardened the lint test's string-replace anchors with explicit `assert anchor in read_file` checks. If sushi's config.py shape ever drifts, the test will now fail at the anchor with a useful message instead of silently no-op-ing and producing a confusing downstream symptom. * Documented the plan-state guard-rail test's mechanic in its docstring (we don't assert on exit_code because state is patched to raise; the spy records the constructor call regardless). * Added a code comment by `load_state = ...` in cli/main.py explaining why the assignment is outside the single-path conditional, so a future tidy-up doesn't re-introduce the multi-path regression that the previous fix commit just closed. * Tightened wording in the `load_state` docstring on GenericContext ("Consulted by load(); has no effect when load() is not called" rather than "Only meaningful when load=True", which was imprecise for manual post-construction `context.load()` calls). * Replaced `f.writelines(read_file)` with `f.write(read_file)` in the lint test. The original worked because strings are iterable, but read as a bug. * Documented the pre-existing Snowflake-default-auth limitation in the spec's Risks section: Postgres state configs validate fine with env_var-resolved None values, but Snowflake's `_validate_authenticator` raises ConfigError at config-load time if user and password are both missing under default auth. Not introduced by this PR (the validator predates it) but it's the one realistic CI setup where "zero secrets" doesn't hold. Coding-Agent: pi Model: anthropic/claude-opus-4-7 Signed-off-by: Joe Hartshorn <8881940+j-hartshorn@users.noreply.github.com> --- docs/2026-05-21_local-only-format/spec.md | 1 + sqlmesh/cli/main.py | 2 + sqlmesh/core/context.py | 2 +- tests/cli/test_cli.py | 63 ++++++++++++++++++++++- tests/core/linter/test_builtin.py | 13 +++-- 5 files changed, 75 insertions(+), 6 deletions(-) diff --git a/docs/2026-05-21_local-only-format/spec.md b/docs/2026-05-21_local-only-format/spec.md index 32ef466e4e..4393315055 100644 --- a/docs/2026-05-21_local-only-format/spec.md +++ b/docs/2026-05-21_local-only-format/spec.md @@ -67,6 +67,7 @@ Tests: add a regression test that constructs a `Context` against a project whose - **`update_model_schemas` cache state.** `load(update_schemas=True)` runs after the state-merge block. When the block is skipped, the local `MappingSchema` won't contain remote snapshot columns. For `format` and `lint` this is fine — neither needs resolved column types from remote projects — but if a linter rule is ever added later that relies on cross-project column resolution, it would silently produce different results under `load_state=False`. Note in the linter rule contract, but no code change needed today. - **CLI test isolation.** Asserting "no state connection was opened" is easier to assert by patching than by mocking the network. The cleanest seam is `EngineAdapterStateSync.get_versions` — patching it to raise should be enough; we then assert `format`/`lint` succeed and the patched method is never called. If that seam turns out to be wrong (e.g., a different method gets called first), we'll adjust during implementation. - **Multi-project monorepos.** The state-merge block's purpose is to import remote snapshots for *other* projects when the current load covers only a subset (`context.py:681-697`, gated on `any(self._projects)`). For `format`/`lint`, this means a model that depends on a model defined in a sibling project not currently being loaded won't have its upstream resolved. That's already the behavior for any model `format`/`lint` touches — they operate per-file, not on rendered downstream queries — so there should be no observable difference. Flagging in case a reviewer raises it. +- **Snowflake default-auth state connections.** `PostgresConnectionConfig` accepts `password=None` from a missing `{{ env_var() }}` (Pydantic coercion), so a Postgres state config validates with no secrets and the gate then prevents the connection. `SnowflakeConnectionConfig._validate_authenticator` (`sqlmesh/core/config/connection.py:633-638`) is stricter: under default authentication it raises `ConfigError("User and password must be provided")` at config-load time if both are missing. That validation runs before our `load_state` gate can take effect, so a project whose state lives in Snowflake with default-auth still requires user/password env vars to be set (any non-empty values — they won't be used). This is pre-existing Snowflake validator behavior, not something this change introduces, but it's the one realistic CI configuration where "zero secrets" doesn't hold. Workarounds: use the Snowflake key-pair or OAuth authenticators (which have their own validators with different requirements), or set placeholder env vars in the CI job. ## Dependencies diff --git a/sqlmesh/cli/main.py b/sqlmesh/cli/main.py index 2f1962dabe..9d0ba7d28e 100644 --- a/sqlmesh/cli/main.py +++ b/sqlmesh/cli/main.py @@ -116,6 +116,8 @@ def cli( configure_console(ignore_warnings=ignore_warnings) load = True + # Computed outside the single-path block: local-only behaviour must apply + # regardless of how many --paths were provided, unlike SKIP_LOAD_COMMANDS. load_state = ctx.invoked_subcommand not in LOCAL_ONLY_COMMANDS if len(paths) == 1: diff --git a/sqlmesh/core/context.py b/sqlmesh/core/context.py index 54ef50ae66..b25ee64fe6 100644 --- a/sqlmesh/core/context.py +++ b/sqlmesh/core/context.py @@ -363,7 +363,7 @@ class GenericContext(BaseContext, t.Generic[C]): connection as it appears in configuration will be used. concurrent_tasks: The maximum number of tasks that can use the connection concurrently. load: Whether or not to automatically load all models and macros (default True). - load_state: Whether to merge remote state into the local project during load (default True). Only meaningful when load=True. + load_state: Whether to merge remote state into the local project during load (default True). Consulted by load(); has no effect when load() is not called. console: The rich instance used for printing out CLI command results. users: A list of users to make known to SQLMesh. """ diff --git a/tests/cli/test_cli.py b/tests/cli/test_cli.py index 2e9e5f5d32..2245197e3e 100644 --- a/tests/cli/test_cli.py +++ b/tests/cli/test_cli.py @@ -2273,7 +2273,13 @@ def test_lint_runs_without_state(runner: CliRunner, tmp_path: Path, mocker): def test_plan_still_loads_state(runner: CliRunner, tmp_path: Path, mocker): """Guard-rail: confirm `plan` is not in LOCAL_ONLY_COMMANDS by checking - the Context constructor received load_state=True for it.""" + the Context constructor received load_state=True for it. + + The `plan` invocation is expected to fail because state access is patched + to raise. We don't assert on exit_code; mocker.spy records the constructor + call before the wrapped __init__ runs, so the kwargs assertion holds + regardless of whether the call ultimately raised. + """ _setup_local_only_project(tmp_path, mocker) init_spy = mocker.spy(Context, "__init__") @@ -2284,3 +2290,58 @@ def test_plan_still_loads_state(runner: CliRunner, tmp_path: Path, mocker): assert all(load_state_values), ( f"Context was constructed with load_state=False for `plan`: {load_state_values}" ) + + +def test_format_runs_without_state_credentials( + runner: CliRunner, tmp_path: Path, mocker, monkeypatch +): + """Realistic CI scenario: a config.yaml declaring a remote Postgres state + connection with credentials sourced from unset env vars. Format must still + succeed without opening the state connection. + + Distinct from test_format_runs_without_state: that one proves the gate by + patching get_versions. This one proves the end-to-end CI use case where + no secrets are provisioned and YAML env_var() resolves to None. + """ + pytest.importorskip("psycopg2") + + for var in ("PG_HOST", "PG_USER", "PG_PASSWORD", "PG_DATABASE"): + monkeypatch.delenv(var, raising=False) + + create_example_project(tmp_path, template=ProjectTemplate.EMPTY) + (tmp_path / "config.yaml").write_text( + """project: cli_test + +gateways: + prod: + state_connection: + type: postgres + host: "{{ env_var('PG_HOST', 'postgres.internal.example.com') }}" + port: 5432 + user: "{{ env_var('PG_USER') }}" + password: "{{ env_var('PG_PASSWORD') }}" + database: "{{ env_var('PG_DATABASE', 'sqlmesh_state') }}" + connection: + type: duckdb + database: "warehouse.db" + +default_gateway: prod + +model_defaults: + dialect: duckdb +""", + encoding="utf-8", + ) + (tmp_path / "models" / "example.sql").write_text( + "MODEL(name local.example, dialect 'duckdb'); SELECT 1 AS col\n", + encoding="utf-8", + ) + + mock = mocker.patch( + "sqlmesh.core.state_sync.db.facade.EngineAdapterStateSync.get_versions", + side_effect=RuntimeError("state should not be accessed"), + ) + + result = runner.invoke(cli, ["--paths", str(tmp_path), "format"]) + assert result.exit_code == 0, f"Format failed: {result.output}\nException: {result.exception}" + mock.assert_not_called() diff --git a/tests/core/linter/test_builtin.py b/tests/core/linter/test_builtin.py index 68d67e7714..6c11f543c2 100644 --- a/tests/core/linter/test_builtin.py +++ b/tests/core/linter/test_builtin.py @@ -244,11 +244,14 @@ def test_lint_without_state_load(tmp_path, copy_to_temp_path, mocker) -> None: # Set a non-empty project name so `any(self._projects)` is truthy and the # state-merge guard in `Context.load()` actually exercises `self._load_state`. + project_anchor = "config = Config(\n gateways=" + assert project_anchor in read_file, ( + "sushi config.py shape drifted; update project_anchor in test" + ) read_file = read_file.replace( - "config = Config(\n gateways=", + project_anchor, 'config = Config(\n project="sushi",\n gateways=', ) - assert 'project="sushi"' in read_file # Enable the linter with one built-in rule so `lint_models` actually executes # a rule under `load_state=False`, not just the empty-rule-set path. @@ -264,11 +267,13 @@ def test_lint_without_state_load(tmp_path, copy_to_temp_path, mocker) -> None: ], ),""" after = 'linter=LinterConfig(enabled=True, rules=["nomissingexternalmodels"]),' + assert before in read_file, ( + "sushi config.py LinterConfig block shape drifted; update `before` in test" + ) read_file = read_file.replace(before, after) - assert after in read_file with open(sushi_path / "config.py", "w") as f: - f.writelines(read_file) + f.write(read_file) mock = mocker.patch( "sqlmesh.core.state_sync.db.facade.EngineAdapterStateSync.get_versions", From 4c5c80e9741918ed04ad7429b9dc35acb3017820 Mon Sep 17 00:00:00 2001 From: Joe Hartshorn <8881940+j-hartshorn@users.noreply.github.com> Date: Thu, 21 May 2026 13:20:26 +0100 Subject: [PATCH 07/10] test: harden plan-still-loads-state guards The previous assertion `call.kwargs.get("load_state", True)` silently passes if the `load_state=load_state` line is removed from cli/main.py entirely - the kwarg becomes absent, defaults to True, assertion holds. Combined with the earlier point that `mock.called` alone is too loose (because `context.plan(...)` touches state regardless of load_state), the right guard checks three complementary things: - "load_state" is in call.kwargs (catches kwarg deletion) - the value is True (catches `plan` being added to the tuple) - the patched get_versions was actually called (catches `plan` being added to the tuple from the other direction) Both regression modes verified to fail loudly with the new asserts. Coding-Agent: pi Model: anthropic/claude-opus-4-7 Signed-off-by: Joe Hartshorn <8881940+j-hartshorn@users.noreply.github.com> --- tests/cli/test_cli.py | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/tests/cli/test_cli.py b/tests/cli/test_cli.py index 2245197e3e..79aa6d32a6 100644 --- a/tests/cli/test_cli.py +++ b/tests/cli/test_cli.py @@ -2272,24 +2272,36 @@ def test_lint_runs_without_state(runner: CliRunner, tmp_path: Path, mocker): def test_plan_still_loads_state(runner: CliRunner, tmp_path: Path, mocker): - """Guard-rail: confirm `plan` is not in LOCAL_ONLY_COMMANDS by checking - the Context constructor received load_state=True for it. + """Guard-rail: confirm `plan` is not in LOCAL_ONLY_COMMANDS. + + Asserts two complementary things, because either alone has a blind spot: + 1. The CLI explicitly passes `load_state=True` to `Context(...)` for `plan`. + Checking `"load_state" in call.kwargs` (not just the value) catches a + regression where someone deletes the `load_state=load_state` line in + cli/main.py entirely — the kwarg would be absent and silently default + to True without this check. + 2. State sync was actually accessed. Catches a regression where someone + adds `"plan"` to LOCAL_ONLY_COMMANDS — `load_state` would be False and + the patched method would never be called. The `plan` invocation is expected to fail because state access is patched to raise. We don't assert on exit_code; mocker.spy records the constructor - call before the wrapped __init__ runs, so the kwargs assertion holds - regardless of whether the call ultimately raised. + call before the wrapped __init__ runs, so kwargs are recorded regardless. """ - _setup_local_only_project(tmp_path, mocker) + mock = _setup_local_only_project(tmp_path, mocker) init_spy = mocker.spy(Context, "__init__") runner.invoke(cli, ["--paths", str(tmp_path), "plan"], input="n\n") assert init_spy.called, "Context was never constructed" - load_state_values = [call.kwargs.get("load_state", True) for call in init_spy.call_args_list] - assert all(load_state_values), ( - f"Context was constructed with load_state=False for `plan`: {load_state_values}" - ) + for call in init_spy.call_args_list: + assert "load_state" in call.kwargs, ( + "CLI didn't pass load_state= explicitly; missing kwarg defaults to True silently" + ) + assert call.kwargs["load_state"] is True, ( + f"Context was constructed with load_state={call.kwargs['load_state']} for `plan`" + ) + assert mock.called, "state-sync was never accessed during `plan`" def test_format_runs_without_state_credentials( From 8016f2ab2c3e316d8062ca332506e13d3f2d1490 Mon Sep 17 00:00:00 2001 From: Joe Hartshorn <8881940+j-hartshorn@users.noreply.github.com> Date: Thu, 21 May 2026 13:20:49 +0100 Subject: [PATCH 08/10] test: rename realistic-CI test, fix env_var note The previous name oversold the scenario - credentials aren't "absent", they're stringified placeholders. Jinja `env_var('FOO')` with FOO unset renders the literal string "None" into the YAML, which Pydantic accepts for Postgres's `user`/`password` (both typed `str`). The test proves the gate prevents a connection attempt, not that credentials are missing. Renamed to test_format_does_not_open_state_connection and rewrote the docstring to describe what actually happens. Coding-Agent: pi Model: anthropic/claude-opus-4-7 Signed-off-by: Joe Hartshorn <8881940+j-hartshorn@users.noreply.github.com> --- tests/cli/test_cli.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/cli/test_cli.py b/tests/cli/test_cli.py index 79aa6d32a6..dd956b5ea7 100644 --- a/tests/cli/test_cli.py +++ b/tests/cli/test_cli.py @@ -2304,7 +2304,7 @@ def test_plan_still_loads_state(runner: CliRunner, tmp_path: Path, mocker): assert mock.called, "state-sync was never accessed during `plan`" -def test_format_runs_without_state_credentials( +def test_format_does_not_open_state_connection( runner: CliRunner, tmp_path: Path, mocker, monkeypatch ): """Realistic CI scenario: a config.yaml declaring a remote Postgres state @@ -2312,8 +2312,12 @@ def test_format_runs_without_state_credentials( succeed without opening the state connection. Distinct from test_format_runs_without_state: that one proves the gate by - patching get_versions. This one proves the end-to-end CI use case where - no secrets are provisioned and YAML env_var() resolves to None. + patching get_versions on a default-DuckDB project. This one proves the + end-to-end CI use case where the config declares a real remote backend and + no secrets are provisioned. Jinja `env_var('FOO')` with `FOO` unset + renders the string `"None"` into the YAML; Pydantic accepts that for + Postgres's `user`/`password` (both typed `str`), so config validation + passes with placeholder values. The gate then prevents any connection. """ pytest.importorskip("psycopg2") From fc1642f587cbc23d42c51d15b54923bffca3ec1f Mon Sep 17 00:00:00 2001 From: Joe Hartshorn <8881940+j-hartshorn@users.noreply.github.com> Date: Thu, 21 May 2026 13:29:00 +0100 Subject: [PATCH 09/10] style: trim redundant comments and docstrings Audit pass against the repo's "comments only when the WHY is non-obvious" standard. Deleted three test docstrings that restated the function name. Shortened five over-explanatory comments to one line each. Kept the assertion-failure diagnostics that genuinely help the next maintainer (sushi config drift messages; the kwarg-presence assertion message documenting why we don't only check the value). Net: -38 lines / +6 lines across the touched files. No behaviour change. Coding-Agent: pi Model: anthropic/claude-opus-4-7 Signed-off-by: Joe Hartshorn <8881940+j-hartshorn@users.noreply.github.com> --- sqlmesh/cli/main.py | 3 +-- sqlmesh/core/context.py | 2 +- tests/cli/test_cli.py | 31 ++----------------------------- tests/core/linter/test_builtin.py | 7 ++----- tests/core/test_format.py | 1 - 5 files changed, 6 insertions(+), 38 deletions(-) diff --git a/sqlmesh/cli/main.py b/sqlmesh/cli/main.py index 9d0ba7d28e..f67d0717cc 100644 --- a/sqlmesh/cli/main.py +++ b/sqlmesh/cli/main.py @@ -116,8 +116,7 @@ def cli( configure_console(ignore_warnings=ignore_warnings) load = True - # Computed outside the single-path block: local-only behaviour must apply - # regardless of how many --paths were provided, unlike SKIP_LOAD_COMMANDS. + # Outside the single-path block: applies regardless of --paths count. load_state = ctx.invoked_subcommand not in LOCAL_ONLY_COMMANDS if len(paths) == 1: diff --git a/sqlmesh/core/context.py b/sqlmesh/core/context.py index b25ee64fe6..baba7b9774 100644 --- a/sqlmesh/core/context.py +++ b/sqlmesh/core/context.py @@ -363,7 +363,7 @@ class GenericContext(BaseContext, t.Generic[C]): connection as it appears in configuration will be used. concurrent_tasks: The maximum number of tasks that can use the connection concurrently. load: Whether or not to automatically load all models and macros (default True). - load_state: Whether to merge remote state into the local project during load (default True). Consulted by load(); has no effect when load() is not called. + load_state: Whether to merge remote state into the local project during load (default True). console: The rich instance used for printing out CLI command results. users: A list of users to make known to SQLMesh. """ diff --git a/tests/cli/test_cli.py b/tests/cli/test_cli.py index dd956b5ea7..c76a771eea 100644 --- a/tests/cli/test_cli.py +++ b/tests/cli/test_cli.py @@ -2240,7 +2240,6 @@ def test_format_leading_comma_default(runner: CliRunner, tmp_path: Path): def _setup_local_only_project(tmp_path, mocker): - """Scaffold a project with a non-empty `project` name and patch state to raise.""" create_example_project(tmp_path, template=ProjectTemplate.EMPTY) config_path = tmp_path / "config.yaml" existing = config_path.read_text(encoding="utf-8") @@ -2272,22 +2271,7 @@ def test_lint_runs_without_state(runner: CliRunner, tmp_path: Path, mocker): def test_plan_still_loads_state(runner: CliRunner, tmp_path: Path, mocker): - """Guard-rail: confirm `plan` is not in LOCAL_ONLY_COMMANDS. - - Asserts two complementary things, because either alone has a blind spot: - 1. The CLI explicitly passes `load_state=True` to `Context(...)` for `plan`. - Checking `"load_state" in call.kwargs` (not just the value) catches a - regression where someone deletes the `load_state=load_state` line in - cli/main.py entirely — the kwarg would be absent and silently default - to True without this check. - 2. State sync was actually accessed. Catches a regression where someone - adds `"plan"` to LOCAL_ONLY_COMMANDS — `load_state` would be False and - the patched method would never be called. - - The `plan` invocation is expected to fail because state access is patched - to raise. We don't assert on exit_code; mocker.spy records the constructor - call before the wrapped __init__ runs, so kwargs are recorded regardless. - """ + """Guard that `plan` explicitly passes `load_state=True` and still reaches state sync.""" mock = _setup_local_only_project(tmp_path, mocker) init_spy = mocker.spy(Context, "__init__") @@ -2307,18 +2291,7 @@ def test_plan_still_loads_state(runner: CliRunner, tmp_path: Path, mocker): def test_format_does_not_open_state_connection( runner: CliRunner, tmp_path: Path, mocker, monkeypatch ): - """Realistic CI scenario: a config.yaml declaring a remote Postgres state - connection with credentials sourced from unset env vars. Format must still - succeed without opening the state connection. - - Distinct from test_format_runs_without_state: that one proves the gate by - patching get_versions on a default-DuckDB project. This one proves the - end-to-end CI use case where the config declares a real remote backend and - no secrets are provisioned. Jinja `env_var('FOO')` with `FOO` unset - renders the string `"None"` into the YAML; Pydantic accepts that for - Postgres's `user`/`password` (both typed `str`), so config validation - passes with placeholder values. The gate then prevents any connection. - """ + """Format must not open a configured remote Postgres state connection when CI secrets are unset.""" pytest.importorskip("psycopg2") for var in ("PG_HOST", "PG_USER", "PG_PASSWORD", "PG_DATABASE"): diff --git a/tests/core/linter/test_builtin.py b/tests/core/linter/test_builtin.py index 6c11f543c2..3c472c5be0 100644 --- a/tests/core/linter/test_builtin.py +++ b/tests/core/linter/test_builtin.py @@ -235,15 +235,13 @@ def test_no_missing_unit_tests(tmp_path, copy_to_temp_path): def test_lint_without_state_load(tmp_path, copy_to_temp_path, mocker) -> None: - """`lint_models` with `load_state=False` runs built-in rules without touching state sync.""" sushi_paths = copy_to_temp_path("examples/sushi") sushi_path = sushi_paths[0] with open(sushi_path / "config.py", "r") as f: read_file = f.read() - # Set a non-empty project name so `any(self._projects)` is truthy and the - # state-merge guard in `Context.load()` actually exercises `self._load_state`. + # Set a project name so state-merge code reaches the `self._load_state` guard. project_anchor = "config = Config(\n gateways=" assert project_anchor in read_file, ( "sushi config.py shape drifted; update project_anchor in test" @@ -253,8 +251,7 @@ def test_lint_without_state_load(tmp_path, copy_to_temp_path, mocker) -> None: 'config = Config(\n project="sushi",\n gateways=', ) - # Enable the linter with one built-in rule so `lint_models` actually executes - # a rule under `load_state=False`, not just the empty-rule-set path. + # Enable one built-in rule so `lint_models` doesn't take the empty-rule-set path. before = """ linter=LinterConfig( enabled=False, rules=[ diff --git a/tests/core/test_format.py b/tests/core/test_format.py index 6881992621..5a44e1b381 100644 --- a/tests/core/test_format.py +++ b/tests/core/test_format.py @@ -147,7 +147,6 @@ def test_ignore_formating_files(tmp_path: pathlib.Path): def test_format_without_state_load(tmp_path: pathlib.Path, mocker: MockerFixture): - """`format` with `load_state=False` runs end-to-end without touching state sync.""" mock = mocker.patch( "sqlmesh.core.state_sync.db.facade.EngineAdapterStateSync.get_versions", side_effect=RuntimeError("state should not be accessed"), From 6427cc8c23359212f9e71f361f8258575bab2098 Mon Sep 17 00:00:00 2001 From: Joe Hartshorn <8881940+j-hartshorn@users.noreply.github.com> Date: Thu, 21 May 2026 13:33:11 +0100 Subject: [PATCH 10/10] chore: remove planning docs from PR The spec, plan, research, and notes under docs/2026-05-21_local-only-format/ guided this change but don't need to live in the upstream tree. They're preserved in git history on the earlier commits of this branch (5c43b3d5, 15bf65bb, 95d2f9d1, ea597854) for anyone who wants the design context. Coding-Agent: pi Model: anthropic/claude-opus-4-7 Signed-off-by: Joe Hartshorn <8881940+j-hartshorn@users.noreply.github.com> --- .../01_format_command.md | 166 ------------- .../02_cli_bootstrap.md | 169 ------------- .../03_contributing.md | 230 ------------------ docs/2026-05-21_local-only-format/plan.md | 149 ------------ docs/2026-05-21_local-only-format/spec.md | 80 ------ .../spec_notes.md | 101 -------- 6 files changed, 895 deletions(-) delete mode 100644 docs/2026-05-21_local-only-format/01_format_command.md delete mode 100644 docs/2026-05-21_local-only-format/02_cli_bootstrap.md delete mode 100644 docs/2026-05-21_local-only-format/03_contributing.md delete mode 100644 docs/2026-05-21_local-only-format/plan.md delete mode 100644 docs/2026-05-21_local-only-format/spec.md delete mode 100644 docs/2026-05-21_local-only-format/spec_notes.md diff --git a/docs/2026-05-21_local-only-format/01_format_command.md b/docs/2026-05-21_local-only-format/01_format_command.md deleted file mode 100644 index 1c5eda3a9b..0000000000 --- a/docs/2026-05-21_local-only-format/01_format_command.md +++ /dev/null @@ -1,166 +0,0 @@ -# SQLMesh Format CLI Command Reference - -This document provides a technical overview and architectural reference for how the `sqlmesh format` command is defined, processed, and executed within the SQLMesh codebase. - -## 1. CLI Entry Point & Click Command Definition -The `format` command is defined as a Click CLI command in the file **`sqlmesh/cli/main.py`** at lines **343–380**: - -```python -@cli.command("format") -@click.argument("paths", nargs=-1) -@click.option( - "-t", - "--transpile", - type=str, - help="Transpile project models to the specified dialect.", -) -@click.option( - "--check", - is_flag=True, - help="Whether or not to check formatting (but not actually format anything).", - default=None, -) -@click.option( - "--rewrite-casts/--no-rewrite-casts", - is_flag=True, - help="Rewrite casts to use the :: syntax.", - default=None, -) -@click.option( - "--append-newline", - is_flag=True, - help="Include a newline at the end of each file.", - default=None, -) -@opt.format_options -@click.pass_context -@error_handler -@cli_analytics -def format( - ctx: click.Context, paths: t.Optional[t.Tuple[str, ...]] = None, **kwargs: t.Any -) -> None: - """Format all SQL models and audits.""" - if not ctx.obj.format(**{k: v for k, v in kwargs.items() if v is not None}, paths=paths): - ctx.exit(1) -``` - -The command takes an optional positional argument `paths` to limit formatting to specific files or directories, and routes all keyword arguments down to `Context.format(...)`. - ---- - -## 2. The Execution Call Chain -When a user executes `sqlmesh format` in the terminal, the execution traverses the following call chain: - -1. **`sqlmesh/cli/main.py` (Line 343)**: The Click CLI parses options/arguments and invokes the `format(...)` command handler. -2. **`sqlmesh/cli/main.py` (Line 377)**: The CLI command delegates execution to the core context format method by calling `ctx.obj.format(...)`. Here, `ctx.obj` is an instance of the `Context` class. -3. **`sqlmesh/core/context.py` (Line 1220)**: `Context.format(...)` performs the orchestration: - - Filters down models and audits from the loaded context (`self._models.values()` and `self._audits.values()`) that are physical `.sql` files on disk (and match specified `paths` filters, if any). - - Skips targets where `target.formatting is False` (configured in model/audit metadata). - - Iterates through the files, opening each one to read its content as `before` (Line 1247). - - Calls `self._format(target, before, ...)` to generate the newly formatted content as `after`. -4. **`sqlmesh/core/context.py` (Line 1280)**: `Context._format(...)` parses and delegates rendering: - - Calls `parse(before, default_dialect=self.config_for_node(target).dialect)` to tokenize and parse the SQL file into a list of SQLGlot AST expressions (defined in `sqlmesh/core/dialect.py` at line 907). - - If transpilation is requested via `--transpile`, it updates the model's/audit's metadata dialect in-place within the first AST expression (the `MODEL` or `AUDIT` metadata block) (Lines 1290–1294). - - Resolves formatting configuration settings from the node's path config: `format_config = self.config_for_node(target).format` (Line 1296). - - Calls `format_model_expressions(expressions, ...)` to format the parsed expressions. -5. **`sqlmesh/core/dialect.py` (Line 754)**: `format_model_expressions(...)` performs the pretty-printing: - - If `rewrite_casts` is set (or configured by default), runs a recursive AST transformer (`cast_to_colon`) to rewrite `exp.Cast` AST nodes as double-colon cast nodes (`DColonCast` / `::`), unless they have custom dialect arguments (Lines 771–796). - - Executes `.sql(pretty=True, dialect=dialect, **kwargs)` on each AST expression to generate pretty-printed SQL string representations, forwarding general SQLGlot generator options. - - Joins multiple statements together with double newlines and semicolons (`";\n\n"`). -6. **`sqlmesh/core/context.py` (Lines 1259–1262)**: - - If the `--check` flag is active, compares `before` and `after`, and registers mismatched paths in `unformatted_file_paths` without modifying the disk. - - Otherwise, seeks back to `0`, writes `after` to the file, and truncates. - ---- - -## 3. Command Options & Configuration -The formatting CLI command accepts the following CLI arguments and flags: - -### Primary CLI Flags: -* **`paths`** (Positional, `nargs=-1`): Limits formatting check or update to specified file or directory paths. -* **`-t`, `--transpile TEXT`**: Transpiles models/audits to a specific target dialect (e.g., `snowflake`, `bigquery`), writing both formatted SQL and updated metadata back to the source files. -* **`--check`**: Evaluates formatting without modifying files. Exits with code `1` if any file needs reformatting. -* **`--rewrite-casts / --no-rewrite-casts`**: Toggles rewriting standard SQL `CAST(x AS TYPE)` syntax to the shorter `x::TYPE` syntax. -* **`--append-newline`**: Ensures a trailing newline is appended to the end of every formatted file. - -### Common SQLGlot Generator Flags (defined under `@opt.format_options` decorator in `sqlmesh/cli/options.py:60`): -* **`--normalize`**: Normalizes all unquoted identifiers to lowercase. -* **`--pad INTEGER`**: Determines padding size (spaces) used in formatting. -* **`--indent INTEGER`**: Determines indentation size (spaces) used in formatting. -* **`--normalize-functions TEXT`**: Formats function names to a specific case (`'upper'` or `'lower'`). -* **`--leading-comma`**: Formats SELECT list commas as leading instead of trailing. -* **`--max-text-width INTEGER`**: Sets maximum line character width before wrapping. - -### Configuration via Config File / Environment variables: -Options can be set in the main config file under `format` (mapped to `FormatConfig` in `sqlmesh/core/config/format.py`), or as model defaults (`ModelDefaultsConfig(formatting=False)` in `sqlmesh/core/config/model.py`), or through environment variables (e.g., `SQLMESH__FORMAT__LEADING_COMMA=true`). - ---- - -## 4. File Discovery & File Types Touched -The `format` command is strictly designed to discover and modify **SQL models and audits** (.sql files only). - -* **Discovery**: SQLMesh loads model definitions via its loader system. The standard loader `SqlMeshLoader` (`sqlmesh/core/loader.py:481`) globs SQL files recursively in the project's `models/` (`c.MODELS`) and `audits/` directories. -* **Filtering**: In `Context.format()`, targets are filtered using the list comprehension: - ```python - filtered_targets = [ - target - for target in chain(self._models.values(), self._audits.values()) - if target._path is not None - and target._path.suffix == ".sql" - and (not paths or any(target._path.samefile(p) for p in paths)) - ] - ``` -* **Excluded File Types**: The command does **not** touch Python models (`.py`), YAML model definitions (`.yaml`, `.yml`), macros, configurations, or schemas. - ---- - -## 5. Under-the-Hood AST Round-Trip -The formatting implementation performs a full **round-trip parsing** of the SQL source code: -1. **Source to AST**: The raw source string is parsed using `sqlmesh.core.dialect.parse`, which returns a list of SQLGlot AST node structures (`exp.Expr`). -2. **AST Mutation**: - - Updates meta-expressions (the `MODEL` or `AUDIT` block) to adjust dialect settings if transpilation is requested. - - Converts `Cast` AST nodes into `DColonCast` AST nodes for cast normalization. -3. **AST to Target SQL**: SQLGlot's `.sql(...)` generator produces the final formatted output from the modified AST using the configured dialect generator rules. -4. **Statement Stitching**: Statements are stitched back together using double newlines and semicolons. - -Because it is an AST-based round-trip, comments that are detached from AST nodes may be relocated or discarded, and invalid SQL syntax anywhere within a formatted file will result in parsing failures. - ---- - -## 6. Project Context & External Warehouse Dependencies -Running `sqlmesh format` **requires loading the full project context and connecting to the warehouse/state sync backend**. It is not a pure local formatting command. - -### Evidence: -1. **No Context Skip**: In `sqlmesh/cli/main.py` lines 25–43, `format` is **not** included in `SKIP_LOAD_COMMANDS` or `SKIP_CONTEXT_COMMANDS`. Therefore, `Context` is instantiated on command invocation with `load=True`. -2. **Full Project Loading**: Instantiating the context executes `context.load()`, which parses **every** script, model, and audit in the project recursively. A single syntax error in an unrelated model will block the formatting command from starting. -3. **Warehouse Connection/State Sync Hit**: - - Inside `Context.load()` (**`sqlmesh/core/context.py` at lines 677–689**), SQLMesh checks the production environment definition from state sync: - ```python - prod = self.state_reader.get_environment(c.PROD) - ``` - - Resolving `self.state_reader` forces the evaluation of the `self.state_sync` property (**`sqlmesh/core/context.py` at line 621**). - - If a state sync backend is uninitialized, the context evaluates `_new_state_sync()` (**line 2944**) which calls `_scheduler.create_state_sync(self)` to establish a connection to the configured database. - - Under database-backed state storage (e.g., Snowflake, BigQuery, Databricks), SQLMesh makes network/database connections to query state tables (and may run schema migrations if `schema_version == 0`). - - Consequently, running `sqlmesh format` requires valid warehouse credentials and an active database connection. - ---- - -## 7. Existing Format Tests -The following files cover format command behavior: - -* **`tests/core/test_format.py`**: - * `test_format_files`: Builds a temporary directory with multiple SQL models, audits, and inline audits. Asserts that formatting check detects unformatted files, verifies transpiling to BigQuery (rewriting both dialect metadata and query-specific syntax successfully), and validates formatting of nested/inline audits. - * `test_ignore_formating_files`: Validates that configuring `formatting false` in `MODEL` metadata, `AUDIT` metadata, or as a global configuration model default successfully skips formatting for those files. -* **`tests/cli/test_cli.py`**: - * `test_format_leading_comma_default`: Runs the CLI `sqlmesh format` command using a Click runner. Ensures environment variables (e.g., `SQLMESH__FORMAT__LEADING_COMMA`) are respected, and that explicit CLI flags (such as `--leading-comma`) override any environment variable configs. -* **`tests/core/test_dialect.py`**: - * `test_format_model_expressions`, `test_macro_format`, `test_format_body_macros`: Test that AST structures, custom macro syntaxes (e.g., `@WITH`, `@EACH`), comments, and lists are formatted into correct SQL syntax. -* **`tests/core/test_model.py` / `tests/core/test_audit.py`**: - * `test_formatting_flag_serde` / `test_audit_formatting_flag_serde`: Verify the `formatting` boolean field is excluded from JSON serializations but safely preserved across Pydantic models. - ---- - -## 8. Contributor Quirks & Non-Obvious Behavior -* **Unrelated Errors Block Execution**: Because the entire context is loaded on startup, any syntax/validation error in **any** file in the project will cause `sqlmesh format` to crash immediately—even if the user only specified a single valid file via the `paths` parameter. -* **In-place Metadata Modification**: Passing the `--transpile` flag will physically rewrite the `dialect` property within the model's SQL block `MODEL (...)` in the source file on disk. This is a mutating write that changes the dialect SQLMesh uses to load that model in all subsequent executions. -* **Credentials/Connection Dependency**: Due to loading state definitions during context initialization, local formatting can fail if database credentials are invalid or if there is no active internet connection to a remote data warehouse. diff --git a/docs/2026-05-21_local-only-format/02_cli_bootstrap.md b/docs/2026-05-21_local-only-format/02_cli_bootstrap.md deleted file mode 100644 index d8581fd055..0000000000 --- a/docs/2026-05-21_local-only-format/02_cli_bootstrap.md +++ /dev/null @@ -1,169 +0,0 @@ -# Research: CLI Bootstrap and Lifecycle - -## Scope -This reference document investigates the general setup, initialization flow, and bootstrap sequence that execute upon invoking the `sqlmesh` or `sqlmesh_cicd` command-line interfaces. It covers CLI entry points, global options, configuration loading, Context instantiation, database and state sync connection lifecycles, and external touchpoints (such as telemetry, disk access, and external APIs). - -Specifically, this document analyzes the behavior of the `sqlmesh format` command to clarify what is always, sometimes, or never performed during its execution, focusing on whether a live database or state sync connection is required. - -## Summary -* **Entry Points:** CLI tools are defined as console scripts in `pyproject.toml`, mapping `sqlmesh` to `sqlmesh.cli.main:cli` and `sqlmesh_cicd` to `sqlmesh.cicd.bot:bot`. -* **Global CLI Parsing:** Top-level global options (such as paths, configs, gateways, and custom `.env` files) are parsed at the click group level before subcommands execute. -* **Configuration Discovery:** Configuration files (`config.py`, `config.yaml`, `sqlmesh.yaml`) are discovered from project paths and user folders (`~/.sqlmesh/`), combined with environment variables starting with `SQLMESH__`. -* **Eager vs. Lazy Loading:** By default, subcommands initialize the `Context` with `load=True`, eagerly parsing local model files. Certain commands skip context construction (`init`, `ui`) or set `load=False` (`clean`, `run`, `migrate`). -* **Lazy DB Connections:** `EngineAdapter` connections are inherently lazy; the database client is initialized via a connection factory and only connects when a query or cursor is requested. -* **State Sync Eagerness:** For `load=True` commands, the bootstrap process queries the state sync backend for the schema version and production environment, eagerly resolving the lazy database connection. -* **Implicit Telemetry:** An anonymized telemetry collector runs in a daemon thread on every invocation, flushing run analytics to Tobiko Cloud on command completion unless disabled via environment variables. -* **`sqlmesh format` Connection Requirement:** Because `format` runs with a fully loaded Context (`load=True`), it **always** attempts to establish a live connection to the state sync backend database. It will throw an exception and fail if the state database is unreachable. - ---- - -## Findings - -### 1. CLI Entry Points & Top-Level Click Groups - -The CLI entry points are defined under `[project.scripts]` in `pyproject.toml:112-116`: -* `sqlmesh = "sqlmesh.cli.main:cli"` -* `sqlmesh_cicd = "sqlmesh.cicd.bot:bot"` - -#### The `sqlmesh` Command Group -The main `sqlmesh` command-line interface is governed by a Click group defined at `sqlmesh/cli/main.py:55`. The entry point function is `cli(...)` at `sqlmesh/cli/main.py:94`. - -On invocation, `cli(...)` handles global environment setup and console configuration: -1. **Logging & Console Setup:** Configures Python logging and Rich-based console output formatting (`sqlmesh/cli/main.py:112-116`). -2. **Context Skip Audits:** If the subcommand is in `SKIP_CONTEXT_COMMANDS = ("init", "ui")` and exactly one path is provided, it immediately sets `ctx.obj` to the absolute path of the project and returns (`sqlmesh/cli/main.py:120-123`), bypassing configuration loading and Context creation entirely. -3. **Config Loading:** Discovers and loads configuration files for the project path(s) (`sqlmesh/cli/main.py:127`). -4. **Log Retention Cleanup:** Deletes older log files in the specified log directory that exceed the maximum log retention limit configured via `log_limit` (`sqlmesh/cli/main.py:128-130`). -5. **Context Construction:** Instantiates the global `Context` (`sqlmesh/cli/main.py:132-137`). - -#### The `sqlmesh_cicd` Command Group -The CI/CD bot is governed by a Click group defined at `sqlmesh/cicd/bot.py:14`. Its entry function is `bot(...)`. It receives project path and configuration arguments, sets up logging to stdout, and stores CLI options in `ctx.obj` (`sqlmesh/cicd/bot.py:25-28`) before delegating to subcommands like `github` (`sqlmesh/integrations/github/cicd/command.py:21`). - -### 2. Global CLI Options and Flags -Global flags are defined as decorators on the Click group inside `sqlmesh/cli/main.py:56-93`: -* `--paths` / `-p` (`sqlmesh/cli/options.py:8`): Multiple path strings pointing to SQLMesh projects. Defaults to `[os.getcwd()]`. -* `--config` (`sqlmesh/cli/options.py:16`): The name of the Python configuration object to load if `config.py` is used. -* `--gateway` (env: `SQLMESH_GATEWAY`): Specifies the gateway configuration to utilize. -* `--ignore-warnings` (env: `SQLMESH_IGNORE_WARNINGS`): Suppresses execution warnings. -* `--debug`: Enables detailed logging and displays full tracebacks on failures. -* `--log-to-stdout`: Prints application logs to stdout. -* `--log-file-dir`: Specifies the folder to write logs to. Defaults to `.logs/` inside the project. -* `--dotenv` (env: `SQLMESH_DOTENV_PATH`): Specifies a custom `.env` file to load. - -### 3. Project Configuration Loading & Precedence -SQLMesh configuration loading is orchestrated by `load_configs(...)` in `sqlmesh/core/config/loader.py:29`. The loading process obeys a strict sequence: - -1. **Dotenv Loading:** If a custom `.env` file is specified via the `--dotenv` CLI option, it is loaded. Otherwise, the loader searches for a `.env` file directly under each discovered absolute project path and loads it using `python-dotenv` with `override=True` (`sqlmesh/core/config/loader.py:44-51`). -2. **Personal Overrides:** The loader searches the user's personal settings directory at `~/.sqlmesh/` for configuration files matching `YAML_CONFIG_FILENAMES` (defined as `config.yml`, `config.yaml`, `sqlmesh.yml`, `sqlmesh.yaml` in `sqlmesh/core/config/common.py:15`). If a personal configuration exists, it parses it and loads environment variables declared under the `env_vars` key (`sqlmesh/core/config/loader.py:58-63`). -3. **Project Configuration Resolution:** For each project path, the configuration is resolved via `load_config_from_paths(...)` (`sqlmesh/core/config/loader.py:76`): - * **YAML Configs:** Scans project directories for files matching `ALL_CONFIG_FILENAMES` (`config.py`, `config.yml`, `config.yaml`, `sqlmesh.yml`, `sqlmesh.yaml`). If YAML configurations are found, they are parsed via `load_config_from_yaml` (`sqlmesh/core/config/loader.py:116`). - * **Python Module Configs:** If `config.py` exists, it is dynamically imported and evaluated via `load_config_from_python_module`, matching the specified configuration variable name (`sqlmesh/core/config/loader.py:121`). -4. **Environment Variables (`SQLMESH__`):** Environment variables are read via `load_config_from_env()` (`sqlmesh/core/config/loader.py:251`). Any environment variable prefixed with `SQLMESH__` (case-insensitive) is split by double underscores `__`. These segments are parsed and nested into a dictionary which overrides the loaded file-based configurations (e.g., `SQLMESH__DEFAULT_GATEWAY=dev` is parsed to `{"default_gateway": "dev"}`). - -### 4. Context Instantiation & The `load` Flag -When the `Context` is instantiated (`sqlmesh/core/context.py:376`), it executes the initialization of its parent class `GenericContext`. The bootstrap flow inside `GenericContext.__init__` performs the following work: - -1. **Configuration Merging:** Aggregates and loads configurations for all specified project paths. -2. **DAG & Cache Setup:** Creates the DAG holder, model caches, metadata indexes, and registers notification targets. -3. **Analytics Opt-out:** Checks if `disable_anonymized_analytics` is configured in the project settings, disabling the telemetry collector if set (`sqlmesh/core/context.py:423-424`). -4. **Gateway & Scheduler Mapping:** Identifies the selected gateway (defaulting to `default_gateway_name` or overriden via the `--gateway` CLI option) and builds the Scheduler configuration (`sqlmesh/core/context.py:427-430`). -5. **Project Parsing and State Check:** If `load=True`, the initialization calls `self.load()` (`sqlmesh/core/context.py:472`). - -#### The Eager Loading Flow (`self.load()`) -When `load` is `True` (the default), `self.load()` performs heavy, eager cross-cutting operations (`sqlmesh/core/context.py:629`): -1. **Local Project Load:** Invokes the registered loaders (e.g., `Loader` or custom converters) to parse and load all local Python/SQL models, audits, macros, and requirements from disk. -2. **State Sync Interaction:** It accesses `self.state_reader` on line 678 to retrieve the production environment state: - ```python - prod = self.state_reader.get_environment(c.PROD) - ``` - This retrieval triggers the `state_sync` property on `GenericContext` (`sqlmesh/core/context.py:609-620`), which boots the state sync backend. - -#### State Sync Boot & Version Validation -Retrieving `self.state_sync` initializes the state sync engine adapter and triggers version verification (`sqlmesh/core/context.py:612-615`): -```python -if self._state_sync.get_versions(validate=False).schema_version == 0: - self.console.log_status_update("Initializing new project state...") - self._state_sync.migrate() -self._state_sync.get_versions() -``` -* `_new_state_sync()` is called, which creates the state sync engine adapter. -* `get_versions(validate=False)` actively queries the database state schema's versions table. -* If the versions table does not exist or has a schema version of `0`, the state sync eagerly runs migrations (`migrate()`) to set up standard tables (`_snapshots`, `_environments`, `_versions`, `_intervals`). -* `get_versions()` is called a second time to validate that the remote schema version is fully compatible with the current running version of SQLMesh. - -Because `self.load()` queries the state sync backend, **any command initializing a Context with `load=True` eagerly resolves a database connection to query the remote state database on bootstrap.** - -### 5. CLI Command Lightweight Paths -Different commands use lightweight or bypassed paths depending on whether they require full project files or database access. - -| Command(s) | Category | Bypasses Context? | Bypasses Eager `load()`? | Description / Cites | -| :--- | :--- | :--- | :--- | :--- | -| `init`, `ui` | `SKIP_CONTEXT_COMMANDS` | **Yes** | **Yes** | Bypasses all config discovery and Context creation. `ctx.obj` is assigned the absolute directory path string (`sqlmesh/cli/main.py:43`). | -| `clean`, `create_external_models`, `destroy`, `environments`, `invalidate`, `janitor`, `migrate`, `rollback`, `run`, `table_name` | `SKIP_LOAD_COMMANDS` | **No** | **Yes** | `load` is set to `False` (`sqlmesh/cli/main.py:31-42`). Instantiates `Context` but skips running `self.load()` on initialization. Avoids loading model files and querying state sync during context creation (though the subcommands may query the state sync during execution). | -| `format`, `render`, `plan`, `diff`, `evaluate`, etc. | Standard Commands | **No** | **No** | Requires a fully loaded context. Eagerly reads all local model files, builds the model DAG, and connects to the state sync backend on boot. | - -### 6. Database and State Sync Connection Lifecycles -Connection handling in SQLMesh is designed to be highly lazy, preventing unnecessary connections until queries must run. However, the boot sequence forces this laziness to resolve immediately for standard commands. - -#### Lazy Connection Construction -1. **Engine Adapter Init:** The `EngineAdapter` is created by `create_engine_adapter()` inside `sqlmesh/core/config/connection.py:167`. Instead of receiving a live connection object, it receives `self._connection_factory_with_kwargs`—a callable factory (`sqlmesh/core/config/connection.py:155`). -2. **Connection Pool Wrapping:** Inside the `EngineAdapter` constructor (`sqlmesh/core/engine_adapter/base.py:126`), the connection factory is wrapped inside a lazy `ConnectionPool` subclass (`SingletonConnectionPool`, `ThreadLocalConnectionPool`, or `ThreadLocalSharedConnectionPool`) created via `create_connection_pool` (`sqlmesh/utils/connection_pool.py:342`). -3. **Deferred Execution:** The connection pool does not execute the connection factory callable until a cursor is explicitly requested (e.g. calling `self._connection_pool.get_cursor()` inside `cursor` property in `sqlmesh/core/engine_adapter/base.py:194`). - -#### State Sync Resolution -The state sync backend is initialized via `EngineAdapterStateSync` at `sqlmesh/core/state_sync/db/facade.py:74`. It holds its own lazy engine adapter. -However, because the `state_sync` property on `Context` eagerly executes `get_versions(validate=False)` on boot, it performs a SQL lookup (`sqlmesh/core/state_sync/db/version.py:61`): -```python -if not self.engine_adapter.table_exists(self.versions_table): - return no_version -``` -This forces `self.engine_adapter` to fetch a cursor, invoking the lazy connection factory. Consequently, **the state sync database connection is eagerly established during CLI boot for all standard commands.** - -### 7. Implicit External Touchpoints -On any standard CLI invocation, SQLMesh touches the local filesystem, environment, and remote services: -* **Filesystem (Project):** Reads `.env` files, parses model/audit/macro files, and writes log files under `.logs/`. Cleans up excess log files using `remove_excess_logs()` (`sqlmesh/__init__.py:171`). -* **Filesystem (Global):** Searches `~/.sqlmesh/` for personal configurations. -* **Implicit Telemetry:** On import of `sqlmesh.core.analytics`, a global telemetry `collector` is initialized. If `SQLMESH__DISABLE_ANONYMIZED_ANALYTICS` is not `"true"`, it launches a background thread with an `AsyncEventDispatcher` (`sqlmesh/core/analytics/__init__.py:29`). The command-run telemetry events are queued and, upon command completion (`atexit`), flushed via a network HTTP POST request to `https://analytics.tobikodata.com/v1/sqlmesh/` (`sqlmesh/core/analytics/dispatcher.py:19`). -* **Implicit Cloud Authentication:** No global, implicit authentication flows are run on general CLI boot. However, if the state sync or project config is configured with an interactive OAuth database (such as MotherDuck without a token, or Snowflake SAML), the lazy-connection resolution on boot will trigger browser-based interactive authentication prompts (`sqlmesh/core/config/connection.py:292`). -* **CI/CD Bot Network Touchpoints:** When invoking `sqlmesh_cicd github`, `GithubController.__init__` eagerly queries the GitHub API over the network via PyGithub to retrieve repository, issue, and pull request data, along with PR review approvals (`sqlmesh/integrations/github/cicd/controller.py:317-329`). - ---- - -## Behavior of `sqlmesh format` - -The table below breaks down what is performed by the `sqlmesh format` command, detailing the underlying mechanisms. - -| Step | Status | Mechanism / Cite | -| :--- | :--- | :--- | -| **Discover & Load Config** | **Always** | Parses `.env` files, searches `~/.sqlmesh/`, loads YAML/Python project configurations, and merges environment overrides (`sqlmesh/core/config/loader.py:29`). | -| **Telemetry Dispatch** | **Always** | Telemetry is initialized, queuing CLI command-run details and attempting to flush to `https://analytics.tobikodata.com/v1/` on completion (`sqlmesh/core/analytics/__init__.py`). | -| **Initialize Context** | **Always** | Instantiates a full `Context` object with `load=True` (`sqlmesh/cli/main.py:132`). | -| **Parse Local Models** | **Always** | Reads and parses all local SQL/Python model files and audits inside the project directory to construct the metadata DAG (`sqlmesh/core/context.py:629`). | -| **Database Connection** | **Always** | Connects to the **state sync database** backend to query `get_versions` and retrieve the `prod` environment configuration (`sqlmesh/core/context.py:609`, `sqlmesh/core/state_sync/db/version.py:61`). | -| **Interactive Login** | **Sometimes** | Prompts for browser authentication if the state sync backend utilizes an engine requiring interactive login (e.g. MotherDuck, BigQuery/Snowflake OAuth) and credentials are not pre-supplied (`sqlmesh/core/config/connection.py:292`). | -| **Metadata Migration** | **Sometimes** | Performs auto-migrations on the state database schema if the remote state schema version is `0` (`sqlmesh/core/context.py:613`). | -| **Data Warehouse Connection** | **Never** | Does not connect to or query the target data warehouse connection (if separate from the state connection) for executing model queries or performing DDL commands. | -| **Run Unit Tests** | **Never** | Bypasses testing hooks and does not evaluate any defined unit tests. | -| **Mutate Physical Tables** | **Never** | Does not perform DDL operations or alter physical user tables in the warehouse. | - -### Why `sqlmesh format` Requires a Live State Connection -A standard reader might assume that code formatting is a pure text-transformation utility that should operate offline on individual files. However, in SQLMesh, `sqlmesh format` does not bypass the standard context bootstrap. - -Because `format` requires the context to be initialized with `load=True`, the initialization flow calls `self.load()`. During this process, `self.load()` merges local files with environment metadata stored in the state sync database. To do this, it invokes `self.state_reader.get_environment(c.PROD)`. This forces SQLMesh to instantiate the state sync backend, verify metadata schema versions, and issue queries to the state database. - -As a result, **`sqlmesh format` cannot run in offline/isolated environments where the state sync database backend is unreachable.** If the database is down, offline, or behind a VPN, the command will crash on bootstrap prior to formatting any files. - ---- - -## Key References -* `pyproject.toml:112-116` — Configuration of standard CLI console script entry points. -* `sqlmesh/cli/main.py:94` — Main Click `cli(...)` entry point function. -* `sqlmesh/cli/main.py:31` & `43` — Definitions of bypassed subcommands (`SKIP_LOAD_COMMANDS` and `SKIP_CONTEXT_COMMANDS`). -* `sqlmesh/core/config/loader.py:29` — Main configuration loader `load_configs(...)`. -* `sqlmesh/core/context.py:376` — Context constructor `GenericContext.__init__` handling configuration mapping. -* `sqlmesh/core/context.py:609-620` — `state_sync` property logic executing eager version check and auto-migrations. -* `sqlmesh/core/config/scheduler.py:67` — Eager engine adapter construction and state sync instantiation. -* `sqlmesh/core/config/connection.py:167` — Dynamic creation of lazy engine adapters. -* `sqlmesh/utils/connection_pool.py:342` — Thread-safe connection pool delaying connection factory invocation. -* `sqlmesh/core/state_sync/db/version.py:61` — `get_versions()` executing version table queries on the database. -* `sqlmesh/core/analytics/__init__.py:29` — Background telemetry dispatcher initialization and flush routines. -* `sqlmesh/integrations/github/cicd/controller.py:286` — `GithubController` initializing and querying GitHub API resources. diff --git a/docs/2026-05-21_local-only-format/03_contributing.md b/docs/2026-05-21_local-only-format/03_contributing.md deleted file mode 100644 index cd444ed7cd..0000000000 --- a/docs/2026-05-21_local-only-format/03_contributing.md +++ /dev/null @@ -1,230 +0,0 @@ -# Contributing to SQLMesh - -This reference guide details the standards, local environments, testing frameworks, and submission workflows required when contributing to SQLMesh. Since you have already forked the repository and started a development branch (`fix/local-only-format`), this document focuses on what happens from this point forward. - ---- - -## 1. Core Contribution Principles - -SQLMesh is a Linux Foundation project and is licensed under the **Apache License 2.0**. Understanding community expectations and legal baselines helps avoid friction during your pull request (PR). - -### Community Policies -* **Code of Conduct (`CODE_OF_CONDUCT.md`)**: SQLMesh adheres to the [LF Projects Code of Conduct](https://lfprojects.org/policies/code-of-conduct/). All community participants are expected to uphold these standards. -* **General Expectations (`CONTRIBUTING.md`)**: Outlines project roles (Contributors, Maintainers, and the Technical Steering Committee), file licensing expectations, and general contribution workflow steps. - -### Legal Safeguards -* **Developer Certificate of Origin (DCO)**: All commits must be signed off to certify that you have the right to submit the code under the project's license. Your commit messages must include a `Signed-off-by` line. - * *How to commit with sign-off:* Use the `-s` flag when committing: - ```bash - git commit -s -m "Your commit message" - ``` - * *How to sign off existing commits:* - * Amending the latest commit: `git commit --amend -s` - * Rebasing multiple commits: `git rebase HEAD~N --signoff` -* **License Headers**: Any new file introduced to the codebase must begin with the SPDX license header: - ```python - # SPDX-License-Identifier: Apache-2.0 - ``` - ---- - -## 2. Developer Environment Setup - -The primary configuration for SQLMesh is defined in the root `pyproject.toml` and orchestrated using the project's `Makefile`. - -### Prerequisites -* **Python**: `>= 3.9` and `< 3.13` (Python 3.13+ is not yet supported). -* **Java**: OpenJDK `>= 11` (required for certain engine tests such as Spark). -* **Docker & Docker Compose V2**: Required for running containerized database engines. - -### Environment Setup Commands -Always work within a virtual environment. Use these commands (detailed in `docs/development.md` and `Makefile`): - -1. **Create and Activate Virtual Environment**: - ```bash - python -m venv .venv - source .venv/bin/activate # On Windows: .venv\Scripts\activate - ``` -2. **Install Development Dependencies**: - ```bash - make install-dev - ``` - This targets the `dev` extra in `pyproject.toml` (`[project.optional-dependencies]`) along with web, slack, dlt, and lsp extras: - ```bash - pip install -e ".[dev,web,slack,dlt,lsp]" ./examples/custom_materializations - ``` -3. **Setup Pre-Commit Hooks**: - ```bash - make install-pre-commit - ``` - This registers the pre-commit configuration in `.pre-commit-config.yaml` with git. - ---- - -## 3. Local Verification (Run Before You Push) - -To maintain a clean main branch, contributors must verify code style, static typing, and basic test suites locally before pushing their branch. - -### Linting and Formatting -SQLMesh uses **Ruff** for linting/formatting and **MyPy** for static type checks, automated via `pre-commit` hooks. - -* **Style Verification**: Run the style command before every push: - ```bash - make style - ``` - This executes `pre-commit run --all-files`, which runs: - * `ruff check --force-exclude --fix --ignore E721 --ignore E741` on python files. - * `ruff format --force-exclude --line-length 100` for PEP 8 compliance. - * `mypy` for static typing (configured in `pyproject.toml` to enforce `disallow_untyped_defs = true` on the core library, with relaxed rules for tests and migrations). - * `valid migrations` using `tooling/validating_migration_numbers.sh` to ensure migration sequences are consecutive and have no gaps or overlaps. -* **Python-Only Style**: If you only touched python files and want to skip node/frontend linters, run: - ```bash - make py-style - ``` - ---- - -## 4. Testing Framework and Conventions - -The SQLMesh test suite lives in the `tests/` directory. Tests are categorised and run using `pytest` markers (defined in `pyproject.toml`). - -### Organization of Tests -* **Unit & Integration Tests**: Organized within subdirectories under `tests/` matching core modules (e.g., `tests/core/`, `tests/dbt/`, `tests/web/`, etc.). -* **Engine-Specific Tests**: Integration tests that verify SQLMesh's behavior against physical databases. - -### Pytest Markers -Pytest markers control execution speed and environment requirements. Key markers include: -* `fast`: Tests that execute rapidly without database interactions (default when no markers are supplied). -* `slow`: Tests involving local DB file/memory interactions (such as DuckDB). -* `docker`: Tests that require running localized Docker containers. -* `remote`: Tests that interact with cloud database endpoints. -* `isolated` / `dialect_isolated` / `registry_isolation`: Tests that must be run sequentially or separately to avoid shared state contamination. - -### Running Tests Locally -Depending on the scope of your changes, use the corresponding Makefile target: - -* **Fast Suite**: Run unit and quick tests: - ```bash - make fast-test - ``` - *Maps to: `pytest -n auto -m "fast and not cicdonly" ...` followed by isolated test suites.* -* **Slow Suite**: Run all local unit and integration tests (including slow ones): - ```bash - make slow-test - ``` - *Maps to: `pytest -n auto -m "(fast or slow) and not cicdonly" ...` followed by isolated test suites.* -* **Doctests**: Verify examples embedded in docstrings: - ```bash - make doc-test - ``` -* **Engine-Specific Unit Tests**: - * **DuckDB**: Run unit tests targeting DuckDB adapter: - ```bash - make duckdb-test - ``` - * **Dockerized Engines** (Postgres, MySQL, MSSQL, ClickHouse, Trino, Spark, RisingWave): - To run these, you must bring the engine up in Docker first, run the tests, and optionally tear down: - ```bash - # Bring up Postgres in Docker and run tests - make engine-postgres-up - make postgres-test - make engine-postgres-down - ``` - *Note: Individual engine targets automatically execute standard Docker Compose files located under `tests/core/engine_adapter/integration/docker/`.* - * **Cloud Engines** (Snowflake, BigQuery, Databricks, Redshift, Athena, Fabric, GCP-Postgres): - These require passing corresponding credentials via environment variables (e.g., `SNOWFLAKE_ACCOUNT`, `BIGQUERY_KEYFILE`, etc.) and executing the respective targets (e.g., `make snowflake-test`). - ---- - -## 5. Agent & AI-Assisted Workflow - -SQLMesh contains explicit expectations for contributors using AI assistants or agents (detailed in `CLAUDE.md`). The project relies on a structured, Test-Driven Development (TDD) cycle. - -### The Agent Loop -When implementing features or bug fixes, you should navigate the code in a systematic loop: -1. **Understand**: Explore the codebase, review existing design patterns, and read relevant GitHub issues. -2. **TDD (Failing Test First)**: *Always begin by writing a failing test (or tests)* that reproduces the bug or asserts the expected behavior of the new feature before editing the source files. -3. **Implement**: Make the smallest surgical change that solves the issue. Avoid speculative coding. -4. **Code Review**: Hand off your implementation to an automated review or use a code-reviewer agent to identify edge cases, test coverage gaps, and style compliance. -5. **Iterate**: Fix any review items, verify the tests pass, and proceed. -6. **Document**: Write or update user-facing documentation for any user-visible behaviors. - ---- - -## 6. Commit and PR Workflow - -### Commit Conventions -Based on the repository's git history, commits follow a semantic naming convention. Maintainers merge PRs using squash-commits, so clean commit messages are highly valued. -* **Prefixes**: Use semantic prefixes to describe the nature of your change: - * `fix:` or `Fix:` for bug fixes. - * `feat:` or `Feat:` for new features. - * `chore:` or `Chore:` for dependency bumps, configuration, or structural updates. - * `docs:` or `(docs):` for documentation edits. - * Add a `!` prefix (e.g., `Chore!:`) for breaking changes. -* **Engine Scopes**: If the change is engine-specific, denote the engine in parentheses: - * `Fix (databricks): use shared connection pool...` -* **PR Reference**: Keep commit titles clean. When merging, the PR number is appended (e.g., `(#5801)`). -* *Mandatory Requirement:* Every commit must contain the DCO sign-off (`Signed-off-by`). - -### Submission Process -1. **Branch Target**: All pull requests must be opened against the `main` branch of `sqlmesh/sqlmesh`. -2. **PR Template**: Fill out `.github/pull_request_template.md` completely. It requires: - * **Description**: A clear summary of the changes. - * **Test Plan**: Specific steps on how you tested the changes. - * **Checklist**: Check boxes certifying you ran `make style`, added tests, verified `make fast-test`, and signed off all commits. -3. **Reviewers & CODEOWNERS**: There is no explicit `CODEOWNERS` file in the repository. Instead, community maintainers (such as Alexander Butler `z3z1ma`, Toby Mao `tobymao`, or Yuki Kakegawa `StuffbyYuki`) manually review and assign reviewers. - -### Required CI Checks (GitHub Actions) -The PR workflow (`.github/workflows/pr.yaml`) runs an extensive validation suite: -* **`doc-tests`**: Runs `make doc-test` to verify docstring examples. -* **`style-and-cicd-tests`**: Runs `make py-style`, `make benchmark-ci`, and `make cicd-test` across a Python version matrix (`3.9`, `3.10`, `3.11`, `3.12`, `3.13`) on Ubuntu. -* **`cicd-tests-windows`**: Runs `make fast-test` on Windows. -* **`migration-test`**: Validates migrations by running integration tests under database schema upgrades using `./.github/scripts/test_migration.sh`. -* **`ui-test`**: Runs frontend unit and Playwright e2e tests for the web UI. -* **`engine-tests-docker`**: Executes test suites for containerized engines (`duckdb`, `postgres`, `mysql`, `mssql`, `trino`, `spark`, `clickhouse`, `risingwave`). -* **`test-dbt-versions`**: Validates compatibility across multiple dbt versions (`1.3` through `1.10`) using `make dbt-fast-test`. - ---- - -## 7. Documentation Contributions - -If your changes alter user-visible APIs, configurations, or behaviors, you must update the documentation (written in connected prose following `docs/HOWTO.md`). - -### Structure and Location -* Documentation markdown files live in the `docs/` folder. -* Configuration guides are located under `docs/guides/configuration.md`. -* Integration-specific documents are located under `docs/integrations/`. - -### Previewing Documentation -1. **Install Docs Dependencies**: - ```bash - make install-doc - ``` - *If version conflicts occur, run the fallback command:* - ```bash - pip install mkdocs mkdocs-include-markdown-plugin mkdocs-material mkdocs-material-extensions mkdocs-glightbox pdoc - ``` -2. **Serve Locally**: - ```bash - make docs-serve - ``` - This launches a local MkDocs development server at `http://127.0.0.1:8000/` that hot-reloads as you edit markdown files. - -### Style and Tone Guidelines -* **Cognitive Load**: SQLMesh is powerful and complex; focus on minimizing cognitive load for readers. -* **Instructional Voice**: Use the second-person voice for instructions ("Add an audit...", "You can partition..."). -* **Narrative Voice**: Use the first-person plural for walk-throughs or narratives ("First, we create a new environment..."). -* **Tone**: Prefer active voice over passive voice, and keep sentences short. -* **Review Flow**: For larger doc additions, the Technical Writer/Editor (Trey) may perform a full review and editing pass by opening a PR directly *against your branch* before team approval. - ---- - -## 8. Summary Checklist for Contributors - -Before pushing and submitting your PR, ensure you can tick off all items on this list: -- [ ] No new files are missing the Apache 2.0 license header. -- [ ] All commits are signed off (`git commit -s`). -- [ ] Code is formatted and typed (`make style`). -- [ ] No gaps or overlaps exist in any newly added migration numbers. -- [ ] The local fast test suite passes (`make fast-test`). -- [ ] The `.github/pull_request_template.md` is fully completed. diff --git a/docs/2026-05-21_local-only-format/plan.md b/docs/2026-05-21_local-only-format/plan.md deleted file mode 100644 index 8034ae58a1..0000000000 --- a/docs/2026-05-21_local-only-format/plan.md +++ /dev/null @@ -1,149 +0,0 @@ -# Plan: local-only `format` and `lint` - -## Goal - -Make `sqlmesh format` and `sqlmesh lint` runnable with no state-sync credentials and no reachable database, by adding a `load_state: bool = True` parameter to `GenericContext` that gates the remote-snapshot merge block in `Context.load()`, and a `LOCAL_ONLY_COMMANDS = ("format", "lint")` tuple in `cli/main.py` that flips it to `False` for those two commands. Everything else stays as it is. - -## Overall validation - -The PR is done when all of these are true: - -- A `Context` constructed against a project whose state connection points at an unreachable host completes both `context.format()` and `context.lint_models()` without raising when `load_state=False` is passed. -- `sqlmesh format` and `sqlmesh lint` invoked through the CLI runner against a project whose `config.yaml` declares an unreachable state connection complete with `exit_code == 0`. `EngineAdapterStateSync.get_versions` is never called during the run (verified by `mocker.patch` with `assert_not_called()`). -- A guard-rail CLI test confirms `sqlmesh plan` against the same project *does* call `EngineAdapterStateSync.get_versions` — the new tuple didn't accidentally cover commands it shouldn't. -- `make py-style` is clean. Ruff, mypy, and the migration-sequence check all pass. -- `make fast-test` is green (covers the new Context-level tests). -- The targeted CLI test passes when run directly (`pytest tests/cli/test_cli.py::test_format_runs_without_state -v` and the same for lint). `make slow-test` is green overall. -- Every commit on the branch carries a `Signed-off-by:` trailer per the DCO bot. `git log --format='%(trailers:key=Signed-off-by)' main..HEAD` shows one for each commit. - -## Key discoveries & assumptions - -**Facts** (each verified during exploration): - -- The state-merge block in `Context.load()` is two consecutive `if any(self._projects):` guards at `sqlmesh/core/context.py:677-699`. The first loads environment statements from prod (lines 677-683); the second populates the local `uncached` set and merges remote snapshots into `self._models` / `self._standalone_audits` (lines 685-699). Both touch state via `self.state_reader.get_environment(c.PROD)`. -- `GenericContext.__init__` lives at `sqlmesh/core/context.py:376` with the existing `load: bool = True` parameter at line 385. `self.load()` is called at the very end of `__init__` if `load` is true (line 473). -- The CLI top-level group `cli(...)` lives at `sqlmesh/cli/main.py:94`. The `SKIP_CONTEXT_COMMANDS` and `SKIP_LOAD_COMMANDS` tuples are at `cli/main.py:31` and `cli/main.py:43`. The Context construction call is at `cli/main.py:132-137`. -- Existing format tests live at `tests/core/test_format.py` (no module-level mark — runs under `fast`). They construct a `Context` directly with `Config()` and `tmp_path` (e.g. `test_format_files` at line 14). -- Existing linter tests live at `tests/core/linter/test_builtin.py` (also no module-level mark — fast). They use the `copy_to_temp_path` fixture from `tests/conftest.py:565` against `examples/sushi`. -- CLI runner tests live at `tests/cli/test_cli.py`. The module is marked `pytestmark = pytest.mark.slow` at line 23. The runner fixture is at line 31, returning `CliRunner(env={"COLUMNS": "80"})`. `create_example_project` helper is at line 36. -- The `Linter` is built per-project in `Context.load()` at `context.py:670-674` before the state-merge block. Linters don't depend on the state-merge having run. -- `format` and `lint` Click handlers are at `sqlmesh/cli/main.py:343-380` and `sqlmesh/cli/main.py:1168-1185` respectively. Neither needs argument changes. - -**Decisions resolved during implementation:** - -- The plan originally suggested configuring an unreachable Postgres state connection. This doesn't work in the dev environment because `psycopg2` is not installed; `PostgresConnectionConfig` runs `_get_engine_import_validator("psycopg2", ...)` at config-validation time (`sqlmesh/core/config/connection.py:1424`) and raises before our code path is reached. Implementation uses the plan's documented fallback: patch `sqlmesh.core.state_sync.db.facade.EngineAdapterStateSync.get_versions` with `side_effect=RuntimeError(...)` and assert it is never called. -- The state-merge blocks are guarded by `if any(self._projects):`, where `self._projects = {config.project for config in self.configs.values()}`. `Config.project` defaults to `""` (`sqlmesh/core/config/root.py:142`), and `any({""})` is `False` — so a `Config()` literal short-circuits the guard before the new `self._load_state` term is evaluated, making the test vacuous. The Context-level tests therefore set a non-empty `project` (`Config(project="local_only")` in the format test; an inline rewrite of sushi's `config.py` to add `project="sushi"` in the linter test) to ensure the guard is actually exercised. -- No existing test exercised the "format/lint succeed despite broken state" path (verified by `rg 'format.*state\|lint.*state' tests/`). - -## Existing patterns & conventions to follow - -**Code patterns:** - -- **CLI command-class tuples.** `SKIP_LOAD_COMMANDS` and `SKIP_CONTEXT_COMMANDS` at `sqlmesh/cli/main.py:31,43` are plain module-level string tuples; the `cli(...)` group reads them inside `if ctx.invoked_subcommand in ...` checks. The new `LOCAL_ONLY_COMMANDS` follows the same shape and is checked in the same block (`cli/main.py:120-123`). -- **Boolean construction parameters.** `GenericContext.__init__` (`context.py:376-389`) uses plain `bool` parameters with `True`/`False` defaults — e.g. `load: bool = True`. Add `load_state: bool = True` in the same style, with a matching docstring entry in the class docstring at `context.py:362-373`. -- **Private instance storage.** Constructor flags are stored as private attributes prefixed with `_` (e.g. `self._loaded`, `self._loaders`, `self._models`). Store the new flag as `self._load_state`. -- **Guard composition.** Add to an existing `if`-guard with `and` rather than wrapping the block in a new outer `if`. Matches the style at `context.py:677` and keeps the diff minimal. - -**Test patterns:** - -- **Fast Context-level tests** construct a `Context` from a `tmp_path` plus a `Config(...)` literal (`tests/core/test_format.py:14-42`). No fixtures beyond `tmp_path` and `mocker` are needed. Tests are top-level `def test_*` functions, not classes. -- **Linter tests** use the `copy_to_temp_path` fixture (`tests/conftest.py:565`) and patch the project's `config.py` text to enable rules (`tests/core/linter/test_builtin.py:7-46`). The same fixture can be reused for the local-only lint test by writing a config with an unreachable state connection. -- **CLI tests** use the session-scoped `runner` fixture and the `create_example_project` helper (`tests/cli/test_cli.py:31-60`). The module-level `pytestmark = pytest.mark.slow` (line 23) inherits to every test added there. -- **Mocker assertions.** `tests/cli/test_cli.py` already uses `MagicMock` patterns. For "method never called", `mocker.patch(...)` followed by `mock.assert_not_called()` is the idiomatic shape in this repo (`rg "assert_not_called" tests/` shows ~20 hits). - -**Contribution conventions:** - -- **DCO sign-off.** Every commit needs `Signed-off-by:` — produced by `git commit -s`. The bot blocks merge otherwise. The docs commit on this branch already carries one. -- **Commit messages.** Recent commits use lowercase semantic prefixes (`fix:`, `chore:`, `docs:`) with imperative subject lines — see `git log --oneline -20` for the pattern. Combined with the global AGENTS.md format: subject ≤50 chars, body wrapped at 72 explaining why, trailers `Coding-Agent: pi` / `Model: ` / `Signed-off-by: ...`. -- **Pre-commit style.** `make py-style` runs ruff (check + format, line length 100), mypy, and the migration-sequence check. Run before each commit. If only Python changed, `make py-style` is faster than `make style` (which also runs prettier/eslint). -- **Test selection.** `make fast-test` runs the fast suite plus isolated groups. CLI tests in `tests/cli/test_cli.py` are slow-marked at the module level and require `make slow-test` for the full suite, but the targeted CLI test can be invoked directly via `pytest tests/cli/test_cli.py::` during development. -- **No new files unless necessary.** Per `03_contributing.md`, new Python files need the `# SPDX-License-Identifier: Apache-2.0` header. This plan adds no new source files — only modifies existing ones — so no header work needed. - -## Out of scope - -- **`dag` and any other CLI command.** Even though the `load_state` mechanism would make some of `dag` local-only, its `--select-model` path constructs a `Selector` that evaluates `self.state_reader` at construction time (`context.py:2947`). Fixing that requires touching `Selector` and every other `_new_selector` call site — separate change, not in this PR. -- **Refactoring `Context.load()` structure.** No extraction of the state-merge block into a private method, no rearrangement of the load sequence. The change is `and self._load_state` added to two existing guards. -- **Suppressing analytics or any other implicit network activity.** Anonymized analytics still fire on `format` and `lint` invocations after this change. That's governed by the existing `disable_anonymized_analytics` config knob and is orthogonal. -- **Tests for "command X still hits state."** No new test asserts that `plan`/`diff`/`run`/etc. continue to touch state. The default `load_state=True` preserves their behavior; the existing test suite already covers them. -- **Documentation updates.** The user-facing behavior change is "CI now works without state credentials." Discoverable from the PR description and release notes. No `docs/` page change in this PR. -- **Backporting or version-gating.** This is a forward-only change on `main`. No release-branch backport, no `@deprecated` shim, no feature flag. - -## Tasks - -Two tasks, two commits, one PR. - ---- - -### Task 1: Add `load_state` to Context and gate the state-merge block - -**Purpose:** Introduce the mechanism that lets a caller construct a `Context` that loads models but never touches the state backend. - -**Files:** -- Modify: `sqlmesh/core/context.py` -- Modify: `tests/core/test_format.py` -- Modify: `tests/core/linter/test_builtin.py` - -**Test cases** (red first, then implementation): - -`tests/core/test_format.py` (fast): -- *`test_format_without_state_load`*: Patch `EngineAdapterStateSync.get_versions` to raise `RuntimeError`. Place one `.sql` model under `models/`. Construct `Context(paths=tmp_path, config=Config(project="local_only"), load_state=False)` (non-empty `project` so `any(self._projects)` is truthy and the gate is exercised). Call `context.format(check=True)`. Assert the patched mock has `assert_not_called()`. Verified non-vacuous by flipping to `load_state=True` and confirming the test fails with the patched `RuntimeError`. - -`tests/core/linter/test_builtin.py` (fast): -- *`test_lint_without_state_load`*: Using `copy_to_temp_path("examples/sushi")`, rewrite the sushi `config.py` to add `project="sushi"` to the `Config(...)` call (so `any(self._projects)` is truthy). Patch `EngineAdapterStateSync.get_versions` to raise. Construct `Context(paths=[sushi_path], load_state=False)`. Call `context.lint_models(raise_on_error=False)`. Assert the patched mock has `assert_not_called()`. Verified non-vacuous the same way as the format test. - -**Implementation outline:** - -In `sqlmesh/core/context.py`: -- Extend the `GenericContext` class docstring args block at lines 356-368 with a one-line entry for the new parameter: a brief description noting it gates the remote-state merge inside `load()` and is only meaningful when `load=True`. -- Add `load_state: bool = True` to `GenericContext.__init__` at the *end* of the parameter list (after `selector`). Placing it last avoids shifting any existing positional arguments for callers outside this repo who may pass `users`, `config_loader_kwargs`, or `selector` positionally. -- Store as `self._load_state` on the instance during `__init__`, alongside the other private attributes (`self._loaded`, `self._loaders`, etc., around lines 396-415). -- In `Context.load()`, tighten the two `if any(self._projects):` guards at lines 677 and 685 by ANDing `self._load_state` into each predicate. No other changes to `load()` body. `update_schemas`, `Linter.from_rules`, and the `analytics.collector.on_project_loaded` call are unaffected. - -**Verification:** -- Automated: `pytest tests/core/test_format.py::test_format_without_state_load tests/core/linter/test_builtin.py::test_lint_without_state_load -v` — new tests pass. -- Automated: `pytest tests/core/test_format.py tests/core/linter/ -v` — existing tests in these files still pass. -- Automated: `make py-style` — ruff, mypy, migration-sequence all clean. -- Manual: read the diff of `context.py` and confirm the only non-test changes are: one docstring line, one constructor parameter, one attribute assignment, two `and` insertions. - -**Commit:** `git commit -s` with subject `feat: add load_state flag to Context` (37 chars). Trailers: `Coding-Agent: pi`, `Model: `, `Signed-off-by: ...`. - ---- - -### Task 2: Wire `LOCAL_ONLY_COMMANDS` in the CLI - -**Purpose:** Make `sqlmesh format` and `sqlmesh lint` construct their `Context` with `load_state=False`, completing the user-facing behavior. - -**Files:** -- Modify: `sqlmesh/cli/main.py` -- Modify: `tests/cli/test_cli.py` - -**Test cases** (red first, then implementation): - -`tests/cli/test_cli.py` (slow, via module-level `pytestmark`): - -All three tests share a setup factored into `_setup_local_only_project(tmp_path, mocker)`: -1. `create_example_project(tmp_path, template=ProjectTemplate.EMPTY)` to scaffold a real project. -2. Prepend `project: cli_test\n\n` to the generated `config.yaml` so `Config.project` is non-empty and `any(self._projects)` is truthy. Without this, the existing outer guard short-circuits regardless of `self._load_state` and the tests are vacuous. -3. Write one `.sql` model under `models/` so the CLI doesn't short-circuit with "no models found". -4. `mocker.patch("sqlmesh.core.state_sync.db.facade.EngineAdapterStateSync.get_versions", side_effect=RuntimeError("state should not be accessed"))`. (No `state_connection: postgres` block in `config.yaml` — same `psycopg2` problem as Task 1; YAML validation through Pydantic fires the import validator. The patch is the only mechanism.) - -- *`test_format_runs_without_state`*: Run setup. Invoke `runner.invoke(cli, ["--paths", str(tmp_path), "format"])` (no `--check` — lets format write in place, exit_code 0 whenever the call returned). Assert `result.exit_code == 0` and `mock.assert_not_called()`. -- *`test_lint_runs_without_state`*: Run setup. Invoke `runner.invoke(cli, ["--paths", str(tmp_path), "lint"])`. Assert `result.exit_code == 0` and `mock.assert_not_called()`. -- *`test_plan_still_loads_state`* (required guard-rail): Run setup. *Additionally* spy on `Context.__init__` via `mocker.spy(Context, "__init__")`. Invoke `runner.invoke(cli, ["--paths", str(tmp_path), "plan"], input="n\n")`. Assert that the spy was called and that every call's `load_state` kwarg was `True` (defaults to `True` when omitted). This is stronger than asserting `mock.called` on `get_versions` — a regression where someone added `"plan"` to `LOCAL_ONLY_COMMANDS` would still hit state later via `context.plan(...)`, so `get_versions.called` alone wouldn't catch it. The spy proves the Context constructor itself was called with `load_state=True` for `plan`. Verified by temporarily appending `"plan"` to `LOCAL_ONLY_COMMANDS`; the spy-based assertion fails. - -**Implementation outline:** - -In `sqlmesh/cli/main.py`: -- Add a new module-level constant `LOCAL_ONLY_COMMANDS = ("format", "lint")` immediately after `SKIP_CONTEXT_COMMANDS` (line 43), matching the surrounding tuple style. -- Inside `cli(...)` (around line 117), compute `load_state = ctx.invoked_subcommand not in LOCAL_ONLY_COMMANDS` *outside* the `if len(paths) == 1:` block. Unlike `SKIP_LOAD_COMMANDS` (whose `load = False` toggle is inside the single-path conditional), `load_state` must apply regardless of how many `--paths` were provided — multi-project monorepo invocations of `format`/`lint` need to be local-only too. -- Pass `load_state=load_state` as an additional keyword argument in the `Context(...)` call at lines 132-137. - -**Verification:** -- Automated: `pytest tests/cli/test_cli.py::test_format_runs_without_state tests/cli/test_cli.py::test_lint_runs_without_state tests/cli/test_cli.py::test_plan_still_loads_state -v` — all three new tests pass. -- Automated: `pytest tests/cli/test_cli.py::test_format_leading_comma_default -v` — existing format CLI test still green. -- Automated: `make py-style` — clean. -- Automated: `make fast-test` — full fast suite green. Confirms Task 1 didn't regress and Task 2 didn't disturb fast tests. -- Manual: from inside the repo, run `sqlmesh --paths examples/sushi format --check` and `sqlmesh --paths examples/sushi lint` against a real sushi project. They should succeed. (Sushi uses DuckDB so this won't *prove* offline behavior, but it sanity-checks the wiring.) -- Manual: read the diff of `cli/main.py` and confirm the only changes are: one new tuple, one new variable, one `if`-check, one `load_state=` kwarg in the `Context(...)` call. - -**Commit:** `git commit -s` with subject `cli: run format and lint without state` (38 chars). Trailers: `Coding-Agent: pi`, `Model: `, `Signed-off-by: ...`. diff --git a/docs/2026-05-21_local-only-format/spec.md b/docs/2026-05-21_local-only-format/spec.md deleted file mode 100644 index 4393315055..0000000000 --- a/docs/2026-05-21_local-only-format/spec.md +++ /dev/null @@ -1,80 +0,0 @@ -# Spec: local-only `format` and `lint` - -## Context - -`sqlmesh format` is a pure text transformation — it parses each `.sql` model and audit file with SQLGlot, optionally rewrites casts or transpiles the dialect, and writes the pretty-printed result back to disk. `sqlmesh lint` is the same shape: it walks loaded model ASTs through linter rules and reports violations. Neither needs a database. But today, both run an authenticated state-sync query before doing any of their actual work. - -The reason is wiring, not intent. Neither command is listed in `SKIP_LOAD_COMMANDS` or `SKIP_CONTEXT_COMMANDS` (`sqlmesh/cli/main.py:31,43`), so the CLI builds a `Context` with `load=True`. `Context.load()` then runs a state-merge block (`sqlmesh/core/context.py:678-697`) that calls `self.state_reader.get_environment(c.PROD)` to pull remote snapshots and environment statements into the local model registry. Touching `state_reader` resolves the lazy `state_sync` property, which connects to the state database and runs `get_versions` (`sqlmesh/core/context.py:609-620`). If state lives in Postgres, Snowflake, BigQuery, or any other shared backend, that's an authenticated network call. In CI, where credentials are typically not provisioned for formatting or lint jobs, this turns a purely local check into a connection-required step. - -Both commands' actual code paths are clean: `Context.format()` (`context.py:1220`) uses `self._models`, `self._audits`, `target._path`, `target.formatting`, `target.dialect`, and `self.config_for_node()` — nothing else. `Context.lint_models()` (`context.py:3210`) iterates over `self._models.values()` and runs each through `Linter.lint_model`; the four places the built-in rules touch `self.context` (`linter/rules/builtin.py:140,161,185,247`) all read from local in-memory data (`models_with_tests`, `get_model`, `extract_references_from_query`, `self.context.path`). Neither command, nor its helpers, ever references `state_sync`, `state_reader`, `engine_adapter`, or `snapshots`. The state connection both commands open today is dead weight from `Context.load()`'s remote-snapshot merge. - -## Goals - -- `sqlmesh format` and `sqlmesh lint` run end-to-end without opening any network connection to the state-sync backend, any other database, or any external service, and without requiring any authentication. -- Neither command requires state credentials, warehouse credentials, or any other auth material to be present in the environment or config. A CI job with zero secrets provisioned can run both successfully against a real project. -- The fix is one focused mechanism — a single new `Context` parameter and a single new CLI tuple — not a per-command refactor. -- Existing behavior of every other CLI command is unchanged. No regression in `plan`, `run`, `render`, `diff`, etc. -- The new behavior is covered by tests that fail without the fix (e.g., constructing a `Context` against a project whose state connection points at an unreachable database, and asserting `format` and `lint` still succeed). - -## Non-goals - -- Making `dag` local-only. It's a plausible candidate but its `--select-model` path constructs a `Selector` (`context.py:2947`) that evaluates `self.state_reader` at construction time, even though `Selector.expand_model_selections` never uses it. Fixing that cleanly would require deferring state-reader resolution inside `Selector` and reviewing every other call site of `_new_selector`. We're not doing it here. -- Making `render`, `rewrite`, `create_test`, `test`, or any other CLI command local-only. They have real state or warehouse dependencies in their actual code paths, not just in `Context.load()`. -- Refactoring `Context.load()` beyond gating the existing state-merge block. The block's logic is unchanged — only whether it runs. -- Suppressing anonymized analytics or any other implicit network activity on CLI invocation. That's governed by the existing `disable_anonymized_analytics` setting and is orthogonal to this change. -- Adding an env-var or config knob to restore the old behavior for `format`/`lint`. The spec commits to the new behavior, consistent with how `SKIP_LOAD_COMMANDS` and `SKIP_CONTEXT_COMMANDS` are handled today (`cli/main.py:31,43`). -- Changing the public Python API beyond the one new `Context(load_state=...)` parameter. No new method, no new module, no rename. - -## Key discoveries - -- `format` and `lint` are wired through `Context` with `load=True`. The CLI's two existing escape tuples don't cover them: neither command appears in `SKIP_LOAD_COMMANDS` or `SKIP_CONTEXT_COMMANDS` (`sqlmesh/cli/main.py:31,43`). -- The state-sync connection both commands open is opened exclusively inside `Context.load()`'s remote-snapshot merge block (`sqlmesh/core/context.py:678-697`). Two calls touch state: `self.state_reader.get_environment(c.PROD)` at line 678, and the snapshot/environment-statement merge gated by `if any(self._projects):` at lines 681-697. -- Reading `self.state_reader` is the trigger. The property returns `self.state_sync` (`context.py:621`), which lazily instantiates the state backend, runs `get_versions(validate=False)`, optionally migrates, and runs `get_versions()` again (`context.py:609-619`). All of that is one authenticated round trip to the configured state database. -- The engine adapter is genuinely lazy. `create_engine_adapter()` wraps the connection in a `ConnectionPool` that doesn't invoke its factory until a cursor is requested (`sqlmesh/utils/connection_pool.py:342`, `sqlmesh/core/engine_adapter/base.py:194`). Neither `Context.format()` nor `Context.lint_models()` ever calls a method that requests a cursor, so no warehouse connection is opened by these commands' own code paths — only by `load()`'s state-merge block. -- `Context.format()` (`context.py:1220`) uses `self._models`, `self._audits`, `target._path`, `target.formatting`, `target.dialect`, and `self.config_for_node()`. Verified by reading the method end to end. -- `Context.lint_models()` (`context.py:3210`) iterates `self._models.values()` and delegates to per-project `Linter.lint_model`. The four places the built-in rules access `self.context` (`linter/rules/builtin.py:140,161,185,247`) are all reads of local in-memory data: `models_with_tests`, `get_model`, `extract_references_from_query`, and `self.context.path`. No rule references `state_sync`, `state_reader`, `engine_adapter`, or `snapshots` (verified by `rg` across `sqlmesh/core/linter/`). -- `update_model_schemas` in the `update_schemas=True` branch of `load()` (`context.py:723`) is local-only — it constructs a `MappingSchema` against `self._models` and uses `OptimizedQueryCache` on the local filesystem. No engine adapter involvement. -- The repo already has the right shape for the change. `cli/main.py` has two precedents — `SKIP_LOAD_COMMANDS` and `SKIP_CONTEXT_COMMANDS` — for marking subcommands that need lighter Context construction, and both flow through to `Context.__init__` via plain boolean parameters. Adding a third tuple with the same shape matches the existing pattern. - -## Proposed approach - -Add a third boolean parameter, `load_state: bool = True`, to `GenericContext.__init__`. Store it on the instance. In `Context.load()`, guard the existing remote-snapshot merge block (`context.py:678-697`) on this flag — when `load_state` is `False`, skip the two `self.state_reader.get_environment(c.PROD)` calls and the snapshot/environment-statement merge between them. The flag is meaningful only when `load=True`; when `load=False`, `load()` isn't called and the flag is inert. Document that in the docstring. - -In `sqlmesh/cli/main.py`, add a new constant alongside the existing two: - -```python -LOCAL_ONLY_COMMANDS = ("format", "lint") -``` - -In `cli(...)`, after the existing `SKIP_CONTEXT_COMMANDS` and `SKIP_LOAD_COMMANDS` checks, set `load_state = False` when `ctx.invoked_subcommand in LOCAL_ONLY_COMMANDS`. Pass it through to the `Context(...)` constructor. The existing `load=True` path is unchanged for these commands — models are still parsed, schemas still updated, audits still loaded — only the state-merge step is skipped. - -No other code changes. `Context.format()`, `Context.lint_models()`, the linter rules, and `update_model_schemas` are already local-only; this spec just removes the unnecessary state-merge that `load()` runs before they get a chance to execute. - -Tests: add a regression test that constructs a `Context` against a project whose state connection points at an unreachable database (e.g., a Postgres `ConnectionConfig` pointing at `localhost:1` or a similar guaranteed-fail target), with `load_state=False`, and asserts that `format` and `lint` complete successfully. Also add a CLI-level test that invokes `sqlmesh format` and `sqlmesh lint` against the same project and asserts neither opens a state connection — likely by patching `EngineAdapterStateSync.get_versions` and asserting it's never called. - -## Key decisions & tradeoffs - -- **`load_state: bool = True` over an enum.** Matches the existing `load: bool = True` shape on `Context`. The nonsensical combination `load=False, load_state=True` is inert (load() never runs), so an enum would add abstraction without preventing a real bug. -- **New `LOCAL_ONLY_COMMANDS` tuple over reusing `SKIP_LOAD_COMMANDS`.** `format` and `lint` *do* need models loaded; they just don't need state merged in. Reusing `SKIP_LOAD_COMMANDS` would require a parallel "models-only loader" and duplicate `load()`'s parsing logic. A third tuple is one line and reads exactly as the existing pattern. -- **Gate the state-merge block, don't extract it.** Pulling lines 678-697 of `context.py` into a private method would be cleaner in isolation but would touch every caller of `load()` and obscure the diff. A single `if self._load_state and any(self._projects):` guard keeps the diff small and the change reviewable. -- **No escape hatch.** Consistent with how `SKIP_LOAD_COMMANDS` and `SKIP_CONTEXT_COMMANDS` are handled. The state-merge work is unused by `format`/`lint`, so there's no scenario where restoring it would be correct behavior for these commands. -- **`Context(load_state=...)` is part of the public API.** It's a Python-level parameter on `GenericContext`, so programmatic users of SQLMesh can construct a local-only Context too. Cost is zero (one kwarg, one docstring line) and the symmetry with `load` is worth it. -- **`load_state=False` is set by the CLI, not inferred at runtime.** We could try to detect at runtime whether a command will touch state, but that's a much larger change. The CLI knows statically which commands are local-only; a constant tuple is the right place for that knowledge. - -## Risks / unknowns - -- **Test fixtures that assume state is queried during load.** Existing tests that build a `Context` with `load=True` may rely on remote snapshots being merged in. Our change defaults `load_state=True`, so these tests should be unaffected — but if any test runs `format` or `lint` through the CLI runner against a project that depends on cross-project snapshot merging for the formatted/linted models to resolve, that test may need a fixture adjustment. Likely none exist, but worth confirming during implementation. -- **`update_model_schemas` cache state.** `load(update_schemas=True)` runs after the state-merge block. When the block is skipped, the local `MappingSchema` won't contain remote snapshot columns. For `format` and `lint` this is fine — neither needs resolved column types from remote projects — but if a linter rule is ever added later that relies on cross-project column resolution, it would silently produce different results under `load_state=False`. Note in the linter rule contract, but no code change needed today. -- **CLI test isolation.** Asserting "no state connection was opened" is easier to assert by patching than by mocking the network. The cleanest seam is `EngineAdapterStateSync.get_versions` — patching it to raise should be enough; we then assert `format`/`lint` succeed and the patched method is never called. If that seam turns out to be wrong (e.g., a different method gets called first), we'll adjust during implementation. -- **Multi-project monorepos.** The state-merge block's purpose is to import remote snapshots for *other* projects when the current load covers only a subset (`context.py:681-697`, gated on `any(self._projects)`). For `format`/`lint`, this means a model that depends on a model defined in a sibling project not currently being loaded won't have its upstream resolved. That's already the behavior for any model `format`/`lint` touches — they operate per-file, not on rendered downstream queries — so there should be no observable difference. Flagging in case a reviewer raises it. -- **Snowflake default-auth state connections.** `PostgresConnectionConfig` accepts `password=None` from a missing `{{ env_var() }}` (Pydantic coercion), so a Postgres state config validates with no secrets and the gate then prevents the connection. `SnowflakeConnectionConfig._validate_authenticator` (`sqlmesh/core/config/connection.py:633-638`) is stricter: under default authentication it raises `ConfigError("User and password must be provided")` at config-load time if both are missing. That validation runs before our `load_state` gate can take effect, so a project whose state lives in Snowflake with default-auth still requires user/password env vars to be set (any non-empty values — they won't be used). This is pre-existing Snowflake validator behavior, not something this change introduces, but it's the one realistic CI configuration where "zero secrets" doesn't hold. Workarounds: use the Snowflake key-pair or OAuth authenticators (which have their own validators with different requirements), or set placeholder env vars in the CI job. - -## Dependencies - -- Research docs: - - `docs/2026-05-21_local-only-format/01_format_command.md` — how `format` works end to end. - - `docs/2026-05-21_local-only-format/02_cli_bootstrap.md` — what runs on any CLI invocation, where state is touched. - - `docs/2026-05-21_local-only-format/03_contributing.md` — DCO sign-off, `make style`, `make fast-test`, PR template, CI matrix. - - `docs/2026-05-21_local-only-format/spec_notes.md` — discovery and planning context that didn't fit in the spec: full CLI command audit, why `dag` was dropped, mechanism alternatives considered, verification methodology. -- No external library or service dependencies. Pure internal change. -- No other in-flight specs or branches to coordinate with — the working branch `fix/local-only-format` is fresh off `main`. diff --git a/docs/2026-05-21_local-only-format/spec_notes.md b/docs/2026-05-21_local-only-format/spec_notes.md deleted file mode 100644 index e1d51dedff..0000000000 --- a/docs/2026-05-21_local-only-format/spec_notes.md +++ /dev/null @@ -1,101 +0,0 @@ -# Spec notes - -Context captured during the discovery and planning conversation that didn't make it into `spec.md`. The spec is the source of truth for what we're building; these are the things a reader would benefit from if the conversation were lost — the audit table, the design alternatives we rejected, the latent issues we noticed, and the verification methodology that backed the spec's confidence. - -## CLI command survey - -When the user asked "are there other commands that should be local-only?", I worked through every `@cli.command` in `sqlmesh/cli/main.py` and classified each by whether it touches state and whether it touches the warehouse. The table below captures the full audit. Only `format` and `lint` made it into this spec; everything else is either already handled (existing skip tuples), genuinely needs state/warehouse, or has an awkward edge case that ruled it out. - -| Command | Current Context mode | Touches state? | Touches warehouse? | Local-only candidate? | -|---|---|---|---|---| -| `init` | `SKIP_CONTEXT_COMMANDS` | No | No | Already covered | -| `ui` | `SKIP_CONTEXT_COMMANDS` | No | No | Already covered | -| `clean` | `SKIP_LOAD_COMMANDS` | No (clears caches) | No | Already effectively local | -| `create_external_models` | `SKIP_LOAD_COMMANDS` | No | **Yes** (queries DB for schemas) | No | -| `destroy` | `SKIP_LOAD_COMMANDS` | Yes | Yes | No | -| `environments` | `SKIP_LOAD_COMMANDS` | Yes | No | No | -| `invalidate` | `SKIP_LOAD_COMMANDS` | Yes | No | No | -| `janitor` | `SKIP_LOAD_COMMANDS` | Yes | Yes | No | -| `migrate` | `SKIP_LOAD_COMMANDS` | Yes | No | No | -| `rollback` | `SKIP_LOAD_COMMANDS` | Yes | No | No | -| `run` | `SKIP_LOAD_COMMANDS` | Yes | Yes | No | -| `table_name` | `SKIP_LOAD_COMMANDS` | Yes (looks up env) | No | No | -| **`format`** | full load | No | No | **Yes** — this spec | -| **`lint`** | full load | No | Possibly via rules — verified no | **Yes** — this spec | -| `dag` | full load | No without `--select-model`; yes with | No | Edge case — see below | -| `render` | full load | Yes (`self.snapshots`, deployability index) | Yes (seed engine adapter) | No | -| `evaluate` | full load | Yes | Yes | No | -| `diff` | full load | Yes (env diff) | No | No | -| `plan` | full load | Yes | Yes | No | -| `create_test` | full load | No | Yes (executes input queries) | No | -| `test` | full load | No | Local test engine | No (uses warehouse-like adapter) | -| `audit` | full load | Yes | Yes | No | -| `check_intervals` | full load | Yes | No | No | -| `fetchdf` | full load | Yes | Yes | No | -| `info` | full load | Optional via `--skip-connection` | Optional | Already has skip | -| `table_diff` | full load | Yes | Yes | No | -| `rewrite` | full load | Unverified — not investigated deeply | Unverified | Out of scope | -| `dlt_refresh` | full load | No (writes models from DLT) | No | Plausible but not investigated | - -`rewrite` and `dlt_refresh` are the two commands where the audit is shallow. They were filed under "out of scope" rather than "verified not a candidate". If someone picks up local-only work later, those are the first ones to check. - -## Why `dag` was dropped - -`dag` looked like a clean third candidate at first. `Context.render_dag` (`context.py:2170`) just writes HTML of `self.get_dag(select_models)`, and the linter rules and engine adapter aren't referenced anywhere in that path. But tracing through `Context.get_dag` (`context.py:2121`) surfaced a latent issue: - -When `select_models` is provided, `get_dag` calls `self._new_selector().expand_model_selections(...)`. `Context._new_selector()` (`context.py:2947`) constructs a `Selector` with `self.state_reader` as its first argument. Reading `self.state_reader` evaluates the lazy property (`context.py:621`), which evaluates `self.state_sync` (`context.py:609-619`), which boots the state backend, runs `get_versions(validate=False)`, optionally migrates, and runs `get_versions()` again. **The act of constructing the Selector opens a state connection**, even though `Selector.expand_model_selections` (`selector.py:191+`) never actually uses the state reader — only `_load_env_models` (`selector.py:153`) does, and `get_dag` never reaches that path. - -Three options were considered: - -1. **Drop `dag`** — what we chose. Tightest spec. `dag` is lower-traffic than `format`/`lint` in CI. -2. **Include `dag` partially** — fix the no-`--select-model` path only, leave `dag --select-model X` broken offline. Would be a subtle, surprising behavior split for the same command. Rejected. -3. **Include `dag` fully** — also defer `state_reader` evaluation inside `Selector`. Would touch every other call site of `_new_selector` (used by `plan`, `diff`, `run`, etc., via state methods). Wider blast radius, departs from "minimal change". Rejected. - -The Selector wart is real but out of scope for this PR. - -## Mechanism alternatives considered - -Before landing on the `load_state` kwarg, three mechanisms were on the table: - -1. **`load_state` kwarg on Context, new `LOCAL_ONLY_COMMANDS` tuple in CLI** — chosen. Mirrors the existing `load: bool = True` shape and the existing `SKIP_*_COMMANDS` tuples. One kwarg, one tuple, two if-guards. -2. **Extract the state-merge block into a private `_merge_remote_state()` method, call selectively** — cleaner separation of concerns inside `Context`, but `load()` is called from `Context.__init__`, so the CLI can't directly influence whether the new method runs. We'd still need a kwarg on `__init__`, which means the same surface area plus an extracted method. No net win. -3. **Add `format`/`lint` to `SKIP_LOAD_COMMANDS` and have those commands load models themselves** — reuses existing infrastructure but requires a new `_load_models_only()` method that duplicates much of `load()`. Bigger surface area for bugs, and doesn't match how the other `SKIP_LOAD` commands work (they genuinely don't need models). - -## Naming alternatives for `load_state` - -The chosen name `load_state` mirrors the existing `load: bool = True`. Other names considered: - -- `state` — too vague; conflicts with `state_sync` / `state_reader` properties and the `state` subcommand group. -- `require_state` — frames it as a requirement rather than an action; less symmetric with `load`. -- `offline` — describes user intent ("offline mode") but inverts polarity vs every other boolean in this area, and is broader than what we're doing (we still hit the filesystem and load models). - -The user also asked whether an enum (`LoadMode.NONE` / `LoadMode.LOCAL` / `LoadMode.FULL`) would be better, since `load=False, load_state=True` is a nonsensical combination. The answer was no: that combination is inert (when `load=False`, `load()` is never called, so `load_state` has no effect), and the repo's existing pattern in this area is plain booleans (`load=True`, the two `SKIP_*` tuples). An enum would stand out as a new abstraction without preventing a real bug. - -## Verification methodology - -To verify a command is local-only after the fix, the procedure is: - -1. Start from the `@cli.command` in `sqlmesh/cli/main.py`. Identify the `Context` method(s) it calls. -2. Read those methods in full. Recursively follow every function/method call into helpers. -3. Look for any read of: `self.state_sync`, `self.state_reader`, `self._state_sync`, `self.engine_adapter`, `self._get_engine_adapter`, `self.connection_config`, `self.snapshots`. These are the seams that boot a connection. Also look for direct calls to `.get_environment`, `.get_versions`, `.get_snapshots`, or other state-sync API methods on any object. -4. Pay attention to **property side effects**, not just method calls. `self.state_reader` is a property that boots the state backend on access — just *passing* it as an argument to another constructor triggers the connection (this is how `dag --select-model` gets caught). -5. For linter rules specifically, check what's reachable through `self.context` from rule classes. The `Rule` base class (`linter/rule.py:75`) stores `self.context = context`, so any attribute read on `self.context` from inside a rule is a code path to audit. -6. Cross-check with existing tests: do they construct a full `Context` with a live DB, or do they get away with a DuckDB stub? Tests that pass without real credentials are evidence about what's actually required at runtime — though note that DuckDB-backed state still hides the boot sequence behind a local file, so test passage isn't proof of "no state access". - -A subtle trap: when a researcher reports "command X is NOT CLEAN", check whether they're describing current behavior (before the fix) or post-fix behavior. The researcher's first verification of `lint`/`dag` reported them as "NOT CLEAN" because today they're wired through `load=True` and thus hit state during `Context.load()` — true but not the question being asked. The actual question is whether the command's *own* code path (everything downstream of `load()`) reaches for state. For `lint`, it doesn't. For `dag`, it does (via Selector). That distinction is what determines whether the spec's mechanism is sufficient. - -## Test seam - -The spec proposes patching `EngineAdapterStateSync.get_versions` as the assertion seam. The reasoning: `get_versions` is the first state-touching method called when the lazy `state_sync` property resolves (`context.py:613`). Patching it to raise turns any accidental state access into a loud failure that the test can detect. If the implementer finds that another method gets called first in practice (e.g., during state-sync construction itself), they should adjust to patch whichever method actually fires first — the principle is "any state access raises", not specifically "`get_versions` raises". - -A lighter alternative: configure the project's state connection to point at an unreachable host (`localhost:1`, or a Postgres config with an obviously-wrong port). This exercises the real connection code and produces a meaningful error if the fix regresses. Both flavors of test have value; the spec leaves room for the implementer to pick. - -## `update_model_schemas` is local - -The `update_schemas=True` branch of `load()` (`context.py:723`) calls `update_model_schemas(self.dag, models=self._models, cache_dir=self.cache_dir)`. This was specifically verified: `sqlmesh/core/model/schema.py:22` constructs a `MappingSchema` against the local models dict and uses `OptimizedQueryCache` (a local-filesystem cache). It never references `self.engine_adapter` or any state API. So `format` and `lint` get fully-resolved local schemas without paying for any connection. - -If anyone later writes a linter rule that needs cross-project column resolution (a model imported from a sibling project that's only present in remote state), that rule would silently produce different results under `load_state=False`. Not a concern today — no such rule exists — but worth flagging if rule authors ever start reaching for remote schemas. - -## What the researcher got right and where it misled - -The research docs in this folder (`01_format_command.md`, `02_cli_bootstrap.md`, `03_contributing.md`) are accurate as references for the current behavior of the codebase. The first round of `lint`/`dag` verification was *also* accurate as a description of current behavior, but its verdict ("NOT CLEAN") was framed against today's wiring rather than against the proposed fix. When delegating verification work to a researcher subagent in this kind of "are these commands ready for X change?" conversation, it's worth explicitly framing the question as "given that we'll make change Y, would the command still touch state?" rather than just "does the command touch state?"