From 2bf0b34f5add2dc6e14ce55fb2c61810fe6c549c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 02:37:21 +0000 Subject: [PATCH 1/6] docs(howto): lead search-by-meaning with a vec0 index The guide taught semantic search only as a path-table subquery, which costs one embed round trip per matched file per query on top of a walk that reads every file's content. Since #956 the ddl batch can declare a vec0 virtual table beside the row table and fill it from a trigger, so a query embeds the question once and sqlite-vec scans in process. The index shape is now the primary recipe: model-width probe, the [[table]] config, the MATCH/k query joined back on rowid, what the two triggers guarantee, and what a model-id edit costs. The path-table shape stays as the zero-setup option with the trade stated explicitly, and the "Top-k is LIMIT k" tip is folded into it -- it existed to explain why MATCH ... AND k = did not apply, which stops being true once the primary example uses vec0. Two corrections to the template sketched in #960, both found by running it: vec0 defaults to L2, so the declaration needs distance_metric=cosine to agree with vec_distance_cosine; and embed(NULL) is NULL, which vec0 rejects outright and which fails the whole table load, so the insert trigger needs a WHEN new.text IS NOT NULL guard. Closes #1003 --- docs/howto/search-by-meaning.md | 199 +++++++++++++++++++++++++++----- 1 file changed, 170 insertions(+), 29 deletions(-) diff --git a/docs/howto/search-by-meaning.md b/docs/howto/search-by-meaning.md index 1d5e15885..31f413c24 100644 --- a/docs/howto/search-by-meaning.md +++ b/docs/howto/search-by-meaning.md @@ -3,10 +3,19 @@ Ask a question in plain language and get the closest documents back — even when they share no keywords with it. Install [`dirsql-plugin-embeddings`](../plugins.md#dirsql-plugin-embeddings) and -semantic search becomes plain SQL: the plugin's `embed()` function turns -text into vectors, [`sqlite-vec`](https://github.com/asg017/sqlite-vec)'s -`vec_distance_cosine()` measures distance, and `ORDER BY … LIMIT` does the -ranking. No config, no API keys, no services — the model runs locally. +semantic search becomes plain SQL: the plugin's `embed()` function turns text +into vectors and loads [`sqlite-vec`](https://github.com/asg017/sqlite-vec), +which does the distance math. No API keys, no services — the model runs +locally. + +Where those vectors live is the one real decision, and there are two answers: + +- **A `vec0` index**, declared in config beside the table and filled by a + trigger as files are ingested. A query then embeds exactly one string — the + question — and `sqlite-vec` scans the stored vectors in process. This is the + shape to build on. +- **A path-table subquery**, with no config at all: embed every matched file + inline, rank, cut. Ideal for a one-off question over a handful of files. Suppose short notes live in `notes/*.md`: @@ -16,7 +25,7 @@ notes/branches.md # git feature branches and pull requests notes/tomatoes.md # planting tomato seedlings after the last frost ``` -## The one-liner +## Try it in one line The plugin package is also its own command. Give it a corpus glob and a question, and it prints the closest paths, ranked by distance: @@ -40,11 +49,147 @@ The first-ever run downloads the model (on the order of a hundred megabytes, with progress on stderr); after that it loads from the local cache. Results print one `pathdistance` line per match, closest first. -## The SQL behind it +That command generates the **path-table** shape below — zero setup, and it +embeds every matched file on every run. + +## Which shape + +Both rank by cosine distance and return the same rows. They differ in what +each query costs and what you have to declare: + +| | Path-table subquery | `vec0` index | +|---|---|---| +| Setup | none | a `[[table]]` with a `ddl` batch | +| Freshness | always current — the walk is the read | watcher-maintained; survives restarts under [`--persist`](./persist.md) | +| Per query | one `embed()` round trip **per matched file**, plus a walk that reads every file's content | one `embed()` round trip **total**, plus an in-process KNN scan | +| Good for | a one-off question, a small corpus, an ad-hoc glob | a corpus you query repeatedly | + +The index only pays off when the table outlives the query — under +`--persist`, or inside a long-running [`dirsql server`](../reference/cli.md). +A one-shot `dirsql query` with an ephemeral index rebuilds the table, and +therefore re-embeds the corpus, before it answers; that is strictly more work +than the subquery. + +## Build a vector index + +### 1. Find your model's width + +`vec0` fixes the vector length when the table is declared, so you need the +number your model returns: + +```bash +uvx --with dirsql-plugin-embeddings dirsql "SELECT vec_length(embed('probe')) AS width" +``` + +Whatever that prints is what goes in `float[…]` below — the examples here +write `512`. Get it wrong and the table load fails with an error naming both +figures (`Expected 512 dimensions but received 768.`), so this is cheap to +correct. + +### 2. Declare the table and the index + +A named table's rows come from its [`on-file`](../reference/config.md#table) +hook — dirsql injects nothing — so put a small reader next to the config. +`note.py` prints one row per file, carrying the path to display and the text +to embed: + +```python +#!/usr/bin/env python3 +import json, os, sys + +path, root = sys.argv[1], sys.argv[2] +text = open(path, encoding="utf-8").read() +print(json.dumps([{"path": os.path.relpath(path, root), "text": text}])) +``` + +`ddl` is a whole SQL batch, so the row table, the vector index and the two +triggers that keep them in sync all arrive together in `.dirsql.toml`: + +```toml +[[table]] +name = "notes" +glob = "notes/*.md" +on-file = "python3 note.py {path} {root}" +ddl = ''' +CREATE TABLE notes (path TEXT, text TEXT); + +CREATE VIRTUAL TABLE notes_vec + USING vec0(embedding float[512] distance_metric=cosine); + +CREATE TRIGGER notes_vec_ai AFTER INSERT ON notes +WHEN new.text IS NOT NULL BEGIN + INSERT INTO notes_vec(rowid, embedding) + VALUES (new.rowid, embed(new.text, 'minishlab/potion-retrieval-32M')); +END; +CREATE TRIGGER notes_vec_ad AFTER DELETE ON notes BEGIN + DELETE FROM notes_vec WHERE rowid = old.rowid; +END; +''' +``` + +Three details earn their keep: + +- **`distance_metric=cosine`.** `vec0` defaults to L2, which ranks these + vectors differently. With cosine declared, `notes_vec.distance` is exactly + what `vec_distance_cosine()` computes in the subquery shape. +- **`WHEN new.text IS NOT NULL`.** `embed(NULL)` is `NULL`, and `vec0` rejects + a NULL vector outright — without the guard, one unreadable file fails the + whole table load (`Inserted vector for the "embedding" column is invalid`). + The guard is this shape's counterpart to the subquery's + `WHERE emb IS NOT NULL`. +- **No `[[dirsql.extension]]` entry.** The plugin ships its own config + fragment declaring both `sqlite-vec` and `embed()`, and the launcher + [loads it automatically](../reference/cli.md#plugins). Only the `[[table]]` + is yours to write. + +### 3. Ask a question + +Match against the index, join back to the row table on `rowid`, and let +`sqlite-vec` do the top-k: + +```bash +uvx --with dirsql-plugin-embeddings dirsql query " + SELECT notes.path, notes_vec.distance + FROM notes_vec + JOIN notes ON notes.rowid = notes_vec.rowid + WHERE notes_vec.embedding MATCH embed('how do I cook pasta?', + 'minishlab/potion-retrieval-32M') + AND k = 3 + ORDER BY distance" -c ./.dirsql.toml --persist +``` + +```json +[{"path":"notes/pasta.md","distance":0.315},{"path":"notes/tomatoes.md","distance":0.881},{"path":"notes/branches.md","distance":0.92}] +``` + +`MATCH … AND k = 3` is `sqlite-vec`'s KNN form: `k` is the number of nearest +neighbours to return, and the virtual table supplies a `distance` column for +each. Embed the question with the **same model id the trigger used** — a +vector from another model is a different space, and the ranking it produces is +noise rather than an error. + +### What the triggers guarantee + +dirsql writes file rows with plain `INSERT` and `DELETE` — an update is a +delete and an insert in one transaction, and there is no `UPDATE` path on user +rows — so those two triggers cover every case. The initial load fills the +index; each [watcher](./react-to-changes.md) event maintains it; deleting a +file removes its row *and* its vector. You maintain nothing, and nothing goes +stale. + +### Changing the model + +The model id is a literal inside the `ddl` string, and the config hash covers +the whole batch. Editing the id therefore invalidates a +[persistent cache](./persist.md) and forces a full re-scan and re-embed +([batch `ddl`](../reference/config.md#batch-ddl)). That is the whole story for +model changes: no stale vectors, no ownership tracking, no migration step. + +## Without config: rank a glob inline -The one-liner generates and runs ordinary `dirsql` SQL, and you can write it -yourself when you want more than ranked paths — a different projection, a -join, a `WHERE` clause, a subset of a JSON file's content: +For a one-off question, skip all of the above. The subquery scans a +[path-table](../reference/path-tables.md) and embeds each matched file's +`content` on the spot: ```bash uvx --with dirsql-plugin-embeddings dirsql " @@ -65,12 +210,11 @@ distance ranking is doing the work. Reading the query inside-out: -1. The subquery scans the [path-table](../reference/path-tables.md) - `'./notes/*.md'` and embeds each file's - [`content`](../reference/path-tables.md#columns) — only the files the - glob matches are ever read or embedded. In hand-written SQL the `./` - prefix is [required](../reference/path-tables.md#writing-the-path); only - the one-liner normalizes a bare glob for you. +1. The subquery scans the path-table `'./notes/*.md'` and embeds each file's + [`content`](../reference/path-tables.md#columns) — only the files the glob + matches are ever read or embedded. In hand-written SQL the `./` prefix is + [required](../reference/path-tables.md#writing-the-path); only the + one-liner normalizes a bare glob for you. 2. `embed('how do I cook pasta?')` embeds the question once (the function is deterministic, so SQLite reuses the value across rows). 3. `WHERE emb IS NOT NULL` drops the files that could not be embedded. A @@ -79,7 +223,9 @@ Reading the query inside-out: and its distance are NULL too — and SQLite sorts NULLs *first* ascending, so without this line the unrankable files take the top-k slots. 4. `vec_distance_cosine(...)` computes cosine distance between the two - vectors; `ORDER BY distance LIMIT 3` keeps the three nearest. + vectors; `ORDER BY distance LIMIT 3` keeps the three nearest. There is no + `vec0` table here, so `MATCH … AND k =` does not apply — for a plain + expression, `ORDER BY … LIMIT k` *is* `sqlite-vec`'s documented top-k. Structured files compose with SQL's JSON operators — embed one field instead of the whole file: @@ -93,24 +239,19 @@ ORDER BY vec_distance_cosine(emb, embed('local private models')) LIMIT 10 ``` -::: tip Top-k is `LIMIT k` -If you know `sqlite-vec` you may reach for its `MATCH … AND k = 10` idiom. -That syntax belongs to `sqlite-vec`'s `vec0` virtual table, which the table -above does not use: a `[[table]]`'s own `name` is always a per-file row table. -For plain expressions, `sqlite-vec`'s own documented pattern is exactly what -this guide uses: `ORDER BY vec_distance_cosine(...) LIMIT k`. To get the `vec0` -idiom instead, declare the `vec0` table alongside the row table in the same -[`ddl` batch](../reference/config.md#batch-ddl) and fill it from a trigger. -::: +The same projection works as an `on-file` hook feeding the indexed shape: +parse the field in the hook, store it as the table's `text` column, and the +trigger embeds it once instead of on every query. ## Repeat runs are cheap Computed vectors are cached on disk, keyed on content and model ([vector cache](../plugins.md#vector-cache)) — re-running a search over -unchanged files skips the model entirely and re-embeds only what changed. -And the plugin costs nothing when idle: a query that never calls `embed()` -spawns no worker and loads no model -([zero cost when unused](../plugins.md#zero-cost-when-unused)). +unchanged files skips the model entirely and re-embeds only what changed. That +cuts the *inference* out of the subquery shape, but not the walk or the +per-file round trip; only the `vec0` index removes those. And the plugin costs +nothing when idle: a query that never calls `embed()` spawns no worker and +loads no model ([zero cost when unused](../plugins.md#zero-cost-when-unused)). ## How `embed()` gets into SQL From d617620b18f50b4d325e0a3b2bb65f2b8c8a20c8 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 02:50:28 +0000 Subject: [PATCH 2/6] test: pin the platforms.py <-> platforms.ts mirror (RED) `internals/distcheck`'s `Platform` dataclass is a hand-maintained subset of `packages/ts/src/platforms.ts`, asserted by a comment and checked by nothing. It drifted both ways: `exe` sits on the Python side justified as mirroring the TypeScript source of truth, which has never carried the field, while `triple` and `libc` exist only on the TypeScript side. The same unchecked-mirror mechanism is behind #972 and #996. These tests specify a `dirsql-checks platforms-mirror` gate that holds the two files to the subset framing: the shared fields agree row for row, every published target has a row on both sides, and a Python field with no TypeScript counterpart is drift. The last integration case runs it over the repo's own two files, which is where it earns its keep. Red: `checks.platforms_mirror` does not exist and the CLI has no such subcommand. Refs #1004 --- .../checks/tests/e2e/platforms_mirror_test.py | 98 +++++++++++ .../integration/platforms_mirror_test.py | 165 ++++++++++++++++++ 2 files changed, 263 insertions(+) create mode 100644 internals/checks/tests/e2e/platforms_mirror_test.py create mode 100644 internals/checks/tests/integration/platforms_mirror_test.py diff --git a/internals/checks/tests/e2e/platforms_mirror_test.py b/internals/checks/tests/e2e/platforms_mirror_test.py new file mode 100644 index 000000000..e953e5430 --- /dev/null +++ b/internals/checks/tests/e2e/platforms_mirror_test.py @@ -0,0 +1,98 @@ +"""E2E test for `dirsql-checks platforms-mirror` through the real CLI. + +No mocking of any kind: spawns the packaged `dirsql-checks` console script as a +subprocess, once against real scratch files and once against the repo's own +`platforms.py` / `platforms.ts` through the command's default paths. +""" + +from __future__ import annotations + +import shutil +import subprocess +from pathlib import Path + +REPO = Path(__file__).resolve().parents[4] + +PYTHON_SOURCE = '''\ +from dataclasses import dataclass + + +@dataclass(frozen=True) +class Platform: + node_platform: str + node_arch: str + slug: str + os: list[str] + cpu: list[str] +{extra} + +PLATFORMS: tuple[Platform, ...] = ( + Platform("linux", "x64", "linux-x64-gnu", ["linux"], ["x64"]{value}), +) +''' + +TYPESCRIPT_SOURCE = """\ +export const PLATFORMS: readonly Platform[] = [ + { + triple: "x86_64-unknown-linux-gnu", + nodePlatform: "linux", + nodeArch: "x64", + libName: "@dirsql/lib-linux-x64-gnu", + os: ["linux"], + cpu: ["x64"], + libc: ["glibc"], + }, +]; +""" + + +def _cli() -> str: + dirsql_checks = shutil.which("dirsql-checks") + assert dirsql_checks is not None, ( + "`dirsql-checks` console script not on PATH -- run " + "`uv run --project internals/checks pytest tests/e2e` " + "or `uv sync --project internals/checks`" + ) + return dirsql_checks + + +def _write(tmp_path, *, stray_field: bool): + python = tmp_path / "platforms.py" + python.write_text( + PYTHON_SOURCE.format( + extra=" exe: bool\n" if stray_field else "", + value=", False" if stray_field else "", + ) + ) + typescript = tmp_path / "platforms.ts" + typescript.write_text(TYPESCRIPT_SOURCE) + return ["--python", str(python), "--typescript", str(typescript)] + + +def _invoke(arguments, cwd=None): + return subprocess.run( + [_cli(), "platforms-mirror", *arguments], + capture_output=True, + text=True, + timeout=60, + cwd=cwd, + ) + + +def describe_dirsql_checks_platforms_mirror(): + def it_exits_zero_when_the_subset_agrees(tmp_path): + proc = _invoke(_write(tmp_path, stray_field=False)) + + assert proc.returncode == 0, f"stdout={proc.stdout!r} stderr={proc.stderr!r}" + assert "ok platforms-mirror" in proc.stdout + + def it_exits_nonzero_and_names_the_unmirrored_field(tmp_path): + proc = _invoke(_write(tmp_path, stray_field=True)) + + assert proc.returncode == 1, f"stdout={proc.stdout!r} stderr={proc.stderr!r}" + assert "Platform.exe" in proc.stdout + + def it_holds_the_repos_own_files_through_the_default_paths(): + proc = _invoke([], cwd=REPO) + + assert proc.returncode == 0, f"stdout={proc.stdout!r} stderr={proc.stderr!r}" diff --git a/internals/checks/tests/integration/platforms_mirror_test.py b/internals/checks/tests/integration/platforms_mirror_test.py new file mode 100644 index 000000000..c680d275e --- /dev/null +++ b/internals/checks/tests/integration/platforms_mirror_test.py @@ -0,0 +1,165 @@ +"""Integration tests for the platforms-mirror check against real files. + +Exercises `gate.run` with its default collaborators -- a real `ast` parse of a +real `platforms.py` and a real parse of a real `platforms.ts` -- rather than the +packaged `dirsql-checks` CLI. + +The reproduction is #1004 exactly: `Platform.exe` sat on the Python dataclass for +the stated reason that it "mirrors packages/ts/src/platforms.ts", while the +TypeScript source of truth has never carried the field. A comment asserted the +correspondence and nothing checked it, which is the same mechanism behind #972 +and #996. The last case here is the one that matters day to day: the repo's own +two files, held to the invariant on every run. +""" + +from __future__ import annotations + +from pathlib import Path + +from checks.platforms_mirror.gate import run + +REPO = Path(__file__).resolve().parents[4] +PYTHON = REPO / "internals" / "distcheck" / "src" / "distcheck" / "node_flow" / "platforms.py" +TYPESCRIPT = REPO / "packages" / "ts" / "src" / "platforms.ts" + +ANNOTATIONS = { + "node_platform": "str", + "node_arch": "str", + "slug": "str", + "os": "list[str]", + "cpu": "list[str]", + "exe": "bool", +} + +SUBSET = ("node_platform", "node_arch", "slug", "os", "cpu") + +LINUX = { + "node_platform": "linux", + "node_arch": "x64", + "slug": "linux-x64-gnu", + "os": ["linux"], + "cpu": ["x64"], + "exe": False, +} +WINDOWS = { + "node_platform": "win32", + "node_arch": "x64", + "slug": "win32-x64-msvc", + "os": ["win32"], + "cpu": ["x64"], + "exe": True, +} + +PYTHON_SOURCE = '''\ +from dataclasses import dataclass + + +@dataclass(frozen=True) +class Platform: +{fields} + + @property + def name(self) -> str: + return f"@dirsql/lib-{{self.slug}}" + + +PLATFORMS: tuple[Platform, ...] = ( +{rows} +) +''' + +TYPESCRIPT_SOURCE = """\ +export interface Platform {{ + triple: string; + nodePlatform: NodeJS.Platform; + nodeArch: NodeJS.Architecture; + libName: string; + os: string[]; + cpu: string[]; + libc?: string[]; +}} + +export const PLATFORMS: readonly Platform[] = [ +{rows} +]; +""" + + +def write(tmp_path, python_rows, typescript_rows, fields=SUBSET): + python = tmp_path / "platforms.py" + python.write_text( + PYTHON_SOURCE.format( + fields="\n".join(f" {name}: {ANNOTATIONS[name]}" for name in fields), + rows="\n".join( + f" Platform({', '.join(repr(row[name]) for name in fields)})," + for row in python_rows + ), + ) + ) + typescript = tmp_path / "platforms.ts" + typescript.write_text( + TYPESCRIPT_SOURCE.format( + rows="\n".join( + " {\n" + f' triple: "{row["node_arch"]}-{row["node_platform"]}",\n' + f' nodePlatform: "{row["node_platform"]}",\n' + f' nodeArch: "{row["node_arch"]}",\n' + f' libName: "{row.get("lib_name", "@dirsql/lib-" + row["slug"])}",\n' + f' os: {list(row["os"])!r},\n' + f' cpu: {list(row["cpu"])!r},\n' + " }," + for row in typescript_rows + ).replace("'", '"') + ) + ) + return str(python), str(typescript) + + +def describe_run_against_real_platform_files(): + def it_passes_when_the_declared_subset_agrees(tmp_path, capsys): + assert run(*write(tmp_path, [LINUX, WINDOWS], [LINUX, WINDOWS])) == 0 + assert "ok platforms-mirror" in capsys.readouterr().out + + def it_rejects_a_python_field_with_no_typescript_counterpart(tmp_path, capsys): + # #1004: `exe` justified by a mirror that never had it. + code = run(*write(tmp_path, [LINUX, WINDOWS], [LINUX, WINDOWS], fields=(*SUBSET, "exe"))) + + assert code == 1 + out = capsys.readouterr().out + assert "::error::Platform.exe" in out + assert "no counterpart in packages/ts/src/platforms.ts" in out + + def it_rejects_a_published_target_missing_from_the_python_rows(tmp_path, capsys): + # A new target half-added: published by the TS source of truth, invisible + # to the node distcheck flow. + code = run(*write(tmp_path, [LINUX], [LINUX, WINDOWS])) + + assert code == 1 + assert "::error::win32-x64 is published by platforms.ts" in capsys.readouterr().out + + def it_rejects_a_python_row_that_is_not_a_published_target(tmp_path, capsys): + code = run(*write(tmp_path, [LINUX, WINDOWS], [LINUX])) + + assert code == 1 + assert "::error::win32-x64 has a row in platforms.py" in capsys.readouterr().out + + def it_rejects_a_shared_field_whose_values_disagree(tmp_path, capsys): + drifted = {**WINDOWS, "cpu": ["arm64"]} + code = run(*write(tmp_path, [LINUX, drifted], [LINUX, WINDOWS])) + + assert code == 1 + out = capsys.readouterr().out + assert "::error::win32-x64: cpu is ['arm64'] in platforms.py" in out + assert "['x64'] in platforms.ts" in out + + def it_rejects_a_lib_name_that_does_not_carry_the_sub_package_prefix(tmp_path, capsys): + stray = {**WINDOWS, "lib_name": "@dirsql/win32-x64-msvc"} + code = run(*write(tmp_path, [LINUX, WINDOWS], [LINUX, stray])) + + assert code == 1 + assert "@dirsql/lib-" in capsys.readouterr().out + + +def describe_run_against_the_repos_own_files(): + def it_holds_the_committed_platform_tables_to_the_invariant(capsys): + assert run(str(PYTHON), str(TYPESCRIPT)) == 0, capsys.readouterr().out From c9a387f5ccf1974a5d0153252b5487c662f80e67 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 03:01:21 +0000 Subject: [PATCH 3/6] feat(checks): enforce the platforms.py <-> platforms.ts mirror `dirsql-checks platforms-mirror` reads both platform tables as source text -- the Python one through `ast`, the TypeScript one through a comment-stripping scan into `json` -- and holds them to the subset framing the Python docstring already claimed: `SHARED` names every Python field and the TypeScript property it comes from, both tables must carry the same targets, and a Python field outside `SHARED` is drift by construction. `triple` and `libc` staying TypeScript-only is the subset working, not a gap. Neither side is imported. internals/checks depends on neither internals/distcheck nor a node toolchain, and a check that executes the file it audits can be fooled by the file. A table it cannot parse fails the check rather than passing on an empty read -- a mirror check that silently stops comparing passes forever. With that in place `Platform.exe` goes: nothing outside its own tests read it, and the comment keeping it alive cited a correspondence packages/ts has never had. The "Keep in lockstep" comment it hid behind is replaced by one that states the subset and names the check enforcing it. Runs in its own workflow rather than as a job in internals-distcheck-ci.yml: that lane's path filter does not watch packages/ts, which is exactly where a half-added target starts. Closes #1004 --- .github/workflows/platforms-mirror-check.yml | 44 +++++ internals/checks/src/checks/cli.py | 2 + internals/checks/src/checks/cli_test.py | 4 + .../src/checks/platforms_mirror/__init__.py | 0 .../checks/src/checks/platforms_mirror/cli.py | 29 +++ .../src/checks/platforms_mirror/cli_test.py | 32 ++++ .../src/checks/platforms_mirror/decide.py | 124 ++++++++++++ .../checks/platforms_mirror/decide_test.py | 144 ++++++++++++++ .../src/checks/platforms_mirror/gate.py | 49 +++++ .../src/checks/platforms_mirror/gate_test.py | 94 ++++++++++ .../src/checks/platforms_mirror/parse.py | 177 ++++++++++++++++++ .../src/checks/platforms_mirror/parse_test.py | 136 ++++++++++++++ .../src/distcheck/node_flow/platforms.py | 18 +- .../src/distcheck/node_flow/platforms_test.py | 13 +- 14 files changed, 846 insertions(+), 20 deletions(-) create mode 100644 .github/workflows/platforms-mirror-check.yml create mode 100644 internals/checks/src/checks/platforms_mirror/__init__.py create mode 100644 internals/checks/src/checks/platforms_mirror/cli.py create mode 100644 internals/checks/src/checks/platforms_mirror/cli_test.py create mode 100644 internals/checks/src/checks/platforms_mirror/decide.py create mode 100644 internals/checks/src/checks/platforms_mirror/decide_test.py create mode 100644 internals/checks/src/checks/platforms_mirror/gate.py create mode 100644 internals/checks/src/checks/platforms_mirror/gate_test.py create mode 100644 internals/checks/src/checks/platforms_mirror/parse.py create mode 100644 internals/checks/src/checks/platforms_mirror/parse_test.py diff --git a/.github/workflows/platforms-mirror-check.yml b/.github/workflows/platforms-mirror-check.yml new file mode 100644 index 000000000..c643158a5 --- /dev/null +++ b/.github/workflows/platforms-mirror-check.yml @@ -0,0 +1,44 @@ +name: Platforms Mirror Check + +# `packages/ts/src/platforms.ts` is the release source of truth for the published +# `@dirsql/lib-*` sub-packages; the node distcheck flow keeps a hand-maintained +# Python subset of it. Nothing checked the correspondence, so it drifted in both +# directions -- a field on the Python side justified as mirroring a TypeScript +# property that never existed, and TypeScript-only fields with no counterpart. +# This holds the two tables to the subset on every change to either, so a new +# published target cannot be half-added. +# +# Deliberately its own workflow rather than a job in internals-distcheck-ci.yml: +# that lane's path filter does not watch packages/ts, which is exactly where the +# half-added-target case starts. + +on: + pull_request: + branches: [main] + paths: + - 'packages/ts/src/platforms.ts' + - 'internals/distcheck/src/distcheck/node_flow/platforms.py' + - 'internals/checks/src/checks/platforms_mirror/**' + - '.github/workflows/platforms-mirror-check.yml' + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + platforms-mirror: + name: Platforms mirror + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + + - name: Install uv + uses: astral-sh/setup-uv@v7 + with: + python-version: "3.13" + + - name: Assert the two platform tables agree on the fields they share + run: uv run --project internals/checks dirsql-checks platforms-mirror diff --git a/internals/checks/src/checks/cli.py b/internals/checks/src/checks/cli.py index 6afb41581..89ee15221 100644 --- a/internals/checks/src/checks/cli.py +++ b/internals/checks/src/checks/cli.py @@ -12,6 +12,7 @@ from checks.changelog_gate.cli import cli as changelog_gate from checks.declared_deps.cli import cli as declared_deps from checks.npm_binary_extension_load.cli import cli as npm_binary_extension_load +from checks.platforms_mirror.cli import cli as platforms_mirror from checks.preflight.cli import cli as preflight from checks.pytest_gate.cli import cli as pytest_gate from checks.release_globs.cli import cli as release_globs @@ -27,6 +28,7 @@ def main() -> None: main.add_command(changelog_gate, name="changelog-gate") main.add_command(declared_deps, name="declared-deps") main.add_command(npm_binary_extension_load, name="npm-binary-extension-load") +main.add_command(platforms_mirror, name="platforms-mirror") main.add_command(preflight, name="preflight") main.add_command(pytest_gate, name="pytest-gate") main.add_command(release_globs, name="release-globs") diff --git a/internals/checks/src/checks/cli_test.py b/internals/checks/src/checks/cli_test.py index 24fbb0007..6d73e99e9 100644 --- a/internals/checks/src/checks/cli_test.py +++ b/internals/checks/src/checks/cli_test.py @@ -30,6 +30,10 @@ def test_declared_deps_is_registered(): assert "declared-deps" in main.commands +def test_platforms_mirror_is_registered(): + assert "platforms-mirror" in main.commands + + def test_preflight_is_registered(): assert "preflight" in main.commands diff --git a/internals/checks/src/checks/platforms_mirror/__init__.py b/internals/checks/src/checks/platforms_mirror/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/internals/checks/src/checks/platforms_mirror/cli.py b/internals/checks/src/checks/platforms_mirror/cli.py new file mode 100644 index 000000000..e35a6189c --- /dev/null +++ b/internals/checks/src/checks/platforms_mirror/cli.py @@ -0,0 +1,29 @@ +"""The platforms-mirror check -- repo-only (#1004). + +Backs `dirsql-checks platforms-mirror`: the node distcheck flow's platform table +and the TypeScript release source of truth must agree on the fields they share. +""" + +from __future__ import annotations + +import click + +from checks.platforms_mirror.decide import PYTHON_FILE, TYPESCRIPT_FILE +from checks.platforms_mirror.gate import run + + +@click.command() +@click.option( + "--python", + "python_path", + default=PYTHON_FILE, + help="The node distcheck flow's platform table.", +) +@click.option( + "--typescript", + "typescript_path", + default=TYPESCRIPT_FILE, + help="The release source of truth for published sub-packages.", +) +def cli(python_path: str, typescript_path: str) -> None: + raise SystemExit(run(python_path, typescript_path)) diff --git a/internals/checks/src/checks/platforms_mirror/cli_test.py b/internals/checks/src/checks/platforms_mirror/cli_test.py new file mode 100644 index 000000000..9da7e0ef6 --- /dev/null +++ b/internals/checks/src/checks/platforms_mirror/cli_test.py @@ -0,0 +1,32 @@ +"""Colocated unit tests for the platforms-mirror command (isolation -- no +`CliRunner`). Driven through `.callback`; `run` is mocked at its import site. +""" + +from unittest import mock + +import pytest + +from checks.platforms_mirror.cli import cli + + +def invoke(**kwargs): + with mock.patch("checks.platforms_mirror.cli.run", **kwargs) as run: + with pytest.raises(SystemExit) as exc_info: + cli.callback(python_path="p.py", typescript_path="t.ts") + return run, exc_info.value.code + + +def test_exits_with_runs_return_code(): + run, code = invoke(return_value=0) + run.assert_called_once_with("p.py", "t.ts") + assert code == 0 + + +def test_propagates_a_drifted_mirror_as_exit_one(): + assert invoke(return_value=1)[1] == 1 + + +def test_defaults_to_the_two_committed_platform_tables(): + python_path, typescript_path = cli.params + assert python_path.default == "internals/distcheck/src/distcheck/node_flow/platforms.py" + assert typescript_path.default == "packages/ts/src/platforms.ts" diff --git a/internals/checks/src/checks/platforms_mirror/decide.py b/internals/checks/src/checks/platforms_mirror/decide.py new file mode 100644 index 000000000..0ee8efd07 --- /dev/null +++ b/internals/checks/src/checks/platforms_mirror/decide.py @@ -0,0 +1,124 @@ +"""Pure decision logic for the platforms-mirror check (#1004). + +`internals/distcheck`'s `Platform` dataclass is a hand-maintained subset of +`packages/ts/src/platforms.ts`, which is the release source of truth for the +published `@dirsql/lib-*` sub-packages. The subset framing is deliberate -- +distcheck needs only enough of each target to reconstruct the host's +sub-package and find its staged addon -- but it was asserted by a comment and +checked by nothing, and it drifted both ways: `exe` sat on the Python side +justified as mirroring TypeScript, which has never had the field. + +So the invariant here is the subset, stated as data: `SHARED` names every +Python field and where its value comes from, both tables carry the same set of +targets, and a Python field outside `SHARED` is drift by construction. +`triple` and `libc` staying TypeScript-only is the subset working as intended, +not a gap. +""" + +from __future__ import annotations + +PYTHON_FILE = "internals/distcheck/src/distcheck/node_flow/platforms.py" +TYPESCRIPT_FILE = "packages/ts/src/platforms.ts" + +# Python field -> the TypeScript property it is sourced from. `slug` is the +# sub-package name minus its prefix, which is what `librarySlug()` returns. +SHARED = { + "node_platform": "nodePlatform", + "node_arch": "nodeArch", + "slug": "libName", + "os": "os", + "cpu": "cpu", +} +LIB_PREFIX = "@dirsql/lib-" + + +def key(node_platform: str, node_arch: str) -> str: + return f"{node_platform}-{node_arch}" + + +def unmirrored_fields(fields) -> list[str]: + """One message per dataclass field with no TypeScript source.""" + return [ + f"Platform.{field} has no counterpart in {TYPESCRIPT_FILE}. platforms.py " + f"holds a deliberate subset of the published target ({', '.join(SHARED)}), " + f"so a field the mirror cannot source is drift: delete it, or add the " + f"property to the TypeScript `Platform` interface and to SHARED here." + for field in fields + if field not in SHARED + ] + + +def typescript_value(field: str, row: dict): + """``row``'s value for a Python field name.""" + value = row.get(SHARED[field]) + if field == "slug" and isinstance(value, str): + return value.removeprefix(LIB_PREFIX) + return value + + +def prefix_problems(rows) -> list[str]: + """One message per `libName` that `librarySlug()` would throw on.""" + return [ + f"{key(row.get('nodePlatform'), row.get('nodeArch'))}: libName " + f"{row.get('libName')!r} does not start with {LIB_PREFIX!r}, so " + f"`librarySlug()` throws on it and the Python `slug` cannot be derived. " + f"Fix the name in {TYPESCRIPT_FILE}." + for row in rows + if not str(row.get("libName", "")).startswith(LIB_PREFIX) + ] + + +def missing_rows(python_keys, typescript_rows) -> list[str]: + """One message per published target with no row in platforms.py.""" + return [ + f"{key(row['nodePlatform'], row['nodeArch'])} is published by platforms.ts " + f"but has no row in platforms.py. Add " + f"Platform({row['nodePlatform']!r}, {row['nodeArch']!r}, " + f"{str(row.get('libName', '')).removeprefix(LIB_PREFIX)!r}, {row.get('os')!r}, " + f"{row.get('cpu')!r}) to PLATFORMS in {PYTHON_FILE}, or the node distcheck " + f"flow cannot resolve the new target on that host." + for row in typescript_rows + if key(row["nodePlatform"], row["nodeArch"]) not in python_keys + ] + + +def stray_rows(typescript_keys, python_rows) -> list[str]: + """One message per platforms.py row that is not a published target.""" + return [ + f"{key(row['node_platform'], row['node_arch'])} has a row in platforms.py " + f"but is not a published target in platforms.ts. Either add the target to " + f"PLATFORMS in {TYPESCRIPT_FILE} or drop the row from {PYTHON_FILE}: " + f"distcheck must not resolve a sub-package the release never publishes." + for row in python_rows + if key(row["node_platform"], row["node_arch"]) not in typescript_keys + ] + + +def field_problems(python_row: dict, typescript_row: dict) -> list[str]: + """One message per shared field whose two values disagree.""" + problems = [] + for field in SHARED: + found = python_row.get(field) + expected = typescript_value(field, typescript_row) + if found != expected: + problems.append( + f"{key(python_row['node_platform'], python_row['node_arch'])}: " + f"{field} is {found!r} in platforms.py, {expected!r} in " + f"platforms.ts. platforms.ts is the release source of truth -- " + f"change {PYTHON_FILE} to match." + ) + return problems + + +def problems(fields, python_rows, typescript_rows) -> list[str]: + """Every way the two tables disagree, most structural first.""" + found = unmirrored_fields(fields) + prefix_problems(typescript_rows) + by_key = {key(row["nodePlatform"], row["nodeArch"]): row for row in typescript_rows} + python_keys = {key(row["node_platform"], row["node_arch"]) for row in python_rows} + found += missing_rows(python_keys, typescript_rows) + found += stray_rows(set(by_key), python_rows) + for row in python_rows: + counterpart = by_key.get(key(row["node_platform"], row["node_arch"])) + if counterpart is not None: + found += field_problems(row, counterpart) + return found diff --git a/internals/checks/src/checks/platforms_mirror/decide_test.py b/internals/checks/src/checks/platforms_mirror/decide_test.py new file mode 100644 index 000000000..216c8a49d --- /dev/null +++ b/internals/checks/src/checks/platforms_mirror/decide_test.py @@ -0,0 +1,144 @@ +"""Colocated unit tests for the platforms-mirror decision logic (isolation -- pure +dicts in, messages out). +""" + +from checks.platforms_mirror.decide import ( + SHARED, + field_problems, + missing_rows, + prefix_problems, + problems, + stray_rows, + typescript_value, + unmirrored_fields, +) + +FIELDS = list(SHARED) + +PY_LINUX = { + "node_platform": "linux", + "node_arch": "x64", + "slug": "linux-x64-gnu", + "os": ["linux"], + "cpu": ["x64"], +} +TS_LINUX = { + "triple": "x86_64-unknown-linux-gnu", + "nodePlatform": "linux", + "nodeArch": "x64", + "libName": "@dirsql/lib-linux-x64-gnu", + "os": ["linux"], + "cpu": ["x64"], + "libc": ["glibc"], +} +PY_WIN = { + "node_platform": "win32", + "node_arch": "x64", + "slug": "win32-x64-msvc", + "os": ["win32"], + "cpu": ["x64"], +} +TS_WIN = { + "nodePlatform": "win32", + "nodeArch": "x64", + "libName": "@dirsql/lib-win32-x64-msvc", + "os": ["win32"], + "cpu": ["x64"], +} + + +def describe_unmirrored_fields(): + def it_passes_the_declared_subset(): + assert unmirrored_fields(FIELDS) == [] + + def it_names_a_field_with_no_typescript_source(): + (message,) = unmirrored_fields([*FIELDS, "exe"]) + assert message.startswith("Platform.exe has no counterpart in ") + assert "packages/ts/src/platforms.ts" in message + + def it_does_not_demand_the_typescript_only_fields(): + assert "triple" not in SHARED + assert "libc" not in SHARED + + +def describe_typescript_value(): + def it_strips_the_sub_package_prefix_off_the_slug(): + assert typescript_value("slug", TS_LINUX) == "linux-x64-gnu" + + def it_reads_other_fields_straight_through(): + assert typescript_value("cpu", TS_LINUX) == ["x64"] + + def it_leaves_a_lib_name_without_the_prefix_alone(): + assert typescript_value("slug", {"libName": "@dirsql/nope"}) == "@dirsql/nope" + + def it_returns_none_for_a_missing_property(): + assert typescript_value("slug", {}) is None + + +def describe_prefix_problems(): + def it_passes_well_formed_names(): + assert prefix_problems([TS_LINUX, TS_WIN]) == [] + + def it_names_a_lib_name_library_slug_would_throw_on(): + (message,) = prefix_problems([{**TS_WIN, "libName": "@dirsql/win32-x64-msvc"}]) + assert message.startswith("win32-x64: libName '@dirsql/win32-x64-msvc'") + assert "@dirsql/lib-" in message + + def it_names_a_row_with_no_lib_name_at_all(): + assert len(prefix_problems([{"nodePlatform": "win32", "nodeArch": "x64"}])) == 1 + + +def describe_missing_rows(): + def it_passes_when_every_published_target_has_a_row(): + assert missing_rows({"linux-x64"}, [TS_LINUX]) == [] + + def it_suggests_the_row_to_add(): + (message,) = missing_rows(set(), [TS_LINUX]) + assert message.startswith("linux-x64 is published by platforms.ts") + assert "Platform('linux', 'x64', 'linux-x64-gnu', ['linux'], ['x64'])" in message + + +def describe_stray_rows(): + def it_passes_when_every_row_is_published(): + assert stray_rows({"linux-x64"}, [PY_LINUX]) == [] + + def it_names_a_row_the_release_never_publishes(): + (message,) = stray_rows(set(), [PY_LINUX]) + assert message.startswith("linux-x64 has a row in platforms.py") + assert "packages/ts/src/platforms.ts" in message + + +def describe_field_problems(): + def it_passes_a_row_that_agrees(): + assert field_problems(PY_LINUX, TS_LINUX) == [] + + def it_names_the_field_and_both_values(): + (message,) = field_problems({**PY_LINUX, "cpu": ["arm64"]}, TS_LINUX) + assert message == ( + "linux-x64: cpu is ['arm64'] in platforms.py, ['x64'] in platforms.ts. " + "platforms.ts is the release source of truth -- change " + "internals/distcheck/src/distcheck/node_flow/platforms.py to match." + ) + + def it_reports_every_disagreeing_field(): + drifted = {**PY_LINUX, "cpu": ["arm64"], "slug": "nope"} + assert len(field_problems(drifted, TS_LINUX)) == 2 + + +def describe_problems(): + def it_passes_two_tables_that_mirror(): + assert problems(FIELDS, [PY_LINUX, PY_WIN], [TS_LINUX, TS_WIN]) == [] + + def it_reports_the_structural_problems_before_the_field_ones(): + found = problems( + [*FIELDS, "exe"], + [PY_LINUX, {**PY_WIN, "cpu": ["arm64"]}], + [TS_LINUX, TS_WIN], + ) + assert found[0].startswith("Platform.exe") + assert found[-1].startswith("win32-x64: cpu") + + def it_compares_only_the_rows_present_on_both_sides(): + found = problems(FIELDS, [PY_LINUX, PY_WIN], [TS_LINUX]) + assert len(found) == 1 + assert found[0].startswith("win32-x64 has a row in platforms.py") diff --git a/internals/checks/src/checks/platforms_mirror/gate.py b/internals/checks/src/checks/platforms_mirror/gate.py new file mode 100644 index 000000000..ef9b98695 --- /dev/null +++ b/internals/checks/src/checks/platforms_mirror/gate.py @@ -0,0 +1,49 @@ +"""Orchestration for the platforms-mirror check (#1004). + +Reads both platform tables off disk and holds them to the subset invariant in +`decide.py`. A table this cannot parse fails the check rather than passing on an +empty read: the whole point is that nothing silently stops comparing. +""" + +from __future__ import annotations + +from collections.abc import Callable + +from checks.platforms_mirror.decide import problems +from checks.platforms_mirror.parse import ParseError, python_table, typescript_table + + +def read(path: str) -> str: + with open(path, encoding="utf-8") as handle: + return handle.read() + + +def run( + python_path: str, + typescript_path: str, + *, + source: Callable[[str], str] = read, + echo: Callable[[str], None] = print, +) -> int: + try: + fields, python_rows = python_table(source(python_path)) + typescript_rows = typescript_table(source(typescript_path)) + except ParseError as error: + echo(f"::error::platforms-mirror could not read a platform table: {error}") + return 1 + found = problems(fields, python_rows, typescript_rows) + for problem in found: + echo(f"::error::{problem}") + if found: + echo( + f"platforms-mirror: {len(found)} problem(s). platforms.ts is the release " + f"source of truth for the published sub-packages; platforms.py mirrors the " + f"subset named in " + f"internals/checks/src/checks/platforms_mirror/decide.py's SHARED." + ) + return 1 + echo( + f"ok platforms-mirror: {python_path} mirrors the shared fields of " + f"{typescript_path} across {len(python_rows)} published target(s)." + ) + return 0 diff --git a/internals/checks/src/checks/platforms_mirror/gate_test.py b/internals/checks/src/checks/platforms_mirror/gate_test.py new file mode 100644 index 000000000..072470dd6 --- /dev/null +++ b/internals/checks/src/checks/platforms_mirror/gate_test.py @@ -0,0 +1,94 @@ +"""Colocated unit tests for the platforms-mirror gate (#1004). + +Isolation: the reader is injected, so nothing here touches the repo's real +platforms.py / platforms.ts. The reader itself is exercised against a scratch +file. +""" + +from checks.platforms_mirror.gate import read, run + +PYTHON = '''\ +from dataclasses import dataclass + + +@dataclass(frozen=True) +class Platform: + node_platform: str + node_arch: str + slug: str + os: list[str] + cpu: list[str] +{extra} + +PLATFORMS: tuple[Platform, ...] = ( + Platform("linux", "x64", "linux-x64-gnu", ["linux"], ["x64"]{value}), +) +''' + +TYPESCRIPT = """\ +export const PLATFORMS: readonly Platform[] = [ + { + triple: "x86_64-unknown-linux-gnu", + nodePlatform: "linux", + nodeArch: "x64", + libName: "@dirsql/lib-linux-x64-gnu", + os: ["linux"], + cpu: ["x64"], + libc: ["glibc"], + }, +]; +""" + + +def sources(python=None, typescript=TYPESCRIPT): + text = { + "p.py": PYTHON.format(extra="", value="") if python is None else python, + "t.ts": typescript, + } + return lambda path: text[path] + + +def invoke(**kwargs): + lines = [] + code = run("p.py", "t.ts", source=sources(**kwargs), echo=lines.append) + return code, "\n".join(lines) + + +def describe_read(): + def it_reads_a_file_as_utf8_text(tmp_path): + path = tmp_path / "platforms.ts" + path.write_text("// caffè\n", encoding="utf-8") + assert read(str(path)) == "// caffè\n" + + +def describe_run(): + def it_reports_the_target_count_when_the_subset_agrees(): + code, out = invoke() + assert code == 0 + assert out == ( + "ok platforms-mirror: p.py mirrors the shared fields of t.ts across " + "1 published target(s)." + ) + + def it_fails_on_a_field_with_no_typescript_counterpart(): + code, out = invoke(python=PYTHON.format(extra=" exe: bool\n", value=", False")) + assert code == 1 + assert "::error::Platform.exe has no counterpart" in out + assert "platforms-mirror: 1 problem(s)." in out + + def it_names_decide_py_so_the_shared_subset_can_be_found(): + code, out = invoke(python=PYTHON.format(extra=" exe: bool\n", value=", False")) + assert code == 1 + assert "platforms_mirror/decide.py's SHARED" in out + + def it_fails_when_the_python_table_cannot_be_read(): + code, out = invoke(python="x = 1\n") + assert code == 1 + assert "::error::platforms-mirror could not read a platform table:" in out + assert "no `class Platform`" in out + + def it_fails_when_the_typescript_table_cannot_be_read(): + code, out = invoke(typescript="export const OTHER = [];\n") + assert code == 1 + assert "could not read a platform table:" in out + assert "no `PLATFORMS" in out diff --git a/internals/checks/src/checks/platforms_mirror/parse.py b/internals/checks/src/checks/platforms_mirror/parse.py new file mode 100644 index 000000000..a7cefd122 --- /dev/null +++ b/internals/checks/src/checks/platforms_mirror/parse.py @@ -0,0 +1,177 @@ +"""Reading the two platform tables as source text (#1004). + +Neither side is imported: `internals/checks` depends on neither +`internals/distcheck` nor a node toolchain, and a check that executes the file +it audits can be fooled by the file. The Python table comes out of `ast`; the +TypeScript one out of a scan that strips comments and hands the remaining +object literal to `json`. + +Every shape this cannot read raises `ParseError` rather than returning an empty +table -- a mirror check that silently sees no rows passes forever. +""" + +from __future__ import annotations + +import ast +import json +import re + +CLASS_NAME = "Platform" +TABLE_NAME = "PLATFORMS" + + +class ParseError(Exception): + """A platform table that could not be read in the shape this check expects.""" + + +def _dataclass_fields(tree: ast.Module) -> list[str]: + for node in tree.body: + if isinstance(node, ast.ClassDef) and node.name == CLASS_NAME: + fields = [ + statement.target.id + for statement in node.body + if isinstance(statement, ast.AnnAssign) + and isinstance(statement.target, ast.Name) + ] + if not fields: + raise ParseError( + f"class {CLASS_NAME} declares no annotated fields; this check reads " + f"the mirrored field list off them." + ) + return fields + raise ParseError( + f"no `class {CLASS_NAME}` at module level; this check reads the mirrored " + f"field list off its annotations." + ) + + +def _table_elements(tree: ast.Module) -> list[ast.expr]: + for node in tree.body: + target = None + if isinstance(node, ast.AnnAssign) and isinstance(node.target, ast.Name): + target = node.target.id + elif isinstance(node, ast.Assign) and len(node.targets) == 1: + first = node.targets[0] + target = first.id if isinstance(first, ast.Name) else None + if target == TABLE_NAME: + if not isinstance(node.value, (ast.Tuple, ast.List)): + raise ParseError( + f"{TABLE_NAME} is not a tuple or list literal; this check reads its " + f"rows statically and cannot evaluate a computed table." + ) + return list(node.value.elts) + raise ParseError(f"no module-level `{TABLE_NAME} = (...)` assignment.") + + +def _row(element: ast.expr, fields: list[str]) -> dict: + if not ( + isinstance(element, ast.Call) + and isinstance(element.func, ast.Name) + and element.func.id == CLASS_NAME + ): + raise ParseError( + f"every {TABLE_NAME} entry must be a literal `{CLASS_NAME}(...)` call." + ) + if len(element.args) > len(fields): + raise ParseError( + f"a {CLASS_NAME}(...) row passes {len(element.args)} positional arguments " + f"but {CLASS_NAME} declares {len(fields)} fields." + ) + row = dict(zip(fields, (_literal(argument) for argument in element.args))) + for keyword in element.keywords: + if keyword.arg is None: + raise ParseError(f"a {CLASS_NAME}(...) row splats **kwargs; read it statically.") + row[keyword.arg] = _literal(keyword.value) + return row + + +def _literal(node: ast.expr): + try: + return ast.literal_eval(node) + except ValueError as error: + raise ParseError( + f"a {CLASS_NAME}(...) argument is not a literal: {error}" + ) from error + + +def python_table(source: str) -> tuple[list[str], list[dict]]: + """`(field names, rows)` from a `platforms.py`-shaped module.""" + tree = ast.parse(source) + fields = _dataclass_fields(tree) + return fields, [_row(element, fields) for element in _table_elements(tree)] + + +def _literal_text(source: str, name: str) -> str: + """The bracketed literal assigned to ``name``, comments stripped. + + Scanned rather than matched: the array spans lines, holds nested objects, + and carries `//` comments that a regex would either keep or cut mid-string. + """ + marker = re.search(rf"\b{name}\b[^=;]*=\s*\[", source) + if marker is None: + raise ParseError(f"no `{name} = [...]` assignment in the TypeScript source.") + index = marker.end() - 1 + depth = 0 + out: list[str] = [] + while index < len(source): + character = source[index] + if character in "\"'`": + index = _copy_string(source, index, out) + continue + if source.startswith("//", index): + newline = source.find("\n", index) + if newline == -1: + break + index = newline + continue + if source.startswith("/*", index): + end = source.find("*/", index) + if end == -1: + raise ParseError(f"unterminated block comment inside `{name}`.") + index = end + 2 + continue + if character in "[{": + depth += 1 + elif character in "]}": + depth -= 1 + out.append(character) + index += 1 + if depth == 0: + return "".join(out) + raise ParseError(f"unbalanced brackets in `{name}`.") + + +def _copy_string(source: str, index: int, out: list[str]) -> int: + quote = source[index] + out.append('"') + index += 1 + while index < len(source) and source[index] != quote: + if source[index] == "\\": + out.append(source[index]) + index += 1 + if index >= len(source): + break + out.append(source[index]) + index += 1 + if index >= len(source): + raise ParseError("unterminated string literal in the TypeScript source.") + out.append('"') + return index + 1 + + +def typescript_table(source: str) -> list[dict]: + """Rows of the `PLATFORMS` array in a `platforms.ts`-shaped module.""" + literal = _literal_text(source, TABLE_NAME) + keyed = re.sub(r"([{,]\s*)([A-Za-z_$][\w$]*)\s*:", r'\1"\2":', literal) + without_trailing_commas = re.sub(r",(\s*[}\]])", r"\1", keyed) + try: + rows = json.loads(without_trailing_commas) + except json.JSONDecodeError as error: + raise ParseError( + f"`{TABLE_NAME}` is not a plain array of object literals ({error}); this " + f"check reads it as data and cannot evaluate spreads, computed keys or " + f"identifiers." + ) from error + if not all(isinstance(row, dict) for row in rows): + raise ParseError(f"every `{TABLE_NAME}` entry must be an object literal.") + return rows diff --git a/internals/checks/src/checks/platforms_mirror/parse_test.py b/internals/checks/src/checks/platforms_mirror/parse_test.py new file mode 100644 index 000000000..89766a4fe --- /dev/null +++ b/internals/checks/src/checks/platforms_mirror/parse_test.py @@ -0,0 +1,136 @@ +"""Colocated unit tests for the platform-table readers (isolation -- pure text in, +data out; nothing here touches the repo's real platforms.py / platforms.ts). +""" + +import pytest + +from checks.platforms_mirror.parse import ParseError, python_table, typescript_table + +PYTHON = '''\ +from dataclasses import dataclass + + +@dataclass(frozen=True) +class Platform: + node_platform: str + node_arch: str + slug: str + + +PLATFORMS: tuple[Platform, ...] = ( + Platform("linux", "x64", "linux-x64-gnu"), + Platform("win32", "x64", slug="win32-x64-msvc", node_arch="x64"), +) +''' + +TYPESCRIPT = """\ +// Leading comment with a "quoted" brace { and a stray ]. +export const PLATFORMS: readonly Platform[] = [ + { + /* block comment */ + triple: "x86_64-unknown-linux-gnu", + nodePlatform: "linux", + os: ["linux"], // trailing comment + }, +]; +""" + + +def describe_python_table(): + def it_reads_the_annotated_fields_and_the_rows(): + fields, rows = python_table(PYTHON) + assert fields == ["node_platform", "node_arch", "slug"] + assert rows[0] == {"node_platform": "linux", "node_arch": "x64", "slug": "linux-x64-gnu"} + + def it_lets_keywords_override_positionals(): + assert python_table(PYTHON)[1][1] == { + "node_platform": "win32", + "node_arch": "x64", + "slug": "win32-x64-msvc", + } + + def it_reads_a_plain_assignment_without_an_annotation(): + source = PYTHON.replace("PLATFORMS: tuple[Platform, ...] =", "PLATFORMS =") + assert len(python_table(source)[1]) == 2 + + def it_rejects_a_module_with_no_platform_class(): + with pytest.raises(ParseError, match="no `class Platform`"): + python_table("PLATFORMS = ()\n") + + def it_rejects_a_platform_class_with_no_annotated_fields(): + with pytest.raises(ParseError, match="declares no annotated fields"): + python_table("class Platform:\n pass\n\n\nPLATFORMS = ()\n") + + def it_rejects_a_module_with_no_table(): + with pytest.raises(ParseError, match="no module-level"): + python_table("class Platform:\n slug: str\n") + + def it_rejects_a_computed_table(): + source = PYTHON.replace("PLATFORMS: tuple[Platform, ...] = (", "PLATFORMS = tuple(") + with pytest.raises(ParseError, match="not a tuple or list literal"): + python_table(source.replace(")\n", ")\n", 1)) + + def it_rejects_a_row_that_is_not_a_platform_call(): + with pytest.raises(ParseError, match="literal `Platform"): + python_table("class Platform:\n slug: str\n\n\nPLATFORMS = (1,)\n") + + def it_rejects_a_row_with_more_positionals_than_fields(): + with pytest.raises(ParseError, match="positional arguments"): + python_table('class Platform:\n slug: str\n\n\nPLATFORMS = (Platform("a", "b"),)\n') + + def it_rejects_a_row_that_splats_keywords(): + with pytest.raises(ParseError, match="splats"): + python_table("class Platform:\n slug: str\n\n\nPLATFORMS = (Platform(**row),)\n") + + def it_rejects_a_non_literal_argument(): + with pytest.raises(ParseError, match="not a literal"): + python_table("class Platform:\n slug: str\n\n\nPLATFORMS = (Platform(slug),)\n") + + +def describe_typescript_table(): + def it_reads_the_array_through_comments_and_trailing_commas(): + assert typescript_table(TYPESCRIPT) == [ + { + "triple": "x86_64-unknown-linux-gnu", + "nodePlatform": "linux", + "os": ["linux"], + } + ] + + def it_normalizes_single_quoted_strings(): + assert typescript_table("const PLATFORMS = [{ os: ['linux'] }];") == [{"os": ["linux"]}] + + def it_keeps_an_escape_sequence_inside_a_string(): + assert typescript_table(r'const PLATFORMS = [{ os: "a\"b" }];') == [{"os": 'a"b'}] + + def it_rejects_a_source_with_no_table(): + with pytest.raises(ParseError, match="no `PLATFORMS"): + typescript_table("export const OTHER = [];\n") + + def it_rejects_an_unbalanced_array(): + with pytest.raises(ParseError, match="unbalanced"): + typescript_table("const PLATFORMS = [{ os: [] }\n") + + def it_rejects_an_unterminated_block_comment(): + with pytest.raises(ParseError, match="unterminated block comment"): + typescript_table("const PLATFORMS = [ /* open\n") + + def it_rejects_an_unterminated_string(): + with pytest.raises(ParseError, match="unterminated string"): + typescript_table('const PLATFORMS = [{ os: "open\n') + + def it_rejects_an_unterminated_string_ending_in_a_backslash(): + with pytest.raises(ParseError, match="unterminated string"): + typescript_table('const PLATFORMS = [{ os: "open\\') + + def it_rejects_a_line_comment_that_runs_to_the_end_of_the_file(): + with pytest.raises(ParseError, match="unbalanced"): + typescript_table("const PLATFORMS = [ // open") + + def it_rejects_an_entry_that_is_not_an_object_literal(): + with pytest.raises(ParseError, match="object literal"): + typescript_table('const PLATFORMS = ["linux"];') + + def it_rejects_a_table_it_cannot_read_as_data(): + with pytest.raises(ParseError, match="not a plain array"): + typescript_table("const PLATFORMS = [{ ...spread }];") diff --git a/internals/distcheck/src/distcheck/node_flow/platforms.py b/internals/distcheck/src/distcheck/node_flow/platforms.py index 66e8bbaa0..3f4188330 100644 --- a/internals/distcheck/src/distcheck/node_flow/platforms.py +++ b/internals/distcheck/src/distcheck/node_flow/platforms.py @@ -18,7 +18,6 @@ class Platform: slug: str os: list[str] cpu: list[str] - exe: bool @property def name(self) -> str: @@ -35,15 +34,16 @@ def addon_name(self) -> str: return f"dirsql.{self.slug}.node" -# Mirror of PLATFORMS in packages/ts/src/platforms.ts (the fields the node distcheck -# flow needs). Keep in lockstep with that file -- it is the release source of -# truth for the published sub-packages. +# The subset of PLATFORMS in packages/ts/src/platforms.ts this flow needs -- that +# file is the release source of truth for the published sub-packages, and its +# `triple` and `libc` reconstruct nothing here. `dirsql-checks platforms-mirror` +# holds the two tables to that subset, in both directions. PLATFORMS: tuple[Platform, ...] = ( - Platform("linux", "x64", "linux-x64-gnu", ["linux"], ["x64"], False), - Platform("linux", "arm64", "linux-arm64-gnu", ["linux"], ["arm64"], False), - Platform("darwin", "x64", "darwin-x64", ["darwin"], ["x64"], False), - Platform("darwin", "arm64", "darwin-arm64", ["darwin"], ["arm64"], False), - Platform("win32", "x64", "win32-x64-msvc", ["win32"], ["x64"], True), + Platform("linux", "x64", "linux-x64-gnu", ["linux"], ["x64"]), + Platform("linux", "arm64", "linux-arm64-gnu", ["linux"], ["arm64"]), + Platform("darwin", "x64", "darwin-x64", ["darwin"], ["x64"]), + Platform("darwin", "arm64", "darwin-arm64", ["darwin"], ["arm64"]), + Platform("win32", "x64", "win32-x64-msvc", ["win32"], ["x64"]), ) _ARCH = { diff --git a/internals/distcheck/src/distcheck/node_flow/platforms_test.py b/internals/distcheck/src/distcheck/node_flow/platforms_test.py index 8047a891b..8569279e7 100644 --- a/internals/distcheck/src/distcheck/node_flow/platforms_test.py +++ b/internals/distcheck/src/distcheck/node_flow/platforms_test.py @@ -27,7 +27,6 @@ def test_find_host_platform_linux_x64(): assert p.addon_name == "dirsql.linux-x64-gnu.node" assert p.os == ["linux"] assert p.cpu == ["x64"] - assert p.exe is False def test_find_host_platform_arch_is_significant(): @@ -36,10 +35,9 @@ def test_find_host_platform_arch_is_significant(): def test_find_host_platform_windows_addon_has_no_exe_suffix(): - # The `.exe` suffix belonged to the standalone binary, which is no longer - # published (#739); an addon is a `.node` on every platform. + # An addon is a `.node` on every platform; the `.exe` suffix belonged to the + # standalone binary, which is no longer published (#739). p = find_host_platform("win32", "x64") - assert p.exe is True assert p.addon_name == "dirsql.win32-x64-msvc.node" @@ -58,13 +56,6 @@ def test_every_platform_addon_name_matches_slug(): assert p.addon_name == f"dirsql.{p.slug}.node" -def test_only_windows_rows_flag_exe(): - # `exe` still marks the Windows row (it mirrors packages/ts/src/platforms.ts), - # but nothing derives a filename from it now that no binary is published. - for p in PLATFORMS: - assert p.exe is (p.node_platform == "win32") - - def test_platform_is_immutable(): p = find_host_platform("linux", "x64") with pytest.raises(AttributeError): From abdcae0e0b2e994d22a4bc7b1e97d86925eb2399 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 03:01:31 +0000 Subject: [PATCH 4/6] e2e attestation for claude/issues-1003-1004-kfn8ug --- .../e2e-attestations/claude-issues-1003-1004-kfn8ug.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 internals/checks/e2e-attestations/claude-issues-1003-1004-kfn8ug.json diff --git a/internals/checks/e2e-attestations/claude-issues-1003-1004-kfn8ug.json b/internals/checks/e2e-attestations/claude-issues-1003-1004-kfn8ug.json new file mode 100644 index 000000000..f9872eb2e --- /dev/null +++ b/internals/checks/e2e-attestations/claude-issues-1003-1004-kfn8ug.json @@ -0,0 +1,7 @@ +{ + "command": "uv run python -m pytest tests/e2e -x -q", + "ran_at": 1787281290, + "exit_code": 0, + "commit": "c9a387f5ccf1974a5d0153252b5487c662f80e67", + "branch": "claude/issues-1003-1004-kfn8ug" +} From 6a12c1e249433d77a97163f48a016282eaf40c22 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 03:24:51 +0000 Subject: [PATCH 5/6] test(checks): kill the platforms-mirror mutation survivors CI found 33 surviving mutants the local run had not: the gate scopes to the diff, and at the time it ran the new modules were still untracked, so it mutated nothing in them. Run it after committing, not before. Most survivors were unkillable by any assertion rather than merely untested. Comparing a parsed identifier against a module-level string literal admits an `==` -> `is` mutant that CPython's interning makes genuinely equivalent, so the name comparisons become dict and set lookups instead: classes and module bindings are collected by name and looked up, and a row's callee is tested for membership in ROW_CALLS. `_assigned_names` also drops the `len(targets) == 1` guard and its `targets[0]`, whose `[-1]` mutant was equivalent under that very guard -- and it now reads `A = PLATFORMS = (...)` correctly as a side effect. The hand-rolled TypeScript scanner is gone, and with it seventeen survivors in its index arithmetic and bounds checks. Comments and string literals are now one regex alternation, and `json`'s own `raw_decode` finds the end of the array, so nothing counts brackets. The three bespoke "unterminated X" errors collapse into one message that names every shape it cannot read. `decide.typescript_value` keys its per-field derivation off a DERIVE table rather than comparing the field name, and `gate.run` drops the keyword-only marker, which cosmic-ray mutates to the positional-only one -- accepted by the interpreter and unobservable to every caller. New assertions cover what remains: the positional-arity guard from both sides, a field whose value sorts above platforms.ts as well as below, a class and a table bound under names sorting either side of the real ones, and the string unwrapping pinned to an exact value. Refs #1004 --- .../src/checks/platforms_mirror/decide.py | 19 +- .../checks/platforms_mirror/decide_test.py | 16 ++ .../src/checks/platforms_mirror/gate.py | 1 - .../src/checks/platforms_mirror/parse.py | 212 ++++++++---------- .../src/checks/platforms_mirror/parse_test.py | 71 ++++-- 5 files changed, 176 insertions(+), 143 deletions(-) diff --git a/internals/checks/src/checks/platforms_mirror/decide.py b/internals/checks/src/checks/platforms_mirror/decide.py index 0ee8efd07..0c20b1a52 100644 --- a/internals/checks/src/checks/platforms_mirror/decide.py +++ b/internals/checks/src/checks/platforms_mirror/decide.py @@ -32,6 +32,16 @@ LIB_PREFIX = "@dirsql/lib-" +def _slug(lib_name: str) -> str: + return lib_name.removeprefix(LIB_PREFIX) + + +# Python field -> how its TypeScript value is derived, for the fields that are +# not a straight read. Keyed rather than branched so the field name is never +# compared, only looked up. +DERIVE = {"slug": _slug} + + def key(node_platform: str, node_arch: str) -> str: return f"{node_platform}-{node_arch}" @@ -51,9 +61,10 @@ def unmirrored_fields(fields) -> list[str]: def typescript_value(field: str, row: dict): """``row``'s value for a Python field name.""" value = row.get(SHARED[field]) - if field == "slug" and isinstance(value, str): - return value.removeprefix(LIB_PREFIX) - return value + derive = DERIVE.get(field) + if derive is None or not isinstance(value, str): + return value + return derive(value) def prefix_problems(rows) -> list[str]: @@ -74,7 +85,7 @@ def missing_rows(python_keys, typescript_rows) -> list[str]: f"{key(row['nodePlatform'], row['nodeArch'])} is published by platforms.ts " f"but has no row in platforms.py. Add " f"Platform({row['nodePlatform']!r}, {row['nodeArch']!r}, " - f"{str(row.get('libName', '')).removeprefix(LIB_PREFIX)!r}, {row.get('os')!r}, " + f"{_slug(str(row.get('libName', '')))!r}, {row.get('os')!r}, " f"{row.get('cpu')!r}) to PLATFORMS in {PYTHON_FILE}, or the node distcheck " f"flow cannot resolve the new target on that host." for row in typescript_rows diff --git a/internals/checks/src/checks/platforms_mirror/decide_test.py b/internals/checks/src/checks/platforms_mirror/decide_test.py index 216c8a49d..a33e8a275 100644 --- a/internals/checks/src/checks/platforms_mirror/decide_test.py +++ b/internals/checks/src/checks/platforms_mirror/decide_test.py @@ -3,7 +3,9 @@ """ from checks.platforms_mirror.decide import ( + DERIVE, SHARED, + _slug, field_problems, missing_rows, prefix_problems, @@ -71,6 +73,14 @@ def it_reads_other_fields_straight_through(): def it_leaves_a_lib_name_without_the_prefix_alone(): assert typescript_value("slug", {"libName": "@dirsql/nope"}) == "@dirsql/nope" + def it_derives_only_the_fields_that_need_it(): + assert set(DERIVE) == {"slug"} + assert _slug("@dirsql/lib-darwin-arm64") == "darwin-arm64" + + def it_does_not_strip_a_prefix_off_a_straight_read_field(): + row = {"nodePlatform": "@dirsql/lib-linux"} + assert typescript_value("node_platform", row) == "@dirsql/lib-linux" + def it_returns_none_for_a_missing_property(): assert typescript_value("slug", {}) is None @@ -124,6 +134,12 @@ def it_reports_every_disagreeing_field(): drifted = {**PY_LINUX, "cpu": ["arm64"], "slug": "nope"} assert len(field_problems(drifted, TS_LINUX)) == 2 + def it_reports_a_value_that_sorts_after_the_typescript_one(): + # The pair above drifts *below* platforms.ts; this one drifts above, so + # neither ordering can stand in for the inequality. + (message,) = field_problems({**PY_LINUX, "slug": "zzz"}, TS_LINUX) + assert message.startswith("linux-x64: slug is 'zzz' in platforms.py") + def describe_problems(): def it_passes_two_tables_that_mirror(): diff --git a/internals/checks/src/checks/platforms_mirror/gate.py b/internals/checks/src/checks/platforms_mirror/gate.py index ef9b98695..45ae7bebe 100644 --- a/internals/checks/src/checks/platforms_mirror/gate.py +++ b/internals/checks/src/checks/platforms_mirror/gate.py @@ -21,7 +21,6 @@ def read(path: str) -> str: def run( python_path: str, typescript_path: str, - *, source: Callable[[str], str] = read, echo: Callable[[str], None] = print, ) -> int: diff --git a/internals/checks/src/checks/platforms_mirror/parse.py b/internals/checks/src/checks/platforms_mirror/parse.py index a7cefd122..0a6c3e99a 100644 --- a/internals/checks/src/checks/platforms_mirror/parse.py +++ b/internals/checks/src/checks/platforms_mirror/parse.py @@ -3,8 +3,9 @@ Neither side is imported: `internals/checks` depends on neither `internals/distcheck` nor a node toolchain, and a check that executes the file it audits can be fooled by the file. The Python table comes out of `ast`; the -TypeScript one out of a scan that strips comments and hands the remaining -object literal to `json`. +TypeScript one is stripped of comments, normalized to JSON, and handed to +`json`'s own decoder, which finds the end of the array so nothing here counts +brackets. Every shape this cannot read raises `ParseError` rather than returning an empty table -- a mirror check that silently sees no rows passes forever. @@ -18,6 +19,15 @@ CLASS_NAME = "Platform" TABLE_NAME = "PLATFORMS" +ROW_CALLS = frozenset({CLASS_NAME}) + +# One alternation of "things a naive scan would cut in half": the three string +# forms first, so a `//` inside a string is matched as string rather than as the +# comment that follows it. +_TOKENS = re.compile( + r'"(?:\\.|[^"\\])*"' r"|'(?:\\.|[^'\\])*'" r"|`(?:\\.|[^`\\])*`" r"|//[^\n]*" r"|/\*.*?\*/", + re.S, +) class ParseError(Exception): @@ -25,57 +35,71 @@ class ParseError(Exception): def _dataclass_fields(tree: ast.Module) -> list[str]: - for node in tree.body: - if isinstance(node, ast.ClassDef) and node.name == CLASS_NAME: - fields = [ - statement.target.id - for statement in node.body - if isinstance(statement, ast.AnnAssign) - and isinstance(statement.target, ast.Name) - ] - if not fields: - raise ParseError( - f"class {CLASS_NAME} declares no annotated fields; this check reads " - f"the mirrored field list off them." - ) - return fields - raise ParseError( - f"no `class {CLASS_NAME}` at module level; this check reads the mirrored " - f"field list off its annotations." - ) + classes = {node.name: node for node in tree.body if isinstance(node, ast.ClassDef)} + declaration = classes.get(CLASS_NAME) + if declaration is None: + raise ParseError( + f"no `class {CLASS_NAME}` at module level; this check reads the mirrored " + f"field list off its annotations." + ) + fields = [ + statement.target.id + for statement in declaration.body + if isinstance(statement, ast.AnnAssign) and isinstance(statement.target, ast.Name) + ] + if not fields: + raise ParseError( + f"class {CLASS_NAME} declares no annotated fields; this check reads the " + f"mirrored field list off them." + ) + return fields -def _table_elements(tree: ast.Module) -> list[ast.expr]: - for node in tree.body: - target = None - if isinstance(node, ast.AnnAssign) and isinstance(node.target, ast.Name): - target = node.target.id - elif isinstance(node, ast.Assign) and len(node.targets) == 1: - first = node.targets[0] - target = first.id if isinstance(first, ast.Name) else None - if target == TABLE_NAME: - if not isinstance(node.value, (ast.Tuple, ast.List)): - raise ParseError( - f"{TABLE_NAME} is not a tuple or list literal; this check reads its " - f"rows statically and cannot evaluate a computed table." - ) - return list(node.value.elts) - raise ParseError(f"no module-level `{TABLE_NAME} = (...)` assignment.") +def _assigned_names(node: ast.stmt) -> list[str]: + """The module-level names ``node`` binds, empty when it binds none.""" + if isinstance(node, ast.AnnAssign): + targets = [node.target] + elif isinstance(node, ast.Assign): + targets = node.targets + else: + return [] + return [target.id for target in targets if isinstance(target, ast.Name)] -def _row(element: ast.expr, fields: list[str]) -> dict: - if not ( - isinstance(element, ast.Call) - and isinstance(element.func, ast.Name) - and element.func.id == CLASS_NAME - ): +def _table_elements(tree: ast.Module) -> list[ast.expr]: + bindings = {name: node for node in tree.body for name in _assigned_names(node)} + assignment = bindings.get(TABLE_NAME) + if assignment is None: + raise ParseError(f"no module-level `{TABLE_NAME} = (...)` assignment.") + if not isinstance(assignment.value, (ast.Tuple, ast.List)): raise ParseError( - f"every {TABLE_NAME} entry must be a literal `{CLASS_NAME}(...)` call." + f"{TABLE_NAME} is not a tuple or list literal; this check reads its rows " + f"statically and cannot evaluate a computed table." ) + return list(assignment.value.elts) + + +def _call_name(element: ast.expr): + """The name of a plain `Name(...)` call, else ``None``.""" + if isinstance(element, ast.Call) and isinstance(element.func, ast.Name): + return element.func.id + return None + + +def _literal(node: ast.expr): + try: + return ast.literal_eval(node) + except ValueError as error: + raise ParseError(f"a {CLASS_NAME}(...) argument is not a literal: {error}") from error + + +def _row(element: ast.expr, fields: list[str]) -> dict: + if _call_name(element) not in ROW_CALLS: + raise ParseError(f"every {TABLE_NAME} entry must be a literal `{CLASS_NAME}(...)` call.") if len(element.args) > len(fields): raise ParseError( - f"a {CLASS_NAME}(...) row passes {len(element.args)} positional arguments " - f"but {CLASS_NAME} declares {len(fields)} fields." + f"a {CLASS_NAME}(...) row passes {len(element.args)} positional arguments but " + f"{CLASS_NAME} declares {len(fields)} fields." ) row = dict(zip(fields, (_literal(argument) for argument in element.args))) for keyword in element.keywords: @@ -85,15 +109,6 @@ def _row(element: ast.expr, fields: list[str]) -> dict: return row -def _literal(node: ast.expr): - try: - return ast.literal_eval(node) - except ValueError as error: - raise ParseError( - f"a {CLASS_NAME}(...) argument is not a literal: {error}" - ) from error - - def python_table(source: str) -> tuple[list[str], list[dict]]: """`(field names, rows)` from a `platforms.py`-shaped module.""" tree = ast.parse(source) @@ -101,76 +116,41 @@ def python_table(source: str) -> tuple[list[str], list[dict]]: return fields, [_row(element, fields) for element in _table_elements(tree)] -def _literal_text(source: str, name: str) -> str: - """The bracketed literal assigned to ``name``, comments stripped. +def _requoted(text: str) -> str: + """A TypeScript string literal as a JSON one.""" + if text.startswith('"'): + return text + return json.dumps(text[1:-1].replace('\\"', '"').replace("\\'", "'")) - Scanned rather than matched: the array spans lines, holds nested objects, - and carries `//` comments that a regex would either keep or cut mid-string. - """ - marker = re.search(rf"\b{name}\b[^=;]*=\s*\[", source) - if marker is None: - raise ParseError(f"no `{name} = [...]` assignment in the TypeScript source.") - index = marker.end() - 1 - depth = 0 - out: list[str] = [] - while index < len(source): - character = source[index] - if character in "\"'`": - index = _copy_string(source, index, out) - continue - if source.startswith("//", index): - newline = source.find("\n", index) - if newline == -1: - break - index = newline - continue - if source.startswith("/*", index): - end = source.find("*/", index) - if end == -1: - raise ParseError(f"unterminated block comment inside `{name}`.") - index = end + 2 - continue - if character in "[{": - depth += 1 - elif character in "]}": - depth -= 1 - out.append(character) - index += 1 - if depth == 0: - return "".join(out) - raise ParseError(f"unbalanced brackets in `{name}`.") - - -def _copy_string(source: str, index: int, out: list[str]) -> int: - quote = source[index] - out.append('"') - index += 1 - while index < len(source) and source[index] != quote: - if source[index] == "\\": - out.append(source[index]) - index += 1 - if index >= len(source): - break - out.append(source[index]) - index += 1 - if index >= len(source): - raise ParseError("unterminated string literal in the TypeScript source.") - out.append('"') - return index + 1 + +def _without_comments(source: str) -> str: + def keep(match: re.Match) -> str: + text = match.group(0) + return "" if text.startswith(("//", "/*")) else _requoted(text) + + return _TOKENS.sub(keep, source) + + +def _as_json(text: str) -> str: + keyed = re.sub(r"([{,]\s*)([A-Za-z_$][\w$]*)\s*:", r'\1"\2":', text) + return re.sub(r",(\s*[}\]])", r"\1", keyed) def typescript_table(source: str) -> list[dict]: """Rows of the `PLATFORMS` array in a `platforms.ts`-shaped module.""" - literal = _literal_text(source, TABLE_NAME) - keyed = re.sub(r"([{,]\s*)([A-Za-z_$][\w$]*)\s*:", r'\1"\2":', literal) - without_trailing_commas = re.sub(r",(\s*[}\]])", r"\1", keyed) + cleaned = _without_comments(source) + marker = re.search(rf"\b{TABLE_NAME}\b[^=;]*=\s*(\[)", cleaned) + if marker is None: + raise ParseError(f"no `{TABLE_NAME} = [...]` assignment in the TypeScript source.") try: - rows = json.loads(without_trailing_commas) + # `raw_decode` stops at the end of the array, so nothing here has to find + # the closing bracket or care what follows it. + rows, _ = json.JSONDecoder().raw_decode(_as_json(cleaned[marker.start(1) :])) except json.JSONDecodeError as error: raise ParseError( - f"`{TABLE_NAME}` is not a plain array of object literals ({error}); this " - f"check reads it as data and cannot evaluate spreads, computed keys or " - f"identifiers." + f"`{TABLE_NAME}` is not a plain array of object literals ({error}); this check " + f"reads it as data and cannot evaluate spreads, computed keys or identifiers, " + f"and cannot read an unterminated array, string or comment." ) from error if not all(isinstance(row, dict) for row in rows): raise ParseError(f"every `{TABLE_NAME}` entry must be an object literal.") diff --git a/internals/checks/src/checks/platforms_mirror/parse_test.py b/internals/checks/src/checks/platforms_mirror/parse_test.py index 89766a4fe..a7b601bd3 100644 --- a/internals/checks/src/checks/platforms_mirror/parse_test.py +++ b/internals/checks/src/checks/platforms_mirror/parse_test.py @@ -53,7 +53,12 @@ def it_reads_a_plain_assignment_without_an_annotation(): source = PYTHON.replace("PLATFORMS: tuple[Platform, ...] =", "PLATFORMS =") assert len(python_table(source)[1]) == 2 - def it_rejects_a_module_with_no_platform_class(): + @pytest.mark.parametrize("name", ["Machine", "Target"]) + def it_rejects_a_module_whose_only_class_is_named_otherwise(name): + with pytest.raises(ParseError, match="no `class Platform`"): + python_table(f"class {name}:\n slug: str\n\n\nPLATFORMS = ()\n") + + def it_rejects_a_module_with_no_class_at_all(): with pytest.raises(ParseError, match="no `class Platform`"): python_table("PLATFORMS = ()\n") @@ -61,18 +66,32 @@ def it_rejects_a_platform_class_with_no_annotated_fields(): with pytest.raises(ParseError, match="declares no annotated fields"): python_table("class Platform:\n pass\n\n\nPLATFORMS = ()\n") + @pytest.mark.parametrize("name", ["OTHERS", "TARGETS"]) + def it_rejects_a_table_bound_under_another_name(name): + with pytest.raises(ParseError, match="no module-level"): + python_table(f"class Platform:\n slug: str\n\n\n{name} = ()\n") + def it_rejects_a_module_with_no_table(): with pytest.raises(ParseError, match="no module-level"): python_table("class Platform:\n slug: str\n") + def it_reads_a_table_bound_alongside_another_name(): + source = "class Platform:\n slug: str\n\n\nOTHERS = PLATFORMS = (Platform('a'),)\n" + assert python_table(source)[1] == [{"slug": "a"}] + def it_rejects_a_computed_table(): source = PYTHON.replace("PLATFORMS: tuple[Platform, ...] = (", "PLATFORMS = tuple(") with pytest.raises(ParseError, match="not a tuple or list literal"): python_table(source.replace(")\n", ")\n", 1)) - def it_rejects_a_row_that_is_not_a_platform_call(): + @pytest.mark.parametrize("row", ["1", "Machine('a')", "Target('a')"]) + def it_rejects_a_row_that_is_not_a_platform_call(row): with pytest.raises(ParseError, match="literal `Platform"): - python_table("class Platform:\n slug: str\n\n\nPLATFORMS = (1,)\n") + python_table(f"class Platform:\n slug: str\n\n\nPLATFORMS = ({row},)\n") + + def it_accepts_a_row_that_fills_every_field_positionally(): + source = "class Platform:\n slug: str\n\n\nPLATFORMS = (Platform('a'),)\n" + assert python_table(source)[1] == [{"slug": "a"}] def it_rejects_a_row_with_more_positionals_than_fields(): with pytest.raises(ParseError, match="positional arguments"): @@ -100,6 +119,19 @@ def it_reads_the_array_through_comments_and_trailing_commas(): def it_normalizes_single_quoted_strings(): assert typescript_table("const PLATFORMS = [{ os: ['linux'] }];") == [{"os": ["linux"]}] + def it_unwraps_a_single_quoted_string_without_trimming_its_contents(): + assert typescript_table("const PLATFORMS = [{ os: ['aba'] }];") == [{"os": ["aba"]}] + + def it_unwraps_a_template_literal(): + assert typescript_table("const PLATFORMS = [{ os: [`aba`] }];") == [{"os": ["aba"]}] + + def it_keeps_a_double_quoted_string_verbatim(): + assert typescript_table('const PLATFORMS = [{ os: ["aba"] }];') == [{"os": ["aba"]}] + + def it_finds_the_array_when_a_comment_mentions_a_bracket(): + source = "// see PLATFORMS = [ elsewhere\nconst PLATFORMS = [{ os: ['a'] }];" + assert typescript_table(source) == [{"os": ["a"]}] + def it_keeps_an_escape_sequence_inside_a_string(): assert typescript_table(r'const PLATFORMS = [{ os: "a\"b" }];') == [{"os": 'a"b'}] @@ -107,25 +139,20 @@ def it_rejects_a_source_with_no_table(): with pytest.raises(ParseError, match="no `PLATFORMS"): typescript_table("export const OTHER = [];\n") - def it_rejects_an_unbalanced_array(): - with pytest.raises(ParseError, match="unbalanced"): - typescript_table("const PLATFORMS = [{ os: [] }\n") - - def it_rejects_an_unterminated_block_comment(): - with pytest.raises(ParseError, match="unterminated block comment"): - typescript_table("const PLATFORMS = [ /* open\n") - - def it_rejects_an_unterminated_string(): - with pytest.raises(ParseError, match="unterminated string"): - typescript_table('const PLATFORMS = [{ os: "open\n') - - def it_rejects_an_unterminated_string_ending_in_a_backslash(): - with pytest.raises(ParseError, match="unterminated string"): - typescript_table('const PLATFORMS = [{ os: "open\\') - - def it_rejects_a_line_comment_that_runs_to_the_end_of_the_file(): - with pytest.raises(ParseError, match="unbalanced"): - typescript_table("const PLATFORMS = [ // open") + @pytest.mark.parametrize( + "source", + [ + "const PLATFORMS = [{ os: [] }\n", + "const PLATFORMS = [ /* open\n", + 'const PLATFORMS = [{ os: "open\n', + 'const PLATFORMS = [{ os: "open\\', + "const PLATFORMS = [ // open", + ], + ids=["unclosed-array", "unclosed-block-comment", "unclosed-string", "trailing-backslash", "comment-to-eof"], + ) + def it_rejects_a_table_it_cannot_read_to_the_end(source): + with pytest.raises(ParseError, match="not a plain array"): + typescript_table(source) def it_rejects_an_entry_that_is_not_an_object_literal(): with pytest.raises(ParseError, match="object literal"): From e60d4d1a07620d61c42c864c0e4abe2be64c52e6 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 03:33:19 +0000 Subject: [PATCH 6/6] docs: split search-by-meaning from the index recipe, fix the vec0 template #1010 landed while this branch was open and put the vec0 template on its own page, search-indexes.md, which frames search-by-meaning.md as the no-config guide. Git merges the two cleanly because neither touched the other's file, but the result would have taught the same recipe twice, with different details. So the division of labour follows the page that landed: the index recipe lives once, on search-indexes.md, and search-by-meaning.md keeps the path-table shape and links out. What #1003 asked for survives that split. The two shapes and their trade are stated up front as a table -- setup, freshness, per-query cost, top-k form -- along with the caveat that an index only pays off when the table outlives the query, since a one-shot run against an ephemeral index re-embeds the corpus before answering. The "Top-k is LIMIT k" tip is retired: it existed to explain why MATCH ... AND k = did not apply, which is now a row in that table and one clause in the walk-through. Two fixes to the template on search-indexes.md, both found by running it: vec0 defaults to L2, which is not the metric the rest of dirsql's semantic search uses -- vec_distance_cosine() backs both the plugin one-liner and search-by-meaning. The two agree only for vectors of equal length, so documents that embed to different magnitudes rank differently: over three notes, L2 returned a, c, b where cosine returned a, b, c. The declaration now carries distance_metric=cosine. embed(NULL) is NULL and vec0 rejects a NULL vector, so one row with missing text failed the whole table load rather than its own insert. The insert trigger now carries WHEN new.body IS NOT NULL; the row still lands, only its vector is skipped. Closes #1003 --- docs/howto/search-by-meaning.md | 185 ++++++-------------------------- docs/howto/search-indexes.md | 29 ++++- 2 files changed, 60 insertions(+), 154 deletions(-) diff --git a/docs/howto/search-by-meaning.md b/docs/howto/search-by-meaning.md index 31f413c24..c62c0a4f5 100644 --- a/docs/howto/search-by-meaning.md +++ b/docs/howto/search-by-meaning.md @@ -8,15 +8,6 @@ into vectors and loads [`sqlite-vec`](https://github.com/asg017/sqlite-vec), which does the distance math. No API keys, no services — the model runs locally. -Where those vectors live is the one real decision, and there are two answers: - -- **A `vec0` index**, declared in config beside the table and filled by a - trigger as files are ingested. A query then embeds exactly one string — the - question — and `sqlite-vec` scans the stored vectors in process. This is the - shape to build on. -- **A path-table subquery**, with no config at all: embed every matched file - inline, rank, cut. Ideal for a one-off question over a handful of files. - Suppose short notes live in `notes/*.md`: ``` @@ -25,7 +16,7 @@ notes/branches.md # git feature branches and pull requests notes/tomatoes.md # planting tomato seedlings after the last frost ``` -## Try it in one line +## The one-liner The plugin package is also its own command. Give it a corpus glob and a question, and it prints the closest paths, ranked by distance: @@ -49,147 +40,36 @@ The first-ever run downloads the model (on the order of a hundred megabytes, with progress on stderr); after that it loads from the local cache. Results print one `pathdistance` line per match, closest first. -That command generates the **path-table** shape below — zero setup, and it -embeds every matched file on every run. - ## Which shape -Both rank by cosine distance and return the same rows. They differ in what -each query costs and what you have to declare: +The one-liner embeds every matched file on every run. That is the right trade +for a question you ask once, and the wrong one for a corpus you search +repeatedly — where a stored [`vec0` index](./search-indexes.md#vector-search-vec0) +embeds each file once, at ingest, and a query embeds only the question: -| | Path-table subquery | `vec0` index | +| | This page: path-table | [Vector index](./search-indexes.md#vector-search-vec0) | |---|---|---| | Setup | none | a `[[table]]` with a `ddl` batch | | Freshness | always current — the walk is the read | watcher-maintained; survives restarts under [`--persist`](./persist.md) | | Per query | one `embed()` round trip **per matched file**, plus a walk that reads every file's content | one `embed()` round trip **total**, plus an in-process KNN scan | +| Top-k | `ORDER BY … LIMIT k` | `MATCH … AND k = …` | | Good for | a one-off question, a small corpus, an ad-hoc glob | a corpus you query repeatedly | -The index only pays off when the table outlives the query — under -`--persist`, or inside a long-running [`dirsql server`](../reference/cli.md). -A one-shot `dirsql query` with an ephemeral index rebuilds the table, and -therefore re-embeds the corpus, before it answers; that is strictly more work -than the subquery. - -## Build a vector index - -### 1. Find your model's width - -`vec0` fixes the vector length when the table is declared, so you need the -number your model returns: - -```bash -uvx --with dirsql-plugin-embeddings dirsql "SELECT vec_length(embed('probe')) AS width" -``` - -Whatever that prints is what goes in `float[…]` below — the examples here -write `512`. Get it wrong and the table load fails with an error naming both -figures (`Expected 512 dimensions but received 768.`), so this is cheap to -correct. - -### 2. Declare the table and the index - -A named table's rows come from its [`on-file`](../reference/config.md#table) -hook — dirsql injects nothing — so put a small reader next to the config. -`note.py` prints one row per file, carrying the path to display and the text -to embed: - -```python -#!/usr/bin/env python3 -import json, os, sys - -path, root = sys.argv[1], sys.argv[2] -text = open(path, encoding="utf-8").read() -print(json.dumps([{"path": os.path.relpath(path, root), "text": text}])) -``` - -`ddl` is a whole SQL batch, so the row table, the vector index and the two -triggers that keep them in sync all arrive together in `.dirsql.toml`: - -```toml -[[table]] -name = "notes" -glob = "notes/*.md" -on-file = "python3 note.py {path} {root}" -ddl = ''' -CREATE TABLE notes (path TEXT, text TEXT); - -CREATE VIRTUAL TABLE notes_vec - USING vec0(embedding float[512] distance_metric=cosine); - -CREATE TRIGGER notes_vec_ai AFTER INSERT ON notes -WHEN new.text IS NOT NULL BEGIN - INSERT INTO notes_vec(rowid, embedding) - VALUES (new.rowid, embed(new.text, 'minishlab/potion-retrieval-32M')); -END; -CREATE TRIGGER notes_vec_ad AFTER DELETE ON notes BEGIN - DELETE FROM notes_vec WHERE rowid = old.rowid; -END; -''' -``` - -Three details earn their keep: - -- **`distance_metric=cosine`.** `vec0` defaults to L2, which ranks these - vectors differently. With cosine declared, `notes_vec.distance` is exactly - what `vec_distance_cosine()` computes in the subquery shape. -- **`WHEN new.text IS NOT NULL`.** `embed(NULL)` is `NULL`, and `vec0` rejects - a NULL vector outright — without the guard, one unreadable file fails the - whole table load (`Inserted vector for the "embedding" column is invalid`). - The guard is this shape's counterpart to the subquery's - `WHERE emb IS NOT NULL`. -- **No `[[dirsql.extension]]` entry.** The plugin ships its own config - fragment declaring both `sqlite-vec` and `embed()`, and the launcher - [loads it automatically](../reference/cli.md#plugins). Only the `[[table]]` - is yours to write. - -### 3. Ask a question - -Match against the index, join back to the row table on `rowid`, and let -`sqlite-vec` do the top-k: - -```bash -uvx --with dirsql-plugin-embeddings dirsql query " - SELECT notes.path, notes_vec.distance - FROM notes_vec - JOIN notes ON notes.rowid = notes_vec.rowid - WHERE notes_vec.embedding MATCH embed('how do I cook pasta?', - 'minishlab/potion-retrieval-32M') - AND k = 3 - ORDER BY distance" -c ./.dirsql.toml --persist -``` - -```json -[{"path":"notes/pasta.md","distance":0.315},{"path":"notes/tomatoes.md","distance":0.881},{"path":"notes/branches.md","distance":0.92}] -``` - -`MATCH … AND k = 3` is `sqlite-vec`'s KNN form: `k` is the number of nearest -neighbours to return, and the virtual table supplies a `distance` column for -each. Embed the question with the **same model id the trigger used** — a -vector from another model is a different space, and the ranking it produces is -noise rather than an error. - -### What the triggers guarantee - -dirsql writes file rows with plain `INSERT` and `DELETE` — an update is a -delete and an insert in one transaction, and there is no `UPDATE` path on user -rows — so those two triggers cover every case. The initial load fills the -index; each [watcher](./react-to-changes.md) event maintains it; deleting a -file removes its row *and* its vector. You maintain nothing, and nothing goes -stale. - -### Changing the model +The index only pays off when the table outlives the query — under `--persist`, +or inside a long-running [`dirsql server`](../reference/cli.md). A one-shot +`dirsql query` against an ephemeral index rebuilds the table, and therefore +re-embeds the corpus, before it answers; that is strictly more work than the +subquery below. The full recipe — the width probe, the `ddl` batch, both +triggers, and what a model-id edit costs — is +[Add a search index to a table](./search-indexes.md#vector-search-vec0). -The model id is a literal inside the `ddl` string, and the config hash covers -the whole batch. Editing the id therefore invalidates a -[persistent cache](./persist.md) and forces a full re-scan and re-embed -([batch `ddl`](../reference/config.md#batch-ddl)). That is the whole story for -model changes: no stale vectors, no ownership tracking, no migration step. +The rest of this page is the zero-setup shape. -## Without config: rank a glob inline +## The SQL behind the one-liner -For a one-off question, skip all of the above. The subquery scans a -[path-table](../reference/path-tables.md) and embeds each matched file's -`content` on the spot: +The one-liner generates and runs ordinary `dirsql` SQL, and you can write it +yourself when you want more than ranked paths — a different projection, a +join, a `WHERE` clause, a subset of a JSON file's content: ```bash uvx --with dirsql-plugin-embeddings dirsql " @@ -210,11 +90,12 @@ distance ranking is doing the work. Reading the query inside-out: -1. The subquery scans the path-table `'./notes/*.md'` and embeds each file's - [`content`](../reference/path-tables.md#columns) — only the files the glob - matches are ever read or embedded. In hand-written SQL the `./` prefix is - [required](../reference/path-tables.md#writing-the-path); only the - one-liner normalizes a bare glob for you. +1. The subquery scans the [path-table](../reference/path-tables.md) + `'./notes/*.md'` and embeds each file's + [`content`](../reference/path-tables.md#columns) — only the files the + glob matches are ever read or embedded. In hand-written SQL the `./` + prefix is [required](../reference/path-tables.md#writing-the-path); only + the one-liner normalizes a bare glob for you. 2. `embed('how do I cook pasta?')` embeds the question once (the function is deterministic, so SQLite reuses the value across rows). 3. `WHERE emb IS NOT NULL` drops the files that could not be embedded. A @@ -224,8 +105,8 @@ Reading the query inside-out: so without this line the unrankable files take the top-k slots. 4. `vec_distance_cosine(...)` computes cosine distance between the two vectors; `ORDER BY distance LIMIT 3` keeps the three nearest. There is no - `vec0` table here, so `MATCH … AND k =` does not apply — for a plain - expression, `ORDER BY … LIMIT k` *is* `sqlite-vec`'s documented top-k. + `vec0` table here, so `sqlite-vec`'s `MATCH … AND k = …` does not apply — + for a plain expression, `ORDER BY … LIMIT k` *is* its documented top-k. Structured files compose with SQL's JSON operators — embed one field instead of the whole file: @@ -240,18 +121,18 @@ LIMIT 10 ``` The same projection works as an `on-file` hook feeding the indexed shape: -parse the field in the hook, store it as the table's `text` column, and the -trigger embeds it once instead of on every query. +parse the field in the hook, store it as a column, and the trigger embeds it +once instead of on every query. ## Repeat runs are cheap Computed vectors are cached on disk, keyed on content and model ([vector cache](../plugins.md#vector-cache)) — re-running a search over unchanged files skips the model entirely and re-embeds only what changed. That -cuts the *inference* out of the subquery shape, but not the walk or the -per-file round trip; only the `vec0` index removes those. And the plugin costs -nothing when idle: a query that never calls `embed()` spawns no worker and -loads no model ([zero cost when unused](../plugins.md#zero-cost-when-unused)). +takes the *inference* out of the shape above, but not the walk or the per-file +round trip; only a stored index removes those. And the plugin costs nothing +when idle: a query that never calls `embed()` spawns no worker and loads no +model ([zero cost when unused](../plugins.md#zero-cost-when-unused)). ## How `embed()` gets into SQL diff --git a/docs/howto/search-indexes.md b/docs/howto/search-indexes.md index f3c137bac..eb165466e 100644 --- a/docs/howto/search-indexes.md +++ b/docs/howto/search-indexes.md @@ -155,8 +155,10 @@ ddl = ''' CREATE TABLE notes (slug TEXT, title TEXT, body TEXT); -- Width must equal what the probe printed for the model id named below. -CREATE VIRTUAL TABLE notes_vec USING vec0(embedding float[512]); -CREATE TRIGGER notes_vi AFTER INSERT ON notes BEGIN +CREATE VIRTUAL TABLE notes_vec + USING vec0(embedding float[512] distance_metric=cosine); +CREATE TRIGGER notes_vi AFTER INSERT ON notes +WHEN new.body IS NOT NULL BEGIN INSERT INTO notes_vec(rowid, embedding) VALUES (new.rowid, embed(new.body, 'minishlab/potion-retrieval-32M')); END; @@ -169,6 +171,29 @@ END; The delete side is an ordinary `DELETE`, not FTS5's `'delete'` command row — `vec0` is a normal-looking table that way. +Two clauses in there are load-bearing: + +- **`distance_metric=cosine`.** `vec0` defaults to L2, which is not the metric + the rest of dirsql's semantic search uses — `vec_distance_cosine()` backs + both the plugin's one-liner and + [Search documents by meaning](./search-by-meaning.md). The two agree only for + vectors of equal length, so as soon as documents embed to vectors of + different magnitude they rank differently: against three notes, L2 returned + `a, c, b` where cosine returned `a, b, c`. Declaring the metric makes + `distance` the number `vec_distance_cosine()` would compute. +- **`WHEN new.body IS NOT NULL`.** `embed(NULL)` is `NULL`, and `vec0` rejects + a NULL vector outright — so without the guard a single row whose text is + missing fails the **whole** table load, not just its own insert: + + ``` + dirsql query: failed to load config: SQLite error: Inserted vector for the + "embedding" column is invalid: Input must have type BLOB (compact format) or + TEXT (JSON), found NULL + ``` + + With it, that row still lands in `notes`; only its vector is skipped. FTS5 + needs no such guard — it indexes a NULL happily. + Query it with `sqlite-vec`'s KNN form, joined back on `rowid`: ```bash