Skip to content

feat(lint): accept model file paths in the lint command - #6053

Open
tripleaceme wants to merge 8 commits into
SQLMesh:mainfrom
tripleaceme:lint-model-paths
Open

tripleaceme wants to merge 8 commits into
SQLMesh:mainfrom
tripleaceme:lint-model-paths

Conversation

@tripleaceme

Copy link
Copy Markdown
Contributor

Description

Closes #6021.

sqlmesh lint could only select models by name with --model, so path-based tooling such as pre-commit — which passes the names of the changed files — could not drive it without a wrapper that mapped paths back to model names.

This adds a positional PATHS argument to sqlmesh lint, mirroring the shape of sqlmesh format:

sqlmesh lint --local --use-project-index models/a.sql models/b.py

Behaviour:

  • Each path is resolved to the model(s) defined in that file. SQL and Python model files both work.
  • Paths can be combined with --model. A model selected both ways is linted once.
  • No paths and no --model still lints every model, unchanged.
  • A path that defines no model (a typo, a non-model file) is rejected with a clear error instead of silently falling back to linting the whole project:
    Error: No models were found at the following path(s): models/typo.sql
    
  • --local and --use-project-index keep working when the selection comes from paths.

Implementation note

--use-project-index needs the paths before models are parsed, so that only the selected files and their upstream dependencies are loaded. The persistent index can only be read from inside Loader.load() (_model_index_id() depends on the macro/signal/audit mtimes that the load tracks), so rather than resolving paths up front, the selected paths are plumbed through Context.load → Loader.load → _load_models alongside the existing model_fqns. SqlMeshLoader._selected_model_paths then seeds its selection from both the requested FQNs and the requested paths, and the existing upstream-closure and stale-index fallbacks apply unchanged.

The final model set is always resolved from the loaded models via Model._path, which is what produces the "no models at this path" error and keeps the multi-project case (one Loader per --paths) correct.

Test Plan

New tests:

  • tests/core/test_context.py::test_lint_models_by_path — a path selects only the models in that file; unrelated violations are not reported; Python model files work; relative paths resolve against the cwd; paths and --model combine and de-duplicate; an unknown path raises; error-level violations still raise.
  • tests/core/test_context.py::test_lint_models_by_path_with_project_index — asserts _load_sql_models is called with selected_paths == {a.sql, b.sql} for a path-selected model with one upstream, and that an unknown path is rejected before any models are loaded.
  • tests/core/test_context.py::test_lint_models_by_path_without_index_falls_back_to_full_load — a missing index falls back to a full load.
  • tests/cli/test_cli.py::test_lint_paths, test_lint_relative_path, test_lint_unknown_path — CLI coverage for single/multiple paths, paths plus --model, relative paths, --local/--use-project-index, and both unknown-path cases.
  • tests/cli/test_cli.py::test_lint_model_scopes_validation_with_multiple_projects — extended to cover selecting the same model by path in a multi-project context.

Also verified by hand on a fresh sqlmesh init duckdb project with rules: "ALL" and an added SELECT * model: linting a single file reports only that model, linting with no arguments still reports all three, an unknown path and an audit file both error out, and --use-project-index produces the same result on the run that builds the index and on the run that reads it.

make fast-test passes (2619 passed). The 5 pre-existing failures in tests/utils/test_git_client.py and test_expand_git_selection_integration reproduce identically on an unmodified checkout in my environment and are unrelated to this change.

Checklist

  • I have run make style and fixed any issues
  • I have added tests for my changes (if applicable)
  • All existing tests pass (make fast-test)
  • My commits are signed off (git commit -s) per the DCO

`sqlmesh lint` could only select models by name with `--model`, so
path-based tooling such as pre-commit — which passes the names of the
changed files — could not drive it without a wrapper that mapped paths
back to model names.

Add a positional `PATHS` argument to `sqlmesh lint`, mirroring the shape
of `sqlmesh format`. Each path is resolved to the model(s) defined in
that file, and paths can be combined with `--model`; a model selected
both ways is linted once. Linting with no selection still lints every
model.

