Reject case-colliding ids when using YAMLStorage

CLAUDE.md

@@ -53,6 +53,7 @@ poetry run ruff check slayer/ tests/
 - **Schema versioning**: `SlayerModel`, `SlayerQuery`, and `DatasourceConfig` carry a `version: int` (currently `7` for `SlayerModel`, `3` for `SlayerQuery`, `1` for `DatasourceConfig`). On load, older versions are upgraded via the converter chain in `slayer/storage/migrations.py` before Pydantic validates the dict. Saves always emit the current version. The hook is on the Pydantic class itself (`@model_validator(mode="before")`), so every storage backend — YAML, SQLite, third-party backends, plus MCP/API/dbt entry points — gets migrations automatically without backend changes. The v1→v2 converter (in `slayer/storage/v2_migration.py`) merges v1 `dimensions`+`measures` into v2 `columns`, repurposes `measures` to hold `ModelMeasure` formulas, and renames `SlayerQuery.fields`→`measures`. The v2→v3 converter (in `slayer/storage/v3_migration.py`) drops the legacy `dry_run`/`explain` fields from `SlayerQuery` (they are now engine kwargs only — `engine.execute(query, dry_run=..., explain=...)`) and walks `SlayerModel.source_queries` entries through the SlayerQuery chain. The v3→v4 converter (in `slayer/storage/v4_migration.py`) requires non-empty `data_source` on table-backed SlayerModel dicts (`sql_table` or `sql` mode); query-backed models (`source_queries` set) are exempt because their `data_source` is filled by `engine._validate_and_populate_cache` from the resolved virtual model before save. The v4 converter also ships layout migrators that move legacy `models/<name>.yaml` flat files into `models/<data_source>/<name>.yaml` and rebuild the SQLite `models` table with a composite `(data_source, name)` primary key. The v4→v5 converter (in `slayer/storage/v5_migration.py`, DEV-1361) coarse-renames `Column.type` legacy values to the new sqlglot-aligned vocabulary (`string→TEXT`, `number→DOUBLE`, `time→TIMESTAMP`, etc.) and strips the dead aggregation pseudo-types (`count`/`sum`/...). The v5→v6 converter (in `slayer/storage/v6_migration.py`, DEV-1375) is a no-op forward — v6 introduces a single new optional field, `Column.sampled: Optional[str]`, that caches the per-column sample-value snapshot consumed by [`search`](docs/concepts/search.md) and `inspect_model`. The v6→v7 converter (in `slayer/storage/v7_migration.py`, DEV-1480) is also a no-op forward — v7 adds two more `Column` fields: `sampled_values: Optional[List[str]]` (the structured top-50-by-frequency list, paired with the existing text `sampled`) and `distinct_count: Optional[int]` (true cardinality at profile time, surfaced via a secondary `count_distinct` query on overflow). Storage backends additionally introspect each model's datasource on first load and refine `DOUBLE → INT` for base columns whose live SQL type is integer (`slayer/storage/type_refinement.py`); the refined model is written back so subsequent loads skip both steps. `SlayerQuery` v3 sets `extra="forbid"` so unknown fields raise. See [docs/concepts/models.md](docs/concepts/models.md#schema-versioning).
 - **DataType / CAST emission** (v5, DEV-1361): `DataType` values match sqlglot's `exp.DataType.Type` byte-for-byte: `TEXT`, `INT`, `DOUBLE`, `BOOLEAN`, `DATE`, `TIMESTAMP`. Auto-ingestion narrows integer DB types (INTEGER/BIGINT/SERIAL/INT8…/UINT8…) to `INT`, and floats/numerics (FLOAT/DOUBLE/DECIMAL with scale>0) to `DOUBLE`; NUMERIC(p,0) and DECIMAL(p,0) are integer-shaped and narrow to `INT`. The SQL generator wraps **non-bare** `Column.sql` (function calls, arithmetic, CASE WHEN) in `CAST(... AS <type>)` driven by `Column.type`; bare identifiers and `sql=None` are emitted unchanged. `TEXT` is a no-op (skipped, since `CAST AS TEXT` is cosmetic and doesn't unwrap SQLite's JSON-quoted strings anyway). `ModelMeasure.type` (also `Optional[DataType]`) declares the formula's result type; when set, the aggregation expression is wrapped in an outer CAST. Lenient `before`-validators on `Column.type` and `ModelMeasure.type` absorb legacy lowercase agent input (`"string"` → `TEXT`, `"number"` → `DOUBLE`, etc.) and silently drop dropped pseudo-types. `slayer storage migrate-types --dry-run [--data-source X]` runs the storage refinement step explicitly for batch / inspectable usage. Schema-drift detection (`data_type_bucket`) keeps `INT` and `DOUBLE` in the same `"number"` bucket so a `DOUBLE`-typed persisted column does not flag as drift against an `INT` live column.
 - **Datasource-scoped storage** (v4, DEV-1330): Models are keyed by `(data_source, name)`, not bare `name`. Two datasources can share a table name without collision. `storage.get_model(name, data_source=None)` and `storage.delete_model(name, data_source=None)` resolve bare names by: (1) returning the unique match if exactly one model has that name, (2) walking `storage.get_datasource_priority()` to pick the first datasource in the priority list that has the name, (3) raising `slayer.core.errors.AmbiguousModelError` otherwise. The priority list is configured via `storage.set_datasource_priority(["db_a", ...])`, the MCP `set_datasource_priority` tool, the REST `PUT /datasources/priority`, or the `slayer datasources priority` CLI subcommand. `engine.execute(query, data_source=...)` passes a hint that wins over the priority list. Joins resolve targets within the parent model's `data_source` only — cross-datasource joins are not auto-mirrored. A sibling Linear issue ([DEV-1342](https://linear.app/motley-ai/issue/DEV-1342)) tracks whether to add `datasource.model_name` dot syntax inside query strings.
