From 6a9491fe52236b1c0a368326cf77b74bb7f0bef6 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 10 Sep 2026 01:01:11 +0000 Subject: [PATCH 1/3] docs(agents): remove duplicated guidance and align checks with CI The agent instruction files had accumulated three kinds of redundancy: checks subsumed by other checks, guidance restated across AGENTS.md, STYLE.md and the skills, and prescribed commands that did not match what CI runs. Correct the check commands so a local pass predicts CI: - Add `-D warnings` to clippy. Without it clippy exits successfully on exactly the warnings CI rejects. - Point fmt at the nightly pinned as NIGHTLY_TOOLCHAIN rather than a floating `+nightly`, which can format differently from the toolchain CI checks against. - Scope the Rust checks with `-p ` instead of running them workspace-wide over 68 crates, which is CI's job. - Use `uvx` for ruff in the Python bindings, matching CI's invocation. Drop subsumed checks: - `cargo build`, already performed by `cargo nextest run` and `cargo clippy --all-targets`, and repeated under a third profile. - The clang-format `--dry-run --Werror` pass over files just formatted in place. - `python -m py_compile`, whose errors are already reported by ruff, basedpyright and pytest collection. - The per-crate `cargo fmt --check` in vortex-python/AGENTS.md, which now defers to the root file and so picks up clippy as well. Remove duplicated prose: the CI Investigation section (contained in the ci-failure-analysis skill it routes to), the Performance section and the hidden-cost accessor bullet (both paraphrases of the STYLE.md table, now a single pointer), the yamllint and Rust-checks-for-docs rules that nested AGENTS.md files already own, and five of the eight Common Mistakes bullets that restated rules stated earlier in the same file. Delete the query skill. Its crate map had already drifted from the Repository Layout section it duplicated, omitting vortex-cloud and vortex-io. Document three CI checks no agent file mentioned, all of which fail on new files rather than edited ones: REUSE SPDX headers, typos, and the assertion that the tree is clean after a build. Signed-off-by: Robert Kruszewski Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Vm8WdEPBCw9hfWivTiuDp1 Signed-off-by: Robert Kruszewski --- .agents/skills/query/SKILL.md | 36 ---------- AGENTS.md | 121 +++++++++++++++------------------- docs/AGENTS.md | 1 - vortex-python/AGENTS.md | 16 ++--- 4 files changed, 61 insertions(+), 113 deletions(-) delete mode 100644 .agents/skills/query/SKILL.md diff --git a/.agents/skills/query/SKILL.md b/.agents/skills/query/SKILL.md deleted file mode 100644 index 2a07f3ef312..00000000000 --- a/.agents/skills/query/SKILL.md +++ /dev/null @@ -1,36 +0,0 @@ ---- -name: query -description: Answer questions about the Vortex codebase or pull requests. Use when asked a question via "/query" or when the user wants to understand code, architecture, behavior, or implementation details. ---- - -# Vortex Query Skill - -Answer questions about the Vortex project, its pull requests, and its implementation. - -## Key Context - -- Vortex is a Rust workspace for columnar arrays, compression encodings, file IO, and scan - integrations. -- `vortex-array` defines the core array traits, dtype system, canonical arrays, and base - encodings. -- `vortex-buffer` owns aligned zero-copy buffers. -- `vortex-file` and `vortex-layout` implement file and layout reading. -- `encodings/*` contains specialized compressed encodings. -- Python, Java, DuckDB, and DataFusion integrations live in their own workspace areas. - -## Workflow - -1. Read `AGENTS.md` and any closer scoped `AGENTS.md` before relying on conventions. -2. Use `rg` and targeted file reads to identify the relevant crate, module, and tests. -3. If the question is about a PR, inspect the diff and comments before answering. -4. If the question is about behavior, trace the implementation through public entry points, - encoding-specific implementations, and tests. -5. Answer with concrete file paths and line numbers when they help. - -## Answering Guidelines - -- Separate confirmed facts from inference. -- Prefer precise code references over broad descriptions. -- Mention important uncertainty and describe what would verify it. -- Do not invent architecture. If the repository does not answer the question, say what you - checked and what is still missing. diff --git a/AGENTS.md b/AGENTS.md index a0e4c6558cd..7c9af7e7206 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -4,10 +4,9 @@ Guidance for Claude, Codex and other coding agents working in the Vortex reposit ## Task Routing -- When asked a question about the PR or codebase, especially via `/query`, use the - `.agents/skills/query` skill. - When asked to investigate a CI failure, especially via `/ci-failure-analysis`, use the - `.agents/skills/ci-failure-analysis` skill. + `.agents/skills/ci-failure-analysis` skill. It is the source of truth for fetching logs, + classifying failures, and attributing causation. ## Overview @@ -40,97 +39,90 @@ Before changing files in a subtree, read the closest nested `AGENTS.md`. In part - `docs/AGENTS.md` covers Sphinx documentation. - `vortex-python/AGENTS.md` covers Python and PyO3 binding work. -## Build +## Verification -Prefer narrow crate builds while iterating: +Run the narrowest check that covers the files you changed. CI already runs the workspace-wide +version of everything below, so reproducing that breadth locally costs far more than the round +trip it saves. -```bash -cargo build -p -``` +Do not run Rust checks at all for changes that only touch Markdown, RST, Sphinx configuration, +agent configuration, comments outside Rust code, symlinks, or other metadata with no Rust/API +behavior impact. Validate those by inspection or with a targeted doc/config command, and verify +symlink or path changes with `ls`, `find`, and `git status`. -Use workspace-wide builds only when the change spans crate boundaries or before handing off a -broad refactor: +### Rust -```bash -cargo build --workspace -``` - -## Testing - -Run tests for the crate or binding you touched before broader checks: +For Rust code, public API, feature flag, or generated-file changes, run these before stopping: ```bash cargo nextest run -p +cargo clippy -p --all-targets --all-features -- -D warnings +cargo +nightly- fmt -p ``` -If cargo-nextest is not available, you can install it with: +Two details make these match CI rather than merely resemble it: -```bash -cargo install --locked cargo-nextest -``` +- `-D warnings` is required. Without it clippy exits successfully on exactly the warnings CI + rejects, so a local pass predicts nothing. +- Use the nightly pinned as `NIGHTLY_TOOLCHAIN` in `.github/workflows/ci.yml`. A floating + `+nightly` can format differently from the toolchain CI checks against, so the reformatted tree + still fails `fmt --check`. + +There is no separate build step. `cargo nextest run` and `cargo clippy --all-targets` each compile +the crate; a preceding `cargo build` only repeats that work under a third profile. -For Rust doc comments or crate documentation, run doctests for the affected crate: +For Rust doc comments or crate documentation, also run doctests for the affected crate: ```bash cargo test --doc -p ``` -## Linting, Formatting, and Generated Files +If cargo-nextest is not available, install it with `cargo install --locked cargo-nextest`. -Run verification that matches the files changed. Do not run expensive Rust checks for changes that -only touch Markdown, agent configuration, comments outside Rust code, symlinks, or other metadata -with no Rust/API behavior impact. For docs/config-only changes, validate formatting by inspection -or with a targeted doc/config command, and verify symlink or path changes with `ls`, `find`, and -`git status`. +### C++ and CUDA -For Rust code, public API, feature flag, or generated-file changes, run these before stopping: - -```bash -cargo +nightly fmt --all -cargo clippy --all-targets --all-features -``` - -For changed C++ and CUDA source or header files covered by CI (`.cpp`, `.hpp`, `.cu`, `.cuh`, and -`.h` files under `lang/cpp`, `vortex-cuda`, `vortex-duckdb`, and `vortex-ffi`), format with the -repository's `.clang-format` configuration and verify the result: +Format changed `.cpp`, `.hpp`, `.cu`, `.cuh`, and `.h` files under `lang/cpp`, `vortex-cuda`, +`vortex-duckdb`, and `vortex-ffi` with the repository's `.clang-format` configuration: ```bash clang-format --style=file -i -clang-format --dry-run --Werror --style=file ``` Pass only the files you changed; CI excludes vendored or generated CUDA and Arrow headers from its -repository-wide check. +repository-wide check. clang-format is idempotent, so a `--dry-run --Werror` pass over the files +you just formatted cannot fail and is not worth running. -Notes: +### New and generated files -- For `.github/` changes, follow `.github/AGENTS.md` and run - `yamllint --strict -c .yamllint.yaml` on changed workflow files. -- If cargo fails with exactly `sccache: error: Operation not permitted`, rerun that command - with `RUSTC_WRAPPER=` so rustc runs directly. Only do this for that exact error. +These CI checks are the ones most often missed when adding files rather than editing them: + +- Every source file needs SPDX headers, in the comment syntax of its language: + + ```text + SPDX-License-Identifier: Apache-2.0 + SPDX-FileCopyrightText: Copyright the Vortex contributors + ``` -## CI Investigation + `REUSE.toml` records the exceptions, including the CC-BY-4.0 licensing of `docs/**`. -- When iterating on CI failures, fetch only failed job logs first: - `gh run view --job --log-failed`. -- Run narrow local repro commands for the affected crate, test, docs target, or binding before - running workspace-wide checks. -- If a `gh` command fails with `error connecting to api.github.com` in the sandbox, immediately - rerun it with escalated network permissions instead of retrying in the sandbox. -- Verify causation from logs, diffs, and local repros before attributing a failure to a PR. +- Spelling is checked by `typos` against `_typos.toml`. +- CI asserts `git status --porcelain` is empty after a build. Regenerate generated files with the + repository's tooling rather than editing them by hand, and commit the result. + +### Notes + +- For `.github/` changes, follow `.github/AGENTS.md`, which covers both the yamllint invocation and + the nightly toolchain pin. +- If cargo fails with exactly `sccache: error: Operation not permitted`, rerun that command + with `RUSTC_WRAPPER=` so rustc runs directly. Only do this for that exact error. ## Rust Code Style -- Follow `STYLE.md` for Rust formatting, documentation, API, error-handling, import, and safety - conventions. +- Follow `STYLE.md` for Rust formatting, documentation, API, error-handling, import, safety, and + performance conventions. Its hidden-cost accessor table is the reference for changes to + per-element loops; back such changes with the benchmarks it names. - Only write comments that explain non-obvious logic or important context. Do not comment self-explanatory code. -- Keep public APIs small and consistent with neighboring crates. - -## Performance - -Avoid hidden-cost per-element accessors in hot loops, follow the performance guidance in -`STYLE.md`, and benchmark changes to hot paths. ## Tests @@ -151,17 +143,10 @@ Avoid hidden-cost per-element accessors in hot loops, follow the performance gui Check new and modified lines against this list before finishing: -- Running broad CI-style commands before trying a narrow local repro. -- Using `unwrap`, `expect`, or panic-oriented assertions in tests where `VortexResult<()>` and - `?` would be clearer. -- Comparing arrays element by element instead of using `assert_arrays_eq!`. - Adding imports inside functions when module-level imports would work. -- Introducing `unsafe` without proving that safe Rust cannot express the same operation. - Updating expected test output to match buggy behavior without independently verifying the intended semantics. - Silently reducing the scope of an approved plan when implementation is harder than expected. -- Calling a hidden-cost per-element accessor (`Validity::is_valid`, `scalar_at`, `BitBuffer:: - value` accumulation) inside a hot loop instead of materializing once. ## Summaries diff --git a/docs/AGENTS.md b/docs/AGENTS.md index f130929d2c2..b45df17793c 100644 --- a/docs/AGENTS.md +++ b/docs/AGENTS.md @@ -10,4 +10,3 @@ Applies to files under `docs/`. contributor guide. Keep that contributor guide as the source of truth for Sphinx commands shared by humans and agents, and do not invoke `sphinx-build` directly. -- Do not run Rust checks for changes confined to RST, Markdown, or Sphinx configuration. diff --git a/vortex-python/AGENTS.md b/vortex-python/AGENTS.md index 536be8352a2..2ae2b9a8e90 100644 --- a/vortex-python/AGENTS.md +++ b/vortex-python/AGENTS.md @@ -15,7 +15,6 @@ the full Python check. Keep the contributor guide as the source of truth for sha Run the narrow checks that match the files changed before broader Python suites: ```bash -python -m py_compile uv run --all-packages pytest ``` @@ -25,15 +24,16 @@ If Python docstrings, `docs/api/python/`, or Sphinx configuration change, also f ## Linting and Formatting ```bash -uv run ty check vortex-python -uv run ruff format --check -uv run ruff check +uvx ruff format --check +uvx ruff check +uv run basedpyright vortex-python ``` -If PyO3 Rust files under `vortex-python/src/` change, include: +Use `uvx` for ruff and `uv run` for basedpyright, matching how CI invokes each. Do not add a +`python -m py_compile` pass: every syntax error it can report is already reported by `ruff check`, +`basedpyright`, and pytest collection. -```bash -cargo +nightly fmt --check -p vortex-python -``` +If PyO3 Rust files under `vortex-python/src/` change, also run the Rust checks in the root +`AGENTS.md`, scoped with `-p vortex-python`. Always finish Python binding work with `git diff --check`. From acb189a9fe30f4829cd6a362d7f7a39ad056ee7c Mon Sep 17 00:00:00 2001 From: Robert Kruszewski Date: Thu, 10 Sep 2026 12:26:47 +0100 Subject: [PATCH 2/3] fixes Signed-off-by: Robert Kruszewski --- .github/workflows/ci.yml | 4 +- .github/workflows/cuda.yaml | 5 +- AGENTS.md | 81 +++++++++++++++++++-- pyproject.toml | 8 +- uv.lock | 27 ------- vortex-ffi/cmake/tests/support.py | 29 +++++--- vortex-python/AGENTS.md | 39 ---------- vortex-python/check.sh | 2 +- vortex-python/python/vortex/__init__.py | 2 +- vortex-python/python/vortex/store/_aws.py | 19 ++--- vortex-python/python/vortex/store/_http.py | 9 +-- vortex-python/python/vortex/store/_local.py | 9 +-- vortex-python/test/test_dataset.py | 2 +- vortex-python/test/test_polars_.py | 3 +- vortex-python/test/test_store.py | 16 +++- 15 files changed, 135 insertions(+), 120 deletions(-) delete mode 100644 vortex-python/AGENTS.md diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 34050c8844a..63a6a85e720 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -90,8 +90,8 @@ jobs: # Rust extension build entirely. - name: Python Lint - ty run: | - uv sync --all-packages --no-install-package vortex-data --no-install-package vortex-data-cuda - uv run --no-sync ty check vortex-python + uv sync --all-packages --no-install-workspace + uvx ty check vortex-python vortex-python-cuda vortex-ffi/cmake/tests scripts/tests python-test: name: "Python (test)" diff --git a/.github/workflows/cuda.yaml b/.github/workflows/cuda.yaml index 5d099f20c5c..a076f7d08cc 100644 --- a/.github/workflows/cuda.yaml +++ b/.github/workflows/cuda.yaml @@ -134,9 +134,8 @@ jobs: env: MATURIN_PEP517_ARGS: "--profile ci" run: | - # --all-packages installs the shared dev tooling (ty) from the - # root `dev` group; --extra cuda adds the vortex-data-cuda extension. - uv run --all-packages --extra cuda ty check vortex-python vortex-python-cuda + uv sync --all-packages --extra cuda + uvx ty check vortex-python vortex-python-cuda uv run --all-packages --extra cuda pytest --benchmark-disable vortex-python/test/test_cuda.py vortex-python-cuda/test cuda-test-sanitizer: diff --git a/AGENTS.md b/AGENTS.md index 7c9af7e7206..e552a3649b1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -37,7 +37,6 @@ Before changing files in a subtree, read the closest nested `AGENTS.md`. In part - `.github/AGENTS.md` covers workflows and other GitHub configuration. - `docs/AGENTS.md` covers Sphinx documentation. -- `vortex-python/AGENTS.md` covers Python and PyO3 binding work. ## Verification @@ -45,6 +44,11 @@ Run the narrowest check that covers the files you changed. CI already runs the w version of everything below, so reproducing that breadth locally costs far more than the round trip it saves. +Test execution is optional and left to the user. The commands below document how tests are run +in this repository; they are not mandatory completion checks. Run tests when the user requests +them, and report which tests ran or were not run. Continue to run applicable lint, formatting, +and static checks. + Do not run Rust checks at all for changes that only touch Markdown, RST, Sphinx configuration, agent configuration, comments outside Rust code, symlinks, or other metadata with no Rust/API behavior impact. Validate those by inspection or with a targeted doc/config command, and verify @@ -55,7 +59,6 @@ symlink or path changes with `ls`, `find`, and `git status`. For Rust code, public API, feature flag, or generated-file changes, run these before stopping: ```bash -cargo nextest run -p cargo clippy -p --all-targets --all-features -- -D warnings cargo +nightly- fmt -p ``` @@ -68,16 +71,69 @@ Two details make these match CI rather than merely resemble it: `+nightly` can format differently from the toolchain CI checks against, so the reformatted tree still fails `fmt --check`. -There is no separate build step. `cargo nextest run` and `cargo clippy --all-targets` each compile -the crate; a preceding `cargo build` only repeats that work under a third profile. +There is no separate build step: `cargo clippy --all-targets` compiles the crate. -For Rust doc comments or crate documentation, also run doctests for the affected crate: +When the user wants Rust tests, scope them to the affected crate. Doctests cover Rust doc comments +and crate documentation: ```bash +cargo nextest run -p cargo test --doc -p ``` -If cargo-nextest is not available, install it with `cargo install --locked cargo-nextest`. +If needed for a requested test run, install cargo-nextest with `cargo install --locked cargo-nextest`. + +### Python + +The following applies to Python bindings and their PyO3 implementation under `vortex-python/`, +and to CUDA bindings under `vortex-python-cuda/`. Run commands from the repository root. + +Follow the [Python binding development workflow](CONTRIBUTING.md#python-bindings) for environment +setup, Maturin rebuilds, targeted testing, Cargo features, and the full Python check. Keep the +contributor guide as the source of truth for shared commands; its test workflows are available +when the user chooses to run them. + +Run the lint, formatting, and type checks that match the files changed: + +```bash +uvx ruff format --check +uvx ruff check +uvx ty check vortex-python vortex-python-cuda vortex-ffi/cmake/tests scripts/tests +``` + +Use `uvx` for both Ruff and ty, matching CI. The command above covers both binding packages and the +CMake and script tests. For a narrower check, pass the affected directory, such as +`uvx ty check vortex-python` or `uvx ty check vortex-ffi/cmake/tests`. Type-check the whole binding +package when changing stubs or annotations so their callers are checked too. + +ty reads Python sources and stubs and needs third-party dependencies for type information. Prepare +them with `uv sync --all-packages --no-install-workspace` to avoid building the Rust extensions for +type checking. Runtime tests still need the installed extensions. + +Use targeted `# ty: ignore[rule-name]` comments for intentional violations, such as invalid-input +tests or third-party stub limitations, and explain non-obvious suppressions. Pyright suppression +comments do not suppress ty diagnostics. Keep shared ty configuration in the root `pyproject.toml`. + +Functions that only return `None`, including tests, may omit the `-> None` annotation. ty does not +require return annotations, and Ruff's `suppress-none-returning` setting permits omitting them for +these functions. Bare `return` and falling through are also allowed when the +return type permits `None`; Ruff's `RET502` and `RET503` rules are explicitly disabled. + +When the user wants Python tests, run the targeted suite with: + +```bash +uv run --all-packages pytest +``` + +Do not add a `python -m py_compile` pass: syntax errors are already reported by `ruff check`, +`ty check`, and pytest collection. + +For Python docstrings, `docs/api/python/`, or Sphinx configuration changes, follow +`docs/AGENTS.md`; the contributor guide documents clean Sphinx builds and doctests. Test execution +remains the user's choice. If PyO3 Rust files change, run the Rust lint and formatting checks above, +scoped to the affected binding crate (`-p vortex-python` or `-p vortex-python-cuda`). + +Always finish Python binding work with `git diff --check`. ### C++ and CUDA @@ -92,6 +148,15 @@ Pass only the files you changed; CI excludes vendored or generated CUDA and Arro repository-wide check. clang-format is idempotent, so a `--dry-run --Werror` pass over the files you just formatted cannot fail and is not worth running. +When the user wants CMake integration tests, CI runs the Python unittest suite with: + +```bash +python3 -m unittest discover -s vortex-ffi/cmake/tests -v +``` + +These tests need CMake, Ninja, the C/C++ and Rust toolchains, and the lockfile-selected Cargo +dependencies cached by `cargo fetch --locked`. + ### New and generated files These CI checks are the ones most often missed when adding files rather than editing them: @@ -131,8 +196,8 @@ These CI checks are the ones most often missed when adding files rather than edi - Prefer test module names `tests`, not `test`. - Use `assert_arrays_eq!` for array comparisons instead of element-by-element assertions. - Keep tests concise and focused on behavior, edge cases, and regressions. -- If a bug fix is requested, add or identify a failing test first when practical. A test that - passes before and after the fix does not prove the fix. +- If a bug fix is requested, add or identify a regression test when practical. Leave execution + to the user; when tests are run, a test that passes before and after the fix does not prove it. - If clippy lints in tests prohibit patterns that are acceptable only in test code, consider allowing the lint at the test module level. - If an existing `foo.rs` module needs many tests, promote it to a directory module: diff --git a/pyproject.toml b/pyproject.toml index eb8b80c3143..6a473ba4174 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -17,7 +17,6 @@ packages = ["dummy"] # Required for workspace project # Shared dev tooling. Member-specific dev deps live in each member's pyproject.toml. # `uv sync --all-packages` picks up dev groups from all workspace members. dev = [ - "ty>=0.0.74", "ipython>=8.26.0", "pip>=23.3.2", "pytest>=7.4.0", @@ -55,9 +54,15 @@ exclude = ["*.md"] [tool.ruff.lint] select = ["F", "E", "W", "UP", "I", "ANN"] +# Allow bare returns and implicit None returns in functions with optional results. +ignore = ["RET502", "RET503"] # Do not auto-fix unused variables. This is really annoying when IntelliJ runs autofix while editing. unfixable = ["F841"] +[tool.ruff.lint.flake8-annotations] +# Functions that only return None, including tests, do not need a -> None annotation. +suppress-none-returning = true + [tool.ruff.lint.per-file-ignores] # Pytest supplies fixture parameters dynamically. "**/test*/**/*.py" = ["ANN001"] @@ -90,6 +95,7 @@ root = [ "vortex-python", "vortex-python-cuda/python", "vortex-python-cuda", + "vortex-ffi/cmake/tests", ] [tool.ty.src] diff --git a/uv.lock b/uv.lock index c5f927c94b4..dbe3b92fc15 100644 --- a/uv.lock +++ b/uv.lock @@ -2233,31 +2233,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/00/c0/8f5d070730d7836adc9c9b6408dec68c6ced86b304a9b26a14df072a6e8c/traitlets-5.14.3-py3-none-any.whl", hash = "sha256:b74e89e397b1ed28cc831db7aea759ba6640cb3de13090ca145426688ff1ac4f", size = 85359, upload-time = "2024-04-19T11:11:46.763Z" }, ] -[[package]] -name = "ty" -version = "0.0.74" -source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/88/0f/c767853e88567a2ec7e996dd95e3105b1bc62c95d103689311ef0f4a603c/ty-0.0.74.tar.gz", hash = "sha256:da14344fc8625fc9ff359bafb856ad575636ea86d9bb6a629b146bff27b380e6", size = 6786318, upload-time = "2026-08-22T15:05:54.054Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/2c/95/6ded58bc97885c6d88fa1f9cd815031489200738f961cbf0466663213f80/ty-0.0.74-py3-none-linux_armv6l.whl", hash = "sha256:8969ef4e508debf00cf58f9ea85a539f799b1732c59cdfcecd037630b9755b30", size = 12790043, upload-time = "2026-08-22T15:05:05.015Z" }, - { url = "https://files.pythonhosted.org/packages/d9/8a/5e323603b6ab8731144421877ee8a0f8ac5a5511e67857127caa09f6730e/ty-0.0.74-py3-none-macosx_10_12_x86_64.whl", hash = "sha256:51fb6cf5b98e1e1140825b2430943f78d744876a735231656eafbb4c3f7eca3c", size = 12371748, upload-time = "2026-08-22T15:05:08.609Z" }, - { url = "https://files.pythonhosted.org/packages/d0/44/ee72e08cb705281e8d8c42917dd577aa598a8a098008495fda5176ee3f6e/ty-0.0.74-py3-none-macosx_11_0_arm64.whl", hash = "sha256:8ebe60b1f0a948c793d6c77fc9e9ddda599e4f023c04ab16e8e03bcb428c3fa0", size = 12282403, upload-time = "2026-08-22T15:05:11.448Z" }, - { url = "https://files.pythonhosted.org/packages/da/b3/fd935b694ff68bc278af50f7ad04770b36ce6306399baef7e1847b553a9d/ty-0.0.74-py3-none-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:aa97f407a695c890a53615966a663c7d2167e2cabe88db7ca1a24d62635cdfc8", size = 12345164, upload-time = "2026-08-22T15:05:14.19Z" }, - { url = "https://files.pythonhosted.org/packages/54/5c/5b5825268e029ebb164c909780103dbbae367f069801410068bf1cef29b3/ty-0.0.74-py3-none-manylinux_2_17_armv7l.manylinux2014_armv7l.whl", hash = "sha256:673ddb733d4a0db31385ba1ed9ff1f6bd9dc5565413ce57b1ca5ac4c7803da5d", size = 12556646, upload-time = "2026-08-22T15:05:16.994Z" }, - { url = "https://files.pythonhosted.org/packages/56/e7/515914e571d62ce0101744fed3f881936eeb1b30dc37beb72b4f7ca1e289/ty-0.0.74-py3-none-manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:1028e7c6b4f6145e9704552f43a5fffdcd51b42263ffdcd9c9677762bc395a4a", size = 13311653, upload-time = "2026-08-22T15:05:20.254Z" }, - { url = "https://files.pythonhosted.org/packages/b0/07/d1452babb6f9266c2122cabc095180b70ed306fb770b2996753814d2237d/ty-0.0.74-py3-none-manylinux_2_17_ppc64le.manylinux2014_ppc64le.whl", hash = "sha256:79841a8890493021fb308772474983316eb91f7b56cb227a6a05a06b262a36f0", size = 13768284, upload-time = "2026-08-22T15:05:23.197Z" }, - { url = "https://files.pythonhosted.org/packages/b1/60/8d4a2fc7842a47210a1cb0a16a187d9de39ad5d509a00fb74c1c073afcde/ty-0.0.74-py3-none-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:94859d321f3c6a6c8f7bfc3f40e8319cda7e6e012e613440f3dfd145d5010e2e", size = 13422306, upload-time = "2026-08-22T15:05:26.248Z" }, - { url = "https://files.pythonhosted.org/packages/de/76/ebbc269a8c4efcc4d44624993bd188145f20d60ebda9680b15aaec42cc50/ty-0.0.74-py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:970a8b2c09ff3be04c8a1c6767332d861be4fce85efe7bb205e4ade7c8655274", size = 12970637, upload-time = "2026-08-22T15:05:29.15Z" }, - { url = "https://files.pythonhosted.org/packages/9e/dd/b99f7236acbf856780ca1779a48143d2d9f2c24d7f531a0ce15a022b8a87/ty-0.0.74-py3-none-manylinux_2_31_riscv64.whl", hash = "sha256:795f763b3ded85574c2c2846a6fb8acf2aa76e9e83d761143e92b1f0c7ffa2cd", size = 13344891, upload-time = "2026-08-22T15:05:32.033Z" }, - { url = "https://files.pythonhosted.org/packages/0b/d7/9ff7449a4c7e6428f2c6f298e74cf24b70668f29d45c249507a723ff3782/ty-0.0.74-py3-none-musllinux_1_2_aarch64.whl", hash = "sha256:dc086db5367d912c31c0cc872deb7387290e779a4b9b54fcb944673a7cd52c7b", size = 12395272, upload-time = "2026-08-22T15:05:34.702Z" }, - { url = "https://files.pythonhosted.org/packages/b1/dd/b23a5b6b35d37df89dc8dc5daa09efd9245a668b50c4c81c25de21567dc1/ty-0.0.74-py3-none-musllinux_1_2_armv7l.whl", hash = "sha256:0314d7b391cf684e47c2fa093d2ce4c597cfc9b01d9a315fe204aed6359b271b", size = 12573079, upload-time = "2026-08-22T15:05:37.683Z" }, - { url = "https://files.pythonhosted.org/packages/23/c5/ccba16239d6129533c8b3603458d0f4dd2ba69478e47059073968e74261d/ty-0.0.74-py3-none-musllinux_1_2_i686.whl", hash = "sha256:c4a45dd2e991e8bdae82ba78c8cd051b253f60bc71a6536598fa3ef580b4fc9b", size = 12832506, upload-time = "2026-08-22T15:05:40.505Z" }, - { url = "https://files.pythonhosted.org/packages/6d/1c/2390912634dff4f341f97b397f2aee341ff062be0a66cda37d59375454f2/ty-0.0.74-py3-none-musllinux_1_2_x86_64.whl", hash = "sha256:210e2eac6b018fb934e2b8dac3956a0ba076a3fb1fa6f135058c825e5b759b81", size = 13154752, upload-time = "2026-08-22T15:05:43.355Z" }, - { url = "https://files.pythonhosted.org/packages/c4/33/a8c12188227e6f74f91853a7374e01ed81d6ad21c16c8b70e92dbebfe46a/ty-0.0.74-py3-none-win32.whl", hash = "sha256:db0bb6a8f098ef9bd1be861f73b4f7c0320d40d4c05c7ae0a8677d4e7aa4f6e5", size = 12130002, upload-time = "2026-08-22T15:05:46.058Z" }, - { url = "https://files.pythonhosted.org/packages/21/5c/064f28ccb9c234cfce5a2f7aa69a256663d5ae5bb0290b3a9706cc4d1e4c/ty-0.0.74-py3-none-win_amd64.whl", hash = "sha256:bebff181515255b3c78bd2e7693ae66fab6064ad4feea2065c68bc01022aa678", size = 12771435, upload-time = "2026-08-22T15:05:48.811Z" }, - { url = "https://files.pythonhosted.org/packages/fe/06/d6becdaca0315346c26b6df97cb0eafa81de4f870945d6989e88704374ed/ty-0.0.74-py3-none-win_arm64.whl", hash = "sha256:1a3469eaaf8c85b1c0a15bede25d36daea4b09fce1d913e965b24e24b3f1d6c6", size = 12558299, upload-time = "2026-08-22T15:05:51.543Z" }, -] - [[package]] name = "typer" version = "0.21.1" @@ -2430,7 +2405,6 @@ dev = [ { name = "pip" }, { name = "pytest" }, { name = "ruff" }, - { name = "ty" }, { name = "urllib3" }, ] @@ -2448,7 +2422,6 @@ dev = [ { name = "pip", specifier = ">=23.3.2" }, { name = "pytest", specifier = ">=7.4.0" }, { name = "ruff", specifier = ">=0.7.1" }, - { name = "ty", specifier = ">=0.0.74" }, { name = "urllib3", specifier = ">=2.6.3" }, ] diff --git a/vortex-ffi/cmake/tests/support.py b/vortex-ffi/cmake/tests/support.py index 009963b38dc..8d0c2db6cff 100644 --- a/vortex-ffi/cmake/tests/support.py +++ b/vortex-ffi/cmake/tests/support.py @@ -12,18 +12,24 @@ import tempfile import textwrap import unittest +from collections.abc import Mapping from pathlib import Path -from typing import Any, TypedDict, Unpack +from typing import TypedDict, Unpack, cast REPO_ROOT = Path(__file__).resolve().parents[3] CMAKE_DIR = REPO_ROOT / "vortex-ffi/cmake" class CommandOptions(TypedDict, total=False): - env: dict[str, str] | None + env: Mapping[str, str] | None cwd: str | Path | None success: bool - timeout: int + timeout: float + + +class CargoRecording(TypedDict): + args: list[str] + env: dict[str, str] def rust_toolchain_environment() -> dict[str, str]: @@ -65,10 +71,10 @@ def executable(self, name: str, body: str) -> str: def command( self, *args: str | Path, - env: dict[str, str] | None = None, + env: Mapping[str, str] | None = None, cwd: str | Path | None = None, success: bool = True, - timeout: int = 120, + timeout: float = 120, ) -> subprocess.CompletedProcess[str]: result = subprocess.run( list(map(str, args)), @@ -87,12 +93,17 @@ def command( return result def cmake_configure( - self, source: Path, build: Path, *options: str, generator: str = "Ninja", **kwargs: Unpack[CommandOptions] + self, + source: str | Path, + build: str | Path, + *options: str, + generator: str = "Ninja", + **kwargs: Unpack[CommandOptions], ) -> subprocess.CompletedProcess[str]: return self.command("cmake", "-G", generator, "-S", source, "-B", build, *options, **kwargs) def cmake_build( - self, build: Path, *options: str, **kwargs: Unpack[CommandOptions] + self, build: str | Path, *options: str, **kwargs: Unpack[CommandOptions] ) -> subprocess.CompletedProcess[str]: return self.command("cmake", "--build", build, *options, **kwargs) @@ -128,5 +139,5 @@ def fake_rustc(self, release: str = "1.95.0", host: str | None = None) -> str: """, ) - def cargo_recording(self, target_dir: Path) -> dict[str, Any]: - return json.loads((target_dir / "environment.json").read_text(encoding="utf-8")) + def cargo_recording(self, target_dir: Path) -> CargoRecording: + return cast(CargoRecording, json.loads((target_dir / "environment.json").read_text(encoding="utf-8"))) diff --git a/vortex-python/AGENTS.md b/vortex-python/AGENTS.md deleted file mode 100644 index 2ae2b9a8e90..00000000000 --- a/vortex-python/AGENTS.md +++ /dev/null @@ -1,39 +0,0 @@ -# Python Binding Guidance - -Applies to the Python bindings and their PyO3 implementation under `vortex-python/`. - -Run the commands below from the repository root. - -## Build and Development - -Follow the [Python binding development workflow](../CONTRIBUTING.md#python-bindings) in the -contributor guide for environment setup, Maturin rebuilds, targeted testing, Cargo features, and -the full Python check. Keep the contributor guide as the source of truth for shared commands. - -## Testing - -Run the narrow checks that match the files changed before broader Python suites: - -```bash -uv run --all-packages pytest -``` - -If Python docstrings, `docs/api/python/`, or Sphinx configuration change, also follow -`docs/AGENTS.md` and run the relevant clean Sphinx checks. - -## Linting and Formatting - -```bash -uvx ruff format --check -uvx ruff check -uv run basedpyright vortex-python -``` - -Use `uvx` for ruff and `uv run` for basedpyright, matching how CI invokes each. Do not add a -`python -m py_compile` pass: every syntax error it can report is already reported by `ruff check`, -`basedpyright`, and pytest collection. - -If PyO3 Rust files under `vortex-python/src/` change, also run the Rust checks in the root -`AGENTS.md`, scoped with `-p vortex-python`. - -Always finish Python binding work with `git diff --check`. diff --git a/vortex-python/check.sh b/vortex-python/check.sh index b6e0a5446f7..97312a79ad0 100755 --- a/vortex-python/check.sh +++ b/vortex-python/check.sh @@ -17,7 +17,7 @@ pushd $ROOT/vortex-python maturin develop ruff format --check ruff check -ty check +uvx ty check . popd pushd $ROOT/docs diff --git a/vortex-python/python/vortex/__init__.py b/vortex-python/python/vortex/__init__.py index 4a6d122ddbd..6396ae75c43 100644 --- a/vortex-python/python/vortex/__init__.py +++ b/vortex-python/python/vortex/__init__.py @@ -91,7 +91,7 @@ from .file import VortexFile, open from .scan import RepeatedScan -assert _lib, "Ensure we eagerly import the Vortex native library" +_ = _lib # Ensure we eagerly import the Vortex native library. # Resolve the installed distribution version so it is available as vortex.__version__. diff --git a/vortex-python/python/vortex/store/_aws.py b/vortex-python/python/vortex/store/_aws.py index 74fe893821a..a8b6d0b6c2a 100644 --- a/vortex-python/python/vortex/store/_aws.py +++ b/vortex-python/python/vortex/store/_aws.py @@ -3,7 +3,7 @@ from collections.abc import Coroutine from datetime import datetime -from typing import Any, Literal, NotRequired, Protocol, Self, TypeAlias, TypedDict, Unpack, cast +from typing import Any, Literal, NotRequired, Protocol, Self, TypeAlias, TypedDict, Unpack from typing_extensions import override @@ -515,16 +515,13 @@ def from_url( S3Store """ - return cast( - Self, - super(cls).from_url( # ty: ignore[unresolved-attribute] - url, - config=config, - client_options=client_options, - retry_config=retry_config, - credential_provider=credential_provider, - **kwargs, - ), + return super().from_url( + url, + config=config, + client_options=client_options, + retry_config=retry_config, + credential_provider=credential_provider, + **kwargs, ) @override diff --git a/vortex-python/python/vortex/store/_http.py b/vortex-python/python/vortex/store/_http.py index b352bcf4af3..c76429e81cd 100644 --- a/vortex-python/python/vortex/store/_http.py +++ b/vortex-python/python/vortex/store/_http.py @@ -1,7 +1,7 @@ # SPDX-License-Identifier: MIT # SPDX-FileCopyrightText: Copyright (c) 2024 Development Seed -from typing import Self, cast +from typing import Self from typing_extensions import override @@ -53,12 +53,7 @@ def from_url( This is an alias of the :class:`~vortex.store.HTTPStore` constructor. """ - return cast( - Self, - super(cls).from_url( # ty: ignore[unresolved-attribute] - url, client_options=client_options, retry_config=retry_config - ), - ) + return super().from_url(url, client_options=client_options, retry_config=retry_config) @override def __eq__(self, value: object) -> bool: diff --git a/vortex-python/python/vortex/store/_local.py b/vortex-python/python/vortex/store/_local.py index f366129eec7..54af7d474a7 100644 --- a/vortex-python/python/vortex/store/_local.py +++ b/vortex-python/python/vortex/store/_local.py @@ -2,7 +2,7 @@ # SPDX-FileCopyrightText: Copyright (c) 2024 Development Seed from pathlib import Path -from typing import Self, cast +from typing import Self from typing_extensions import override @@ -67,12 +67,7 @@ def from_url( store = LocalStore.from_url(url) """ - return cast( - Self, - super(cls).from_url( # ty: ignore[unresolved-attribute] - url, automatic_cleanup=automatic_cleanup, mkdir=mkdir - ), - ) + return super().from_url(url, automatic_cleanup=automatic_cleanup, mkdir=mkdir) @override def __eq__(self, value: object, /) -> bool: diff --git a/vortex-python/test/test_dataset.py b/vortex-python/test/test_dataset.py index d23c2cbce62..cfa9fccf946 100644 --- a/vortex-python/test/test_dataset.py +++ b/vortex-python/test/test_dataset.py @@ -192,7 +192,7 @@ def test_filter_with_nested_null_dtype(tmp_path: Path) -> None: def test_duckdb(ds: vx.dataset.VortexDataset) -> None: - assert ds # the type checker cannot determine that ds is used by duckdb.execute + _ = ds # DuckDB resolves the dataset from the SQL query below. tbl = duckdb.execute("select * from ds where string >= '950000' and float < 975.0").arrow().read_all() assert len(tbl) == 6176 diff --git a/vortex-python/test/test_polars_.py b/vortex-python/test/test_polars_.py index 41b279770cb..4c0b36e45ce 100644 --- a/vortex-python/test/test_polars_.py +++ b/vortex-python/test/test_polars_.py @@ -37,8 +37,7 @@ ], ) def test_exprs(polars: pl.Expr, vortex: ve.Expr) -> None: - # Dump the clickbench filters - assert polars_to_vortex(polars) == vortex + assert polars_to_vortex(polars).serialize() == vortex.serialize() @pytest.fixture(scope="module") diff --git a/vortex-python/test/test_store.py b/vortex-python/test/test_store.py index 98b8a7a788c..d3909e5a4da 100644 --- a/vortex-python/test/test_store.py +++ b/vortex-python/test/test_store.py @@ -3,11 +3,25 @@ from pathlib import Path -from vortex.store import LocalStore +import pytest +from vortex.store import HTTPStore, LocalStore, S3Store import vortex as vx +@pytest.mark.parametrize( + "store_type, url, options", + [ + (LocalStore, "file:///", {}), + (HTTPStore, "https://example.com/data", {}), + (S3Store, "s3://test-bucket/data", {"region": "us-east-1", "skip_signature": True}), + ], +) +def test_store_from_url(store_type, url, options): + store = store_type.from_url(url, **options) + assert isinstance(store, store_type) + + def test_store_roundtrip(tmp_path: Path) -> None: # create a local store to write into local = LocalStore(prefix=tmp_path) From 956777bac77b886313d86d8f58a0277eff3ce83b Mon Sep 17 00:00:00 2001 From: Robert Kruszewski Date: Thu, 10 Sep 2026 13:19:53 +0100 Subject: [PATCH 3/3] more Signed-off-by: Robert Kruszewski --- AGENTS.md | 62 +++++++++++++++++++++++++------------------------------ 1 file changed, 28 insertions(+), 34 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index e552a3649b1..36ccc5ad1c9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -40,33 +40,29 @@ Before changing files in a subtree, read the closest nested `AGENTS.md`. In part ## Verification -Run the narrowest check that covers the files you changed. CI already runs the workspace-wide -version of everything below, so reproducing that breadth locally costs far more than the round -trip it saves. +Linting, formatting, testing, builds, benchmarks, and other verification commands are optional. +Users decide which checks to run and when. Do not run them automatically or make task completion +depend on them. The commands below, and verification commands in scoped guidance or contributor +workflows, are reference instructions for when the user requests a check. -Test execution is optional and left to the user. The commands below document how tests are run -in this repository; they are not mandatory completion checks. Run tests when the user requests -them, and report which tests ran or were not run. Continue to run applicable lint, formatting, -and static checks. - -Do not run Rust checks at all for changes that only touch Markdown, RST, Sphinx configuration, -agent configuration, comments outside Rust code, symlinks, or other metadata with no Rust/API -behavior impact. Validate those by inspection or with a targeted doc/config command, and verify -symlink or path changes with `ls`, `find`, and `git status`. +When verification is requested, use the narrowest check that covers the relevant changes. CI +already runs workspace-wide checks. Markdown, RST, Sphinx configuration, agent configuration, +comments outside Rust code, symlinks, and other metadata with no Rust/API behavior impact do not +need Rust checks. Targeted doc/config commands or path inspection with `ls`, `find`, and +`git status` are available for those changes. ### Rust -For Rust code, public API, feature flag, or generated-file changes, run these before stopping: +For requested Rust linting and formatting, scope these commands to the affected crate: ```bash cargo clippy -p --all-targets --all-features -- -D warnings cargo +nightly- fmt -p ``` -Two details make these match CI rather than merely resemble it: +To match CI when running these commands: -- `-D warnings` is required. Without it clippy exits successfully on exactly the warnings CI - rejects, so a local pass predicts nothing. +- Include `-D warnings` so clippy fails on the warnings that CI rejects. - Use the nightly pinned as `NIGHTLY_TOOLCHAIN` in `.github/workflows/ci.yml`. A floating `+nightly` can format differently from the toolchain CI checks against, so the reformatted tree still fails `fmt --check`. @@ -86,14 +82,15 @@ If needed for a requested test run, install cargo-nextest with `cargo install -- ### Python The following applies to Python bindings and their PyO3 implementation under `vortex-python/`, -and to CUDA bindings under `vortex-python-cuda/`. Run commands from the repository root. +and to CUDA bindings under `vortex-python-cuda/`. These commands use the repository root as their +working directory. Follow the [Python binding development workflow](CONTRIBUTING.md#python-bindings) for environment setup, Maturin rebuilds, targeted testing, Cargo features, and the full Python check. Keep the -contributor guide as the source of truth for shared commands; its test workflows are available -when the user chooses to run them. +contributor guide as the source of truth for shared commands; its verification workflows are +available when the user chooses to run them. -Run the lint, formatting, and type checks that match the files changed: +Python linting, formatting, and type-checking commands: ```bash uvx ruff format --check @@ -103,12 +100,12 @@ uvx ty check vortex-python vortex-python-cuda vortex-ffi/cmake/tests scripts/tes Use `uvx` for both Ruff and ty, matching CI. The command above covers both binding packages and the CMake and script tests. For a narrower check, pass the affected directory, such as -`uvx ty check vortex-python` or `uvx ty check vortex-ffi/cmake/tests`. Type-check the whole binding -package when changing stubs or annotations so their callers are checked too. +`uvx ty check vortex-python` or `uvx ty check vortex-ffi/cmake/tests`. Checking the whole binding +package covers callers affected by stub or annotation changes. -ty reads Python sources and stubs and needs third-party dependencies for type information. Prepare -them with `uv sync --all-packages --no-install-workspace` to avoid building the Rust extensions for -type checking. Runtime tests still need the installed extensions. +ty reads Python sources and stubs and needs third-party dependencies for type information. +`uv sync --all-packages --no-install-workspace` prepares those dependencies without building the +Rust extensions. Runtime tests still need the installed extensions. Use targeted `# ty: ignore[rule-name]` comments for intentional violations, such as invalid-input tests or third-party stub limitations, and explain non-obvious suppressions. Pyright suppression @@ -125,20 +122,17 @@ When the user wants Python tests, run the targeted suite with: uv run --all-packages pytest ``` -Do not add a `python -m py_compile` pass: syntax errors are already reported by `ruff check`, -`ty check`, and pytest collection. - For Python docstrings, `docs/api/python/`, or Sphinx configuration changes, follow -`docs/AGENTS.md`; the contributor guide documents clean Sphinx builds and doctests. Test execution -remains the user's choice. If PyO3 Rust files change, run the Rust lint and formatting checks above, -scoped to the affected binding crate (`-p vortex-python` or `-p vortex-python-cuda`). +`docs/AGENTS.md`; the contributor guide documents clean Sphinx builds and doctests. All verification +remains the user's choice. The Rust commands above cover PyO3 files when scoped to the affected +binding crate (`-p vortex-python` or `-p vortex-python-cuda`). -Always finish Python binding work with `git diff --check`. +`git diff --check` is available for checking patch whitespace. ### C++ and CUDA -Format changed `.cpp`, `.hpp`, `.cu`, `.cuh`, and `.h` files under `lang/cpp`, `vortex-cuda`, -`vortex-duckdb`, and `vortex-ffi` with the repository's `.clang-format` configuration: +For requested formatting of `.cpp`, `.hpp`, `.cu`, `.cuh`, and `.h` files under `lang/cpp`, +`vortex-cuda`, `vortex-duckdb`, and `vortex-ffi`, use the repository's `.clang-format` configuration: ```bash clang-format --style=file -i