Model file paths are plumbed into the load path alongside `model_fqns`,
so `--use-project-index` scopes a path-based selection the same way it
scopes `--model`: only the selected models and their transitive upstream
dependencies are loaded, resolved and validated. A path that defines no
models is rejected with a clear error rather than silently falling back
to linting the whole project.

Closes SQLMesh#6021

Signed-off-by: Adegbite Ayoade <tripleaceme@gmail.com>
@mday-io mday-io self-assigned this Sep 11, 2026
@mday-io
mday-io self-requested a review September 11, 2026 12:41
@mday-io

mday-io commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

Two things worth a look before merge:

1. Perf: the indexed path lookup does a filesystem call per model, not per selection

In SqlMeshLoader._selected_model_paths, matching by --model name is a pure in-memory dict lookup (fast, no I/O, which is the whole point of --use-project-index). But matching by path does this:

selected.update(
    fqn for fqn, path in model_to_path.items() if path.resolve() in model_paths
)

path.resolve() runs once per model in the whole project's index, not once per path the user actually asked for. On a big project (thousands of models - exactly who uses --use-project-index), that's thousands of stat/symlink-resolution syscalls just to find the one or two files someone selected. It'll still be way cheaper than a full parse, so nobody will notice on small-to-medium projects, but it does undercut the "index avoids touching unrelated files" guarantee for large ones.

Suggest normalizing the index side the same cheap way the paths are already built (config_path / relative_path, lexical join, no symlink resolution), rather than calling .resolve() per entry. Only the handful of user-supplied paths need the expensive resolve, not every entry in the index.

2. Docs: the added description text doesn't match sqlmesh lint --help

docs/reference/cli.md mirrors real --help output for every command. This PR adds "Models can be selected by name with --model, by model file path, or by both." to the lint description, but that sentence isn't actually in the CLI's docstring, so running sqlmesh lint --help doesn't print it. Either add that sentence to the lint command's docstring in cli/main.py so it's real, or drop it from the docs page so the page stays an accurate mirror.

Matching a selected path walked every entry in the index and called
path.resolve() on each one, so selecting a single file cost one filesystem
call per model in the project — the opposite of what --use-project-index is
for. Resolve the project root once and key the index by that instead, turning
the match into a dict lookup per path the user actually asked for.

A model file that is itself a symlink is no longer covered by joining onto the
resolved root, so the previous resolve-based scan is kept as a fallback for
paths left unmatched.

Also move the path-selection sentence into the lint docstring so that it is
real --help output, and mirror it in the CLI reference rather than documenting
text the command never prints.

Signed-off-by: Adegbite Ayoade <tripleaceme@gmail.com>
@tripleaceme

tripleaceme commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor Author

Thanks, both good catches — fixed in 27057ae.

1. Path lookup

You're right, and it was worse than a normalisation mismatch: the index side was already being built lexically (self.config_path / relative_path), so the .resolve() existed purely to make the two sides comparable at match time, and paid a syscall per indexed model to do it.

Now the project root is resolved once and the index is keyed by resolved_config_path / relative_path, which turns the match into a dict lookup per path the user actually passed rather than a scan over every model.

One thing worth flagging, since it wasn't only a perf change: the old .resolve() also followed symlinks on individual model files, not just the root. Joining onto a resolved root doesn't, so a model file that is itself a symlink would have silently stopped matching. I kept the old resolve-based scan as a fallback for paths still unmatched after the lookup — the common case never reaches it, and the symlink case keeps working. Happy to drop the fallback if you'd rather not support that.

Added two tests: one asserting no .sql path is resolved while matching off the index (it fails on the previous implementation), and one covering the symlinked model file.

2. Docs

Agreed the page should stay an accurate mirror. It's now a second paragraph on the lint docstring, so sqlmesh lint --help actually prints it, and I mirrored the real output on the page. The summary line stays a single terse sentence, matching format and the rest of the commands.

I left the rest of that block alone, though while I was in there I noticed the page's --local text is longer than what the command prints and --model TEXT is really --models, --model TEXT. Both predate this PR, so I've not touched them — happy to fix separately if useful.

Signed-off-by: Adegbite Ayoade <tripleaceme@gmail.com>
@tripleaceme

Copy link
Copy Markdown
Contributor Author

@mday-io — gentle nudge on this one, since it's been sitting since the 11th.

Both points from your review are addressed in 27057ae:

  1. The index is now keyed on resolved_config_path / relative_path, so the project root is resolved once and matching is a dict lookup per path the user actually passed, rather than a .resolve() per indexed model.
  2. The description sentence now lives on the lint command's docstring, so sqlmesh lint --help really prints it, and the docs page mirrors the actual output.

Two questions from my reply are still open, and they're the only things that would change the diff further:

  • I kept the old resolve-based scan as a fallback for paths still unmatched after the index lookup, so a model file that is itself a symlink keeps matching. The common case never reaches it. Happy to drop it if you'd rather not carry that case.
  • The page's --local text is longer than what the command prints, and --model TEXT is really --models, --model TEXT. Both predate this PR so I left them alone — say the word if you'd like them fixed here or separately.

Branch is merged up with main as of the 13th. Still showing zero checks while the workflow run waits on approval.

@mday-io

mday-io commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

Hey there. Three days - especially over the weekend - likely won't be enough time for pull requests or issues to work their way through the pipeline. This is not a full-time maintained project. If you have a truly urgent problem that is disrupting your organization then feel free to call it out, otherwise it could take one to many weeks to get a PR reviewed (especially more than one time).

Signed-off-by: Adegbite Ayoade <tripleaceme@gmail.com>
@tripleaceme

Copy link
Copy Markdown
Contributor Author

Thanks for the feedback @mday-io

Signed-off-by: Adegbite Ayoade <tripleaceme@gmail.com>
@mday-io

mday-io commented Sep 25, 2026 •

Copy link
Copy Markdown
Collaborator

Thanks for this, and for the pre-commit example in the docs. That makes the use case clear.

A question to check we're solving the same problem: is the main goal pre-commit, i.e. tools that pass the changed file names to sqlmesh lint? If so, positional paths are the right interface, and I'm happy to take this approach once the items below are addressed.

For the other use case, linting only the models changed in CI, we'd rather reuse the existing model selector (git:main, tag:, + for upstream/downstream, etc.) that plan, run and dag already support. lint doesn't accept --select-model today, so we're opening a separate issue for that: . It doesn't need to be part of this PR. I've suggested a small docs tweak so the two options are positioned clearly.

Blocking: the flaky test assertion (inline comment on tests/cli/test_cli.py). The other inline comments are small.

Comment thread docs/guides/linter.md Outdated

Use `sqlmesh lint --help` for more information.

Models can be selected by name with `--model`, by model file path, or by both at once. Selecting by

@mday-io mday-io Sep 25, 2026 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we frame path selection as the option for pre-commit and editor hooks, and point people who want to lint changed models in CI elsewhere? Something like:

Selecting by path is intended for tools that pass file names, such as pre-commit hooks.

"path-based tooling" is broad enough that people may reach for sqlmesh lint $(git diff ...) in CI, which we'd rather steer towards git:main once lands.

Comment thread sqlmesh/core/context.py
use_project_index: Whether to use the persistent project index. If omitted, the
value of ``linter.use_project_index`` is used. Indexed linting of selected
models reloads an already-loaded context so the requested scope is applied.
paths: Model file paths to lint, each resolved to the model(s) defined in it. Can be

@mday-io mday-io Sep 25, 2026 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: worth saying relative paths are resolved from the current working directory, not the project path. From the Python API, Context(paths="proj").lint_models(paths=["models/x.sql"]) only works when run from inside proj/.

Comment thread sqlmesh/core/context.py
not model_fqns <= self._models.keys()
if (model_fqns or model_paths) and (
not (model_fqns or set()) <= self._models.keys()
or (model_paths is not None and not model_paths <= self._loaded_model_paths())

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not blocking, just noting it: when a path exists but defines no models (an audit, a macro, an ignored file), this condition forces a full project load before _models_for_paths raises "No models were found". The docs' files: ^models/... filter makes that unlikely in practice. If it's cheap, checking unmatched paths against the index before reloading would let it error without the full load. Happy to leave it as a follow-up.

@mday-io

mday-io commented Sep 25, 2026 •

Copy link
Copy Markdown
Collaborator

The three "Merge branch 'main' into lint-model-paths" commits (31a0099, 0f782af, e2e34b1) don't have a Signed-off-by line, and the author name on them ("Adegbite Ayoade Abel") differs from the sign-off on your other commits. Could you sign those off so the DCO check passes?

Comment thread tests/core/test_context.py Outdated
# Case: Python model files are selectable too.
assert create_context().lint_models(paths=[python_path]) == []

# Case: relative paths are resolved against the current working directory.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This case changes directory to tmp_path, which is also the project path, so it can't tell "relative to the current directory" apart from "relative to the project". I checked: if lint_models resolves relative paths against self.path instead, every test still passes. test_lint_relative_path in test_cli.py has the same blind spot.

That distinction is the pre-commit case: pre-commit runs from the repo root, which isn't the project directory when the SQLMesh project lives in a subfolder. Could you put the project in a subdirectory (e.g. tmp_path / "proj"), chdir to tmp_path, and pass "proj/models/c.py"?

Comment thread tests/cli/test_cli.py
assert result.exit_code == 1


def test_lint_paths(runner, tmp_path):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test_lint_relative_path fails when the suite runs in parallel (pytest -n auto, as CI does). The console wraps at 80 columns, and under xdist the temp path gains a popen-gwN/ segment, so the file name gets split across lines and "seed_model.sql" in result.output fails:

Linter errors for
/tmp/pytest-of-root/pytest-6/popen-gw3/test_lint_relative_path0/models/seed_mode
l.sql:

It reproduces every time with pytest -n 4 tests/cli/test_cli.py -k lint. test_lint_paths has the same assertion at line 1495 and will break the same way on a runner with a longer temp path.

Rather than patching the assertions, could you fold the three CLI tests into one? The Python API tests already cover the behaviours (single and multiple files, combining with --model, deduplication, unknown paths), so the CLI only needs to prove the argument is wired through. One test that mirrors how pre-commit calls it (several relative paths plus --local --use-project-index, checking the exit code and the count of "Linter errors for" lines) would do, and avoids matching file names in wrapped output.

Comment thread tests/cli/test_cli.py Outdated
assert "No models were found at the following path(s)" in result.output
assert "Linter errors for" not in result.output

# A file that exists but defines no models is an error too.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This "file exists but defines no models" case is the only test for the error raised in _models_for_paths. The context tests only use a missing file, which is rejected earlier by the is_file() check. When you consolidate the CLI tests, could you move this case into test_lint_models_by_path (e.g. pass an audit or macro file) so the Python API test owns it?

Comment thread tests/core/test_context.py Outdated
assert {path.name for path in selected_paths} == {"a.sql", "b.sql"}


def test_lint_models_by_path_without_index_falls_back_to_full_load(tmp_path: pathlib.Path) -> None:

@mday-io mday-io Sep 25, 2026 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first call in test_lint_models_by_path_with_project_index (line 3492) already lints by path with no index and checks it succeeds. The only extra thing here is that the private selected_paths kwarg is None, so I think this one can go.

Comment thread tests/core/test_context.py Outdated
assert load_mock.call_count == 0


def test_lint_models_by_path_with_project_index_resolves_only_given_paths(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This patches pathlib.Path.resolve globally, calls the private _selected_model_paths directly, and counts .resolve() calls. A behaviour-preserving refactor (os.path.realpath, caching, a signature change) would break it, while a real slowdown that stats paths some other way would pass. I'd drop it and rely on the code comment explaining why the root is resolved once.

Comment thread sqlmesh/core/context.py
target_paths = [Path(path) for path in paths] if paths is not None else []

# Fail fast on a mistyped path instead of loading and linting the whole project.
missing_paths = [str(path) for path in target_paths if not path.is_file()]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: sqlmesh lint models/ currently says "No models were found at the following path(s): models", which reads as if the directory has no models. A separate message for directories ("... is a directory; pass model files") would be clearer. Expanding directories would also be fine.

Comment thread tests/core/test_context.py Outdated
ctx.get_model(model_name, raise_if_missing=True).fqn for model_name in ("a", "b")
}

# Case: an unknown path is rejected off the index, before any models are loaded.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: the path is rejected by the is_file() check before the index is read. Maybe "rejected before any models are loaded"?

The two assertions on the linted file name compared against raw console
output. The console wraps at 80 columns, so a long enough temporary path
splits the name across lines and the assertion fails. Under xdist the
path gains a popen-gwN segment, which is what makes it reproducible
there. Reproduced directly: a path of the right length renders the line
as "models/seed_model.sq\nl:". Both assertions now compare against the
output with the wrapping removed.

A directory now says so instead of reporting that no models were found
in it, which read as though the directory were empty.

The docstring records that relative paths resolve against the current
working directory rather than the project path. That is what path-based
tools pass, but it means the Python API only accepts relative paths when
called from inside the project.

The linter guide now frames path selection as the option for tools that
hand over file names, and points CI use towards a model selector instead
of paths from git diff, so that the models affected by a change are
linted rather than only the files that were edited.

Signed-off-by: Adegbite Ayoade <tripleaceme@gmail.com>
The relative-path cases ran from the project directory, so they could
not tell "resolved against the current working directory" apart from
"resolved against the project path". Both now run from a directory that
is not the project and assert the path is not found there, then repeat
from the project. Checked against the alternative: resolving relative
paths against self.path now fails both, where before it passed
everything.

Moves the "file exists but defines no models" case to the Python API
test, which owns that error, rather than leaving the CLI as its only
coverage.

Drops two tests. One asserted that no .sql path is resolved while
matching off the index, by patching pathlib.Path.resolve globally and
counting calls; it would break on a behaviour-preserving refactor and
pass on a slowdown that stats paths another way. The other duplicated a
case already covered earlier in the indexed test, differing only in a
private kwarg being None.

Also corrects a comment: an unknown path is rejected by the is_file()
check before the index is consulted, not off the index.

Signed-off-by: Adegbite Ayoade <tripleaceme@gmail.com>
@tripleaceme

Copy link
Copy Markdown
Contributor Author

Thanks @mday-io — all of it addressed across 8cd5ca5 and ecf41ac, and the three merge commits are signed off now.

On the goal: yes, pre-commit is the target. Tools that hand over changed file names, plus editor hooks. I agree the CI case belongs with the model selector rather than paths from git diff, and the docs now say so — happy to point at the lint --select-model issue once it exists (the link came through empty in your comment).

The flaky assertion. Reproduced it directly rather than taking it on trust: at the right path length the console renders the line as

Linter errors for
/private/var/.../pytest-9/popen-gw3/xxxx.../models/seed_model.sq
l:

so "seed_model.sql" in result.output is False. Both assertions now compare against the output with the wrapping removed, test_lint_paths included.

The relative-path blind spot — this was the useful one. You were right, and it was worse than a gap in coverage: the case changed directory to the project path, so it asserted nothing about which of the two resolutions was in use. Both the API and CLI cases now run from a directory that is not the project, assert the path is not found there, then repeat from the project. I checked against your exact alternative — resolving relative paths against self.path now fails both tests, where before every test passed.

The two tests you flagged are gone. The resolve-counting one was measuring the implementation rather than the behaviour, as you say; the code comment carries that reasoning instead. The other duplicated the earlier case in the indexed test.

Moved the "file exists but defines no models" case to the Python API test, so the test that owns the error in _models_for_paths covers it.

Directories now say so: Expected model files but got directory: models. Pass the model files themselves.

The docstring records that relative paths resolve against the current working directory rather than the project path, and what that means from the Python API.

On context.py:722 — leaving it as a follow-up as you suggested. Worth noting the cost is narrower than it looks: the is_file() check already rejects missing paths before any load, so the full load only happens for a path that exists but defines no models, which the docs' files: filter makes unlikely. Happy to pick it up separately if you'd like it closed off.

DCO: the three merge commits were authored as "Adegbite Ayoade Abel" with no sign-off. Rewritten with the sign-off and the name matching the rest, and I verified the tree is byte-identical to before the rewrite.

pytest -n 4 tests/cli/test_cli.py tests/core/test_context.py
197 passed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sqlmesh lint should accept model file paths

2 participants