+- **Case-collision rejection, YAML-only** (issue #249): saving a datasource, model, or memory whose id differs only by letter case (`casefold()`-equal, raw-different) from an existing one raises `slayer.core.errors.IdCollisionError(SlayerError, ValueError)` — attrs `kind`/`new_id`/`existing_id`/`data_source` — in the **YAML backend only** (ids are filenames there; case variants alias on macOS/Windows). SQLite deliberately keeps case-variant ids as distinct rows (case-sensitive TEXT PKs) — which means a SQLite store holding such a pair can't be exported to the YAML layout without renaming. Mechanism: the checks live in the `StorageBackend` base templates (`save_model` under `_validate`, `save_memory` for user-supplied ids) gated on the class flag `_ids_collide_as_filenames` (False on the base, True on `YAMLStorage`, copied from `inner` by `JoinSyncStorage.__init__` so wrapped YAML still rejects); this placement preserves the `save_model(_validate=False)` migration-write-back bypass. The datasource check is the public base helper `check_datasource_id_collision(name)` (checks datasource names ∪ model data_sources — shared `models/<ds>/` dir namespace) called by `YAMLStorage.save_datasource`; `save_datasource` itself stays a backend-implemented abstract method. Custom backends with a case-insensitive keyspace opt in via the flag + helper. Exact-id re-saves remain upserts; model saves check `data_source` and `name` (within the same datasource). YAML reads/deletes verify the EXACT directory entry via `os.listdir` comparison (`_exact_entry_exists`) — a case-variant `get_model`/`get_datasource`/`get_memory` returns not-found and a delete is a no-op instead of silently hitting the wrong file. `migrate_memories_layout` and `migrate_yaml_layout` pre-check collisions before writing anything (legacy files preserved on failure). Surfacing: REST `POST /datasources` → 400 (model/memory endpoints already mapped ValueError→400); MCP `create_datasource` returns a friendly message and its auto-ingest loop reports per-model save failures without aborting; CLI prints and exits (or skips-and-continues in ingest loops).
 - **Source modes**: a `SlayerModel` has exactly one source: `sql_table` (physical table), `sql` (explicit SQL subquery), or `source_queries` (query-backed: `List[SlayerQuery]`). Validators enforce mutual exclusivity, reject empty `source_queries=[]`, require `name` on every non-final stage, and reject duplicate stage names. `SlayerModel.source_queries` entries given as dicts are auto-parsed into `SlayerQuery` instances by a Pydantic before-validator.
 - **Stages form a DAG, not just a chain**: any stage in `source_queries` or a runtime query list may use a named sibling as `source_model` or as `joins.target_model`. The **runtime list path** (`engine.execute(query=[...])`, CLI `slayer query @file.json`, MCP `query_nested`) **auto-sorts** the input via `SlayerQueryEngine._topologically_order_queries` — a hand-rolled Kahn's algorithm (no `graphlib` dep) that reorders so every stage appears after the siblings it references. The last entry of the input stays last as the entry point / DAG root, so the result returned doesn't depend on sort order. Validations performed up front: missing `name` on a non-final entry; duplicate names; self-references; the root being referenced by any other stage (root must be the dependency sink); cycles. Unreachable utility sub-queries are accepted — they flow through the sort but are silently dropped from the emitted SQL since nothing resolves them. Stored `SlayerModel.source_queries` retains strict-order semantics (relies on `_scope_named_queries_to_prior` plus the `_forbidden_sibling_refs_var` ContextVar in `_resolve_model_inner` / `_resolve_join_target`) — YAML-defined source_queries are read top-to-bottom and the strict-order error pins typos at resolve time. The runtime list's reference extractor walks `source_model` (string sibling refs), `ModelExtension.source_name`, and `joins[].target_model` from both dict and typed shapes; formula-only references resolve correctly because the explicit join they require is what the extractor picks up.
 - **Query-backed models**: `SlayerModel.query_variables: Dict[str, Any]` provides defaults for `{var}` placeholders in `source_queries`. `engine.create_model_from_query(query, name, variables=..., save=True)` saves a query as a query-backed model and populates the `columns` + `backing_query_sql` cache. `engine.save_model(model)` is the engine-side save helper that runs source-mode validation + cache refresh + persistence; user-supplied `columns` and `backing_query_sql` on a query-backed model are rejected at save with a clear error. The cache is refreshed only on save paths (`engine.save_model` / `create_model_from_query(save=True)`); `engine.execute` never writes to storage, even on stale or empty caches (closes #74 — `tests/test_query_backed_models.py::test_execute_never_writes_to_storage` pins this).

docs/concepts/ingestion.md

@@ -276,6 +276,8 @@ Ingest-on-startup: N/M datasources ingested (K failed: name1, name2)
 - **Existing `sql_table`-mode model** → append new columns and joins from the live schema. Existing columns and joins are **never** mutated — `description`, `label`, `format`, `meta`, and `allowed_aggregations` are preserved verbatim.
 - **Existing `sql`-mode or query-backed model with the matching name** → skipped silently; those are user-authored.
 
+With the default YAML storage, two live tables whose quoted names differ only by letter case (`"Orders"` vs `orders`) cannot both be persisted — model names collide as filenames on macOS / Windows, so the save is rejected (`IdCollisionError`). The first table wins; the second surfaces as a per-model entry in `IdempotentIngestResult.errors` (or a per-model message on the CLI / MCP paths) without aborting the rest of the ingest. SQLite storage persists both.
+
 After the additive pass, `validate_models` runs against the in-scope models and the result is merged into the response (`IdempotentIngestResult.to_delete`). Type-bucket drift on existing columns surfaces there — apply via `slayer validate-models --force-clean`, then re-ingest to pick up the new live type. See [Schema Drift](schema-drift.md) for the full diff / cascade contract.
 
 ### Search side effects

docs/concepts/memories.md

@@ -77,7 +77,10 @@ int-shaped id (`"1"`, `"2"`, ...). Supply a string for a stable
 user-controlled id (`"kb.policy.42"`) — useful for knowledge-base
 ingestion pipelines. Charset excludes `:`, `/`, `?`, `#`, whitespace,
 and ASCII control characters. Duplicate id → unconditional **upsert**,
-`created_at` preserved.
+`created_at` preserved. In the default YAML storage, an id that differs
+only by letter case from an existing one (`X` vs `x`) raises
+`IdCollisionError` — ids are filenames there, and case variants collide
+on macOS / Windows.
 
 `description` is optional (≤ 500 chars). When set, `search(compact=True)`
 and `inspect_model(compact=True)` surface this short preview instead of

docs/concepts/models.md

@@ -22,7 +22,7 @@ A query then asks for `revenue:sum` (aggregate the `revenue` column), `aov` (the
 
 | Field | Type | Required | Description |
 |-------|------|----------|-------------|
-| `name` | string | Yes | Unique model name |
+| `name` | string | Yes | Unique model name. In the default YAML storage, names differing only by letter case (`Orders` vs `orders`) are rejected at save time (`IdCollisionError`) — they would collide as filenames on macOS / Windows |
 | `sql_table` | string | One of | A physical database table (e.g. `public.orders`) |
 | `sql` | string | these | A SQL subquery to use as the source |
 | `source_queries` | list[SlayerQuery] | three | Saved query stages — makes the model **query-backed** |

docs/configuration/storage.md

@@ -64,6 +64,8 @@ slayer_data/
 
 **Layout note:** Models live under `models/<data_source>/<name>.yaml` so two datasources sharing a table name don't collide. Opening a `YAMLStorage` on a legacy flat directory migrates `models/<name>.yaml` files into the nested layout automatically. If a flat file has an empty `data_source` and exactly one datasource is registered, the migrator auto-fills it; otherwise it hard-fails so the user can edit `data_source` by hand before reopening.
 
+**Name collisions:** Because names become filenames here, two ids differing only by letter case (`Orders` vs `orders`) would address the same file on macOS / Windows. Saving a datasource, model, or memory whose id differs only by case from an existing one therefore raises `IdCollisionError` — on every platform, so a YAML store created on Linux stays portable. Re-saving the exact same id is still a normal upsert. On the read side, lookups compare the exact filename, so `get_model("Orders")` when only `orders.yaml` exists returns "not found" instead of the wrong model (and a delete is a no-op). The layout migrations apply the same check up front and refuse to run — legacy files untouched — if migrating would collide. This is YAML-specific: SQLite keys are case-sensitive and store case-variant ids distinctly (which also means such a SQLite store cannot be exported to the YAML layout without renaming).
+
 **Embeddings sidecar (DEV-1405):** Embedding rows used by the optional dense-search channel live in a SQLite file at `<base_dir>/embeddings.db`, **not** in `embeddings.yaml`. Embeddings are derived artifacts (regeneratable by `slayer ingest` / `--ingest-on-startup`), not user-authored config, so the diffable-in-git property that drives the YAML choice for models doesn't apply. A pre-DEV-1405 `embeddings.yaml` or `counters.yaml` is silently renamed to `<name>.yaml.legacy` on first open and ignored thereafter; re-run `slayer ingest` to repopulate `embeddings.db`. The schema is identical to the `SQLiteStorage` embedding table — both backends delegate to a shared `SidecarEmbeddingStore` helper.
 
 **Memory id allocation (DEV-1658):** The next int-shaped memory id is derived by scanning the `memories/` directory of per-id `.md` files — `max(int-shaped id) + 1` — rather than a separate counter file. Non-int ids (e.g. `help.intro`, `kb.policy.42`) are ignored by the allocator. Ids of deleted memories may be reused by future saves; cascade-on-delete in `delete_memory` already removes the matching embedding row.
@@ -109,21 +111,25 @@ from slayer.storage.base import StorageBackend
 from slayer.core.models import SlayerModel, DatasourceConfig
 
 class MyCustomStorage(StorageBackend):
-    # Models are keyed by (data_source, name).
-    def save_model(self, model: SlayerModel) -> None: ...
-    def _list_all_model_identities(self) -> list[tuple[str, str]]: ...
-    def get_model(self, name: str, data_source: str | None = None) -> SlayerModel | None: ...
-    def delete_model(self, name: str, data_source: str | None = None) -> bool: ...
+    # Models are keyed by (data_source, name). ``save_model`` is a template
+    # method on the base class (it runs shared validation); backends
+    # implement only ``_save_model_impl``.
+    async def _save_model_impl(self, model: SlayerModel) -> None: ...
+    async def _list_all_model_identities(self) -> list[tuple[str, str]]: ...
+    async def get_model(self, name: str, data_source: str | None = None) -> SlayerModel | None: ...
+    async def _delete_model_row(self, *, data_source: str, name: str) -> bool: ...
 
     # ``StorageBackend`` provides default implementations of ``list_models``
     # and ``resolve_model_identity`` on top of ``_list_all_model_identities``.
 
-    def save_datasource(self, datasource: DatasourceConfig) -> None: ...
-    def get_datasource(self, name: str) -> DatasourceConfig | None: ...
-    def list_datasources(self) -> list[str]: ...
-    def delete_datasource(self, name: str) -> bool: ...
+    async def save_datasource(self, datasource: DatasourceConfig) -> None: ...
+    async def get_datasource(self, name: str) -> DatasourceConfig | None: ...
+    async def list_datasources(self) -> list[str]: ...
+    async def _delete_datasource_row(self, name: str) -> bool: ...
 ```
 
+If your backend stores ids as filenames (or another case-insensitive keyspace), set the class attribute `_ids_collide_as_filenames = True` — the base save templates then reject ids differing only by case — and call `await self.check_datasource_id_collision(datasource.name)` at the top of your `save_datasource`.
+
 Register it for URI-based resolution:
 
 ```python

slayer/api/server.py

@@ -523,9 +523,15 @@ async def get_datasource(name: str) -> dict[str, Any]:
                 data[secret_field] = "***"
         return data
 
-    @app.post("/datasources")
+    @app.post(
+        "/datasources",
+        responses={400: {"description": "Name conflicts with an existing datasource (differs only by case)."}},
+    )
     async def create_datasource(datasource: DatasourceConfig) -> dict[str, str]:
-        await storage.save_datasource(datasource)
+        try:
+            await storage.save_datasource(datasource)
+        except ValueError as e:
+            raise HTTPException(status_code=400, detail=str(e))
         return {"status": "created", "name": datasource.name}
 
     @app.delete("/datasources/{name}")

slayer/cli.py

@@ -1805,7 +1805,11 @@ def _persist_ingested_models(models, storage, *, assume_yes: bool, pre_save=None
     for model in models:
         if pre_save is not None:
             pre_save(model)
-        run_sync(storage.save_model(model))
+        try:
+            run_sync(storage.save_model(model))
+        except ValueError as e:
+            print(f"Skipped {model.name}: {e}")
+            continue
         print(f"Ingested: {model.name} ({len(model.columns)} columns, {len(model.measures)} measures)")
 
 
@@ -1839,7 +1843,11 @@ def _run_datasources_create(args, storage):
         print("Aborted.")
         sys.exit(1)
 
-    run_sync(storage.save_datasource(ds))
+    try:
+        run_sync(storage.save_datasource(ds))
+    except ValueError as e:
+        print(f"Error: {e}")
+        sys.exit(1)
     print(f"Created datasource '{ds.name}' ({ds.type}).")
 
     if not args.ingest:
@@ -1907,7 +1915,11 @@ def _run_datasources_create_demo(args, storage):  # NOSONAR S3776 — linear dem
         print("Aborted.")
         sys.exit(1)
 
-    run_sync(storage.save_datasource(ds))
+    try:
+        run_sync(storage.save_datasource(ds))
+    except ValueError as e:
+        print(f"Error: {e}")
+        sys.exit(1)
     print(f"Created datasource '{ds.name}' (duckdb).")
 
     if not args.ingest: