Skip to content

Add pre-commit CI job and replace artifact-based generated-file checks#1247

Open
shi-eric wants to merge 1 commit intoNVIDIA:mainfrom
shi-eric:ershi/pre-commit-ci
Open

Add pre-commit CI job and replace artifact-based generated-file checks#1247
shi-eric wants to merge 1 commit intoNVIDIA:mainfrom
shi-eric:ershi/pre-commit-ci

Conversation

@shi-eric
Copy link
Contributor

@shi-eric shi-eric commented Feb 21, 2026

Summary

  • Add pre-commit CI job to both GitHub Actions and GitLab CI that runs uvx pre-commit run --all-files --show-diff-on-failure
  • Add two local pre-commit hooks: warp-check-generated-files (regenerates exports.h, version.h, __init__.pyi from Python source) and warp-check-version-consistency (validates version strings match across VERSION.md, config.py, version.h)
  • Remove artifact-dependent check-generated-files job from GitHub Actions (replaced by pre-commit)
  • Remove __init__.pyi stubs check from check-build-docs-output in GitHub Actions (replaced by pre-commit)
  • Slim down GitLab check generated filescheck generated docs (only Sphinx docs remain)
  • Move pre-commit hook scripts to tools/pre-commit-hooks/ for discoverability

Test plan

  • uvx pre-commit run -a passes locally
  • GitHub Actions pre-commit job passes
  • GitLab pre-commit checks job passes
  • GitLab check generated docs job passes

Summary by CodeRabbit

  • Chores
    • CI simplified: removed intermediate check jobs and artifact propagation; MR checks now focus only on Sphinx-generated docs (API and language references) and no longer enforce native or stub files.
    • Pre-commit enhanced: added local hooks to validate/regenerate generated outputs and enforce version consistency with auto-formatting.
    • Build tooling consolidated: generation of shared headers/stubs moved to a common utility used by the build flow.
    • Removed legacy publishing/version-check script used in CI.

@coderabbitai
Copy link

coderabbitai bot commented Feb 21, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Removed CI artifact propagation and two CI jobs; consolidated version/header generation into a new shared module; added repository pre-commit hooks and scripts to regenerate/check generated headers, stubs, and version consistency; adjusted GitLab MR docs checks to only validate Sphinx-generated docs.

Changes

Cohort / File(s) Summary
GitHub Actions workflow
.github/workflows/ci.yml
Removed check-stubs and check-generated-files jobs; removed artifact-url output and Upload native headers step from build-warp; removed native-file checks from check-build-docs-output.
GitLab CI config
.gitlab-ci.yml
Renamed MR check to focus on generated docs, removed linux-x86_64 from needs, dropped native-file and init.pyi checks; MR diff validation now only checks docs/api_reference and docs/language_reference.
Pre-commit config
.pre-commit-config.yaml
Added two local hooks: warp-check-generated-files and warp-check-version-consistency (always_run, pass_filenames: false); kept existing ruff entries.
Pre-commit hook scripts
tools/pre-commit-hooks/check_generated_files.py, tools/pre-commit-hooks/check_version_consistency.py
Added check_generated_files.py (regenerates warp/native/exports.h and warp/__init__.pyi, formats/checks with ruff; exposes main()). Added check_version_consistency.py (reads VERSION.md, regenerates warp/native/version.h when needed, compares to warp/config.py; exposes main() and helpers).
Generation utilities
warp/_src/generated_files.py, build_lib.py
New module warp/_src/generated_files.py provides generate_version_header() and generate_exports_header_file() and re-exports export_builtins/export_stubs. build_lib.py now imports and calls these functions, removing its in-file implementations.
Removed CI helper
tools/ci/publishing/check_version_consistency.py
Deleted legacy CI script that previously validated VERSION.md vs warp/config.py and warp/native/version.h (functionality moved to new pre-commit tooling).

Sequence Diagram(s)

mermaid
sequenceDiagram
participant Dev as Developer / CI
participant Repo as Repository
participant Hook as pre-commit hook
participant Gen as warp._src.generated_files
participant Ruff as ruff CLI
participant FS as Filesystem

Dev->>Repo: checkout / run hooks
Repo->>Hook: invoke check_generated_files / check_version_consistency
Hook->>Gen: call generate_exports_header_file / generate_version_header
Gen->>FS: write/update `warp/native/exports.h`, `warp/native/version.h`, `warp/__init__.pyi`
Hook->>Ruff: run `ruff format` and `ruff check --fix`
Ruff-->>Hook: return diagnostics/fixes
Hook-->>Dev: exit status (pass/fail)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately reflects the main changes: adding pre-commit CI job and replacing artifact-based generated-file checks.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link

greptile-apps bot commented Feb 21, 2026

Greptile Summary

This PR replaces artifact-dependent CI jobs for checking generated files (exports.h, version.h, __init__.pyi) with local pre-commit hooks, consolidating generation logic into a shared warp/_src/generated_files.py module. The GitLab CI gains a dedicated pre-commit checks job, while GitHub Actions relies on the pre-commit.ci service. The old check-generated-files GitHub Actions job and the check generated files GitLab job are slimmed down or removed accordingly.

  • Two new local pre-commit hooks added: warp-check-generated-files (regenerates exports.h and __init__.pyi) and warp-check-version-consistency (validates version strings and regenerates version.h)
  • generate_version_header and generate_exports_header_file moved from build_lib.py to warp/_src/generated_files.py for shared use by build scripts, docs, and pre-commit hooks
  • The version-consistency hook intentionally duplicates _c_copyright_header to avoid importing warp, keeping it dependency-free for pre-commit.ci
  • ci.skip is empty in .pre-commit-config.yaml — the warp-check-generated-files hook (which requires numpy and import warp) will run on pre-commit.ci, which may need verification
  • The comment in .github/workflows/pr.yml:35 still references "Generated files checks (exports.h, version.h)" which no longer exists in ci.yml

Confidence Score: 4/5

  • This PR is safe to merge with one item to verify: whether the warp-check-generated-files hook works on pre-commit.ci given its heavy dependencies.
  • The refactoring is clean and well-structured. The duplicated _c_copyright_header is explicitly documented and justified. The main concern is whether pre-commit.ci can handle the warp-check-generated-files hook (which imports the full warp package). The CI/CD changes correctly remove dependencies and slim down jobs. No functional regressions in build or version-header generation.
  • .pre-commit-config.yaml — verify that ci.skip correctly handles or skips the warp-check-generated-files hook on pre-commit.ci

Important Files Changed

Filename Overview
.github/workflows/ci.yml Removes the check-generated-files job, native-headers artifact upload, and stubs check from check-build-docs-output. Cleanly removes the artifact-based checks that are replaced by pre-commit hooks.
.gitlab-ci.yml Adds pre-commit checks job in quality stage and slims check generated filescheck generated docs to only validate Sphinx docs. Correctly removes the linux-x86_64 build dependency that was only needed for the old generated-file checks.
.pre-commit-config.yaml Adds two local pre-commit hooks for generated-file regeneration and version consistency. The warp-check-generated-files hook uses heavy dependencies (numpy, import warp) but ci.skip is empty, which may cause issues on pre-commit.ci.
build_lib.py Moves generate_version_header and generate_exports_header_file to warp/_src/generated_files.py and updates the import. The call sites in main() remain unchanged.
tools/ci/publishing/check_version_consistency.py Deleted — functionality replaced by tools/pre-commit-hooks/check_version_consistency.py with enhanced version.h regeneration capability.
tools/pre-commit-hooks/check_generated_files.py New pre-commit hook that regenerates exports.h and __init__.pyi, then formats with ruff. Handles ruff exit codes correctly (1=unfixable violations OK, >=2=internal error fails the hook).
tools/pre-commit-hooks/check_version_consistency.py New pre-commit hook replacing the CI-only version checker. Adds version.h regeneration (fixer) alongside the config.py consistency check. Deliberately duplicates _c_copyright_header to avoid importing warp.
warp/_src/generated_files.py New shared module consolidating generated-file functions from build_lib.py. Re-exports export_builtins and export_stubs from warp._src.context, adds generate_version_header with skip-if-unchanged optimization and generate_exports_header_file.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    subgraph "Pre-commit Hooks (New)"
        A[warp-check-generated-files] -->|regenerates| B[warp/native/exports.h]
        A -->|regenerates + ruff formats| C[warp/__init__.pyi]
        D[warp-check-version-consistency] -->|regenerates| E[warp/native/version.h]
        D -->|validates match| F{VERSION.md == config.py?}
        F -->|Yes| G[Pass]
        F -->|No| H[Fail with error]
    end

    subgraph "Shared Module"
        I[warp/_src/generated_files.py] -->|export_builtins| B
        I -->|export_stubs| C
        I -->|generate_version_header| E
        J[build_lib.py] -->|imports| I
        A -->|imports| I
    end

    subgraph "CI Jobs (Removed/Slimmed)"
        K[❌ check-generated-files - GitHub Actions]
        L[❌ native-headers artifact upload]
        M[❌ stubs check in check-build-docs-output]
        N[check generated docs - GitLab - docs only]
    end
Loading

Last reviewed commit: 7291f89

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

6 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (1)
tools/pre-commit-hooks/check_generated_files.py (1)

60-64: Prefer [sys.executable, "-m", "ruff"] over the internal ruff.__main__.find_ruff_bin API.

find_ruff_bin is an implementation detail of ruff's Python package and not part of its public API. In the pre-commit isolated virtualenv, sys.executable already points to the Python binary that has ruff installed as an additional_dependency, so using the module invocation is more robust and version-agnostic.

♻️ Proposed refactor
-    from ruff.__main__ import find_ruff_bin  # noqa: PLC0415
-
-    ruff = find_ruff_bin()
-    subprocess.run([ruff, "format", stub_path], check=True)
-    subprocess.run([ruff, "check", "--fix", "--unsafe-fixes", stub_path], check=False)
+    subprocess.run([sys.executable, "-m", "ruff", "format", stub_path], check=True)
+    subprocess.run([sys.executable, "-m", "ruff", "check", "--fix", "--unsafe-fixes", stub_path], check=False)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tools/pre-commit-hooks/check_generated_files.py` around lines 60 - 64, The
code imports and uses the internal ruff.__main__.find_ruff_bin() API to get a
ruff binary and call subprocess.run, which is fragile; replace that with
invoking the ruff module via the current Python interpreter: import sys and call
subprocess.run([sys.executable, "-m", "ruff", ...]) for both the format and
check invocations instead of using find_ruff_bin(), and remove the find_ruff_bin
import; update the subprocess.run calls that reference ruff and stub_path to use
[sys.executable, "-m", "ruff", ...].
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/ci.yml:
- Around line 249-251: Replace the mutable tag reference for the GitHub Action
in the workflow: change the "uses: actions/checkout@v4" entry to reference the
verified commit SHA instead (use the SHA
11bd71901bbe5b1630ceea73d27597364c9af683) so the checkout action is pinned;
locate the "uses: actions/checkout@v4" line in the CI workflow and update it to
use the full commit hash.

In `@tools/pre-commit-hooks/check_generated_files.py`:
- Around line 39-41: Remove the unused noqa suppressions on the import lines:
delete the "# noqa: E402" and "# noqa: PLC0415" comments from the import
statements that reference warp, warp.config and the functions export_stubs,
generate_exports_header_file, generate_version_header so the imports read
normally; this eliminates the RUF100 false positives and lets the ruff --fix
pre-commit step manage any required fixes automatically while keeping the
symbols warp, warp.config, export_stubs, generate_exports_header_file, and
generate_version_header unchanged.

In `@warp/_src/generated_files.py`:
- Around line 43-44: The generated version header currently uses the runtime
value current_year (assigned where version_header_path is defined) inside
generate_version_header which causes spurious diffs each New Year; change the
implementation to use a stable hardcoded year constant (matching
generate_exports_header_file's approach, e.g. 2022) or remove the year from the
header template entirely so that generate_version_header no longer depends on
datetime.date.today().year; update the variable/current_year usage in
generate_version_header and any template substitution to reference the fixed
value or omit the field.
- Around line 80-111: The generate_exports_header_file currently swallows all
exceptions (printing errors without re-raising), so a mid-write failure (e.g.,
in export_builtins) leaves a partial exports.h and prevents callers from
detecting the failure; update generate_exports_header_file to match
generate_version_header by removing the surrounding try/except (or at minimum
re-raising the caught exceptions after logging) so exceptions propagate to the
caller; ensure export_builtins(...) errors are not silently swallowed and remove
the dead FileNotFoundError handler that is unnecessary because os.makedirs(...,
exist_ok=True) ensures the directory exists.

---

Nitpick comments:
In `@tools/pre-commit-hooks/check_generated_files.py`:
- Around line 60-64: The code imports and uses the internal
ruff.__main__.find_ruff_bin() API to get a ruff binary and call subprocess.run,
which is fragile; replace that with invoking the ruff module via the current
Python interpreter: import sys and call subprocess.run([sys.executable, "-m",
"ruff", ...]) for both the format and check invocations instead of using
find_ruff_bin(), and remove the find_ruff_bin import; update the subprocess.run
calls that reference ruff and stub_path to use [sys.executable, "-m", "ruff",
...].

Comment on lines 249 to 251
- name: Checkout repository
uses: actions/checkout@v4
- name: Install uv
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

actions/checkout@v4 must be pinned to a commit hash.

The new pre-commit job uses a mutable tag reference. The verified SHA for v4.2.2 is 11bd71901bbe5b1630ceea73d27597364c9af683.

🔒 Proposed fix
-        uses: actions/checkout@v4
+        uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683  # v4.2.2

As per coding guidelines: "Pin GitHub Actions to commit hashes, not tags or semantic versions."

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Checkout repository
uses: actions/checkout@v4
- name: Install uv
- name: Checkout repository
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
- name: Install uv
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/ci.yml around lines 249 - 251, Replace the mutable tag
reference for the GitHub Action in the workflow: change the "uses:
actions/checkout@v4" entry to reference the verified commit SHA instead (use the
SHA 11bd71901bbe5b1630ceea73d27597364c9af683) so the checkout action is pinned;
locate the "uses: actions/checkout@v4" line in the CI workflow and update it to
use the full commit hash.

Comment on lines 39 to 41
import warp # noqa: E402 — populates builtin_functions via builtins.py
import warp.config # noqa: E402
from warp._src.generated_files import export_stubs, generate_exports_header_file, generate_version_header # noqa: E402
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Remove unused noqa directives (RUF100).

Ruff reports all four # noqa: E402 and # noqa: PLC0415 suppressions as unused because those rules are not enabled in the project's Ruff config. The ruff --fix step in the ruff pre-commit hook will auto-remove them; leaving them causes false RUF100 warnings on every run.

🧹 Proposed cleanup
-import warp  # noqa: E402 — populates builtin_functions via builtins.py
-import warp.config  # noqa: E402
-from warp._src.generated_files import export_stubs, generate_exports_header_file, generate_version_header  # noqa: E402
+import warp  # populates builtin_functions via builtins.py
+import warp.config
+from warp._src.generated_files import export_stubs, generate_exports_header_file, generate_version_header

And on line 60:

-    from ruff.__main__ import find_ruff_bin  # noqa: PLC0415
+    from ruff.__main__ import find_ruff_bin
🧰 Tools
🪛 Ruff (0.15.1)

[warning] 39-39: Unused noqa directive (non-enabled: E402)

Remove unused noqa directive

(RUF100)


[warning] 40-40: Unused noqa directive (non-enabled: E402)

Remove unused noqa directive

(RUF100)


[warning] 41-41: Unused noqa directive (non-enabled: E402)

Remove unused noqa directive

(RUF100)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tools/pre-commit-hooks/check_generated_files.py` around lines 39 - 41, Remove
the unused noqa suppressions on the import lines: delete the "# noqa: E402" and
"# noqa: PLC0415" comments from the import statements that reference warp,
warp.config and the functions export_stubs, generate_exports_header_file,
generate_version_header so the imports read normally; this eliminates the RUF100
false positives and lets the ruff --fix pre-commit step manage any required
fixes automatically while keeping the symbols warp, warp.config, export_stubs,
generate_exports_header_file, and generate_version_header unchanged.

Comment on lines 43 to 44
version_header_path = os.path.join(base_path, "warp", "native", "version.h")
current_year = datetime.date.today().year
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

current_year in the copyright header causes annual churn in version.h.

generate_version_header stamps the current calendar year into version.h's copyright notice. Since the hook always regenerates the file, a year change (e.g., 2026 → 2027) will produce a diff every January 1st, forcing developers to commit a change that is semantically meaningless. By contrast, generate_exports_header_file uses a hardcoded year (2022). Consider using a fixed year (or omitting the year from generated headers) to keep the output stable.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@warp/_src/generated_files.py` around lines 43 - 44, The generated version
header currently uses the runtime value current_year (assigned where
version_header_path is defined) inside generate_version_header which causes
spurious diffs each New Year; change the implementation to use a stable
hardcoded year constant (matching generate_exports_header_file's approach, e.g.
2022) or remove the year from the header template entirely so that
generate_version_header no longer depends on datetime.date.today().year; update
the variable/current_year usage in generate_version_header and any template
substitution to reference the fixed value or omit the field.

@shi-eric shi-eric force-pushed the ershi/pre-commit-ci branch from 6421f94 to 8f342d7 Compare February 22, 2026 18:02
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

7 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)

246-258: Consider caching the pre-commit hook environments.

uvx pre-commit run --all-files cold-creates all hook virtual environments on each run. Adding a ~/.cache/pre-commit cache step eliminates redundant installs (especially for numpy and ruff pulled by warp-check-generated-files).

⚡ Suggested cache step (insert before "Run pre-commit")
      - name: Cache pre-commit environments
        uses: actions/cache@<pinned-sha>  # pin to commit hash
        with:
          path: ~/.cache/pre-commit
          key: pre-commit-${{ hashFiles('.pre-commit-config.yaml') }}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/ci.yml around lines 246 - 258, Add a cache step to the
"pre-commit" job to persist pre-commit hook environments so uvx doesn't
cold-create venvs each run: insert a step named like "Cache pre-commit
environments" before the step "Run pre-commit" that uses actions/cache (pinned
to a commit SHA) with path set to ~/.cache/pre-commit and a key derived from the
pre-commit config (e.g. using hashFiles('.pre-commit-config.yaml')); this will
reduce redundant installs for hooks invoked by the "Run pre-commit" step.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tools/pre-commit-hooks/check_version_consistency.py`:
- Around line 125-126: Update the misleading recovery guidance printed by the
two print(...) calls: change the second message so it instructs developers to
run the dedicated helper that regenerates version.h locally (e.g., the
pre-commit hook script that produces version.h) rather than telling them to run
"uv run build_lib.py" which only updates config.py in nightly CI; also mention
that updating config.py is performed by CI (CI_PIPELINE_SOURCE == "schedule") so
developers should rely on the helper to regenerate version.h and open a
follow-up CI/nightly run to propagate config.py if needed.

---

Duplicate comments:
In @.github/workflows/ci.yml:
- Around line 249-250: The workflow uses the actions/checkout@v4 tag which must
be pinned to a commit SHA; update the uses entry that references
actions/checkout@v4 to the verified commit SHA for v4.2.2
(11bd71901bbe5b1630ceea73d27597364c9af683) so the step referencing
actions/checkout is changed to use the pinned commit instead of the tag.

In `@tools/pre-commit-hooks/check_generated_files.py`:
- Line 60: Remove the dead noqa directive on the ruff import: the import "from
ruff.__main__ import find_ruff_bin" should not include "noqa: PLC0415" because
PLC0415 isn't enabled in the project's Ruff config; edit the import line in
check_generated_files.py to drop the "noqa: PLC0415" suppressor (or replace it
with a relevant, enabled code if a suppression is actually required).
- Around line 39-41: The three dead `# noqa: E402` directives attached to the
imports of `warp`, `warp.config`, and the names `export_stubs`,
`generate_exports_header_file`, and `generate_version_header` should be removed:
open the import statements that include those symbols and delete the trailing `#
noqa: E402` comments so Ruff no longer flags/removes them on every run; ensure
imports remain on top-level and that no other noqa tokens are added.

In `@warp/_src/generated_files.py`:
- Line 69: The generated header currently calls datetime.date.today().year in
generate_version_header which creates yearly churn; change
generate_version_header (or the helper _c_copyright_header) to use a stable
value (e.g., a fixed constant year like 2022 or omit the year) instead of
computing the current year at runtime so generated version.h remains
deterministic—mirror the approach used in generate_exports_header_file.

---

Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 246-258: Add a cache step to the "pre-commit" job to persist
pre-commit hook environments so uvx doesn't cold-create venvs each run: insert a
step named like "Cache pre-commit environments" before the step "Run pre-commit"
that uses actions/cache (pinned to a commit SHA) with path set to
~/.cache/pre-commit and a key derived from the pre-commit config (e.g. using
hashFiles('.pre-commit-config.yaml')); this will reduce redundant installs for
hooks invoked by the "Run pre-commit" step.

@shi-eric shi-eric force-pushed the ershi/pre-commit-ci branch from 8f342d7 to 5ed7cd5 Compare February 22, 2026 18:39
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

8 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tools/pre-commit-hooks/check_version_consistency.py`:
- Line 150: The print statement using an f-string has no placeholders; change
print(f"version.h:              regenerated from VERSION.md") to a regular
string literal without the leading f (print("version.h:              regenerated
from VERSION.md")) to eliminate the unused f-string (F541) in the script
containing that print call.
- Line 1: Update the SPDX header string "# SPDX-FileCopyrightText: Copyright (c)
2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved." in the file to use
year 2026 so it matches the other new files (change 2025 → 2026).

---

Duplicate comments:
In @.github/workflows/ci.yml:
- Around line 239-240: Replace the unpinned action reference "uses:
actions/checkout@v4" with the pinned commit SHA for the verified release
(v4.2.2) so the workflow uses
"actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683"; update the uses
entry for actions/checkout accordingly to reference that SHA instead of the tag.

In `@tools/pre-commit-hooks/check_generated_files.py`:
- Around line 43-44: Remove the unused noqa directives from the import lines
that reference warp and the generated_files symbols (the lines importing warp
and "from warp._src.generated_files import export_stubs,
generate_exports_header_file") and also remove the duplicate suppression on the
later import (line referencing the same generated import at 60); since E402 and
PLC0415 are not enabled in Ruff, delete the "noqa: E402" (and any redundant
"noqa" tokens) so the imports are clean and Ruff won't re-add them on --fix.

In `@tools/pre-commit-hooks/check_version_consistency.py`:
- Line 78: The _regenerate_version_header function currently uses
datetime.date.today().year causing annual churn; change it to derive the year
exactly the same way generate_version_header does so both functions remain
byte-identical. Replace the datetime.date.today().year argument in the call to
_c_copyright_header with the same deterministic year value or helper used by
generate_version_header (reuse the same helper/function/constant) or read the
year from the existing version header content so the output matches
generate_version_header verbatim. Ensure you update only the year source so
_regenerate_version_header and generate_version_header produce identical bytes.

In `@warp/_src/generated_files.py`:
- Line 77: The code calls _c_copyright_header(datetime.date.today().year) which
causes annual churn; change it to use a fixed year literal or a module-level
constant (e.g., COPYRIGHT_YEAR = 2022) instead so
generate_version_header/new_content is stable across runs; update the assignment
in generated_files.py to pass that fixed value (refer to the new_content
assignment and _c_copyright_header call) and ensure any other callers use the
same constant.

if verbose:
print(f"VERSION.md: {version_md}")
print(f"config.py: {config_py_version}")
print(f"version.h: regenerated from VERSION.md")
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Remove spurious f prefix — f-string has no placeholders (F541).

📝 Proposed fix
-        print(f"version.h:              regenerated from VERSION.md")
+        print("version.h:              regenerated from VERSION.md")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
print(f"version.h: regenerated from VERSION.md")
print("version.h: regenerated from VERSION.md")
🧰 Tools
🪛 Ruff (0.15.1)

[error] 150-150: f-string without any placeholders

Remove extraneous f prefix

(F541)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tools/pre-commit-hooks/check_version_consistency.py` at line 150, The print
statement using an f-string has no placeholders; change print(f"version.h:      
regenerated from VERSION.md") to a regular string literal without the leading f
(print("version.h:              regenerated from VERSION.md")) to eliminate the
unused f-string (F541) in the script containing that print call.

@shi-eric shi-eric force-pushed the ershi/pre-commit-ci branch 2 times, most recently from 119429a to a52f796 Compare February 22, 2026 18:54
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

8 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
.pre-commit-config.yaml (1)

21-21: Consider upgrading to ruff ≥ 0.15 when ready.

ruff v0.15.0 (released February 3, 2026) introduced a new "2026 style guide" with formatting changes, including updated lambda and method-chain formatting. Staying on 0.14.10 is safe and intentional, but the project will eventually need to absorb these formatting changes. Bumping both additional_dependencies: [ruff==0.14.10] and rev: v0.14.10 together to the latest 0.15.x (keeping them in sync, as the existing comment notes) is the upgrade path.

Also applies to: 32-32

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.pre-commit-config.yaml at line 21, Update the pinned ruff version to the
0.15.x series in both places so they stay in sync: replace the current
additional_dependencies entry referencing ruff==0.14.10 (the list item named
additional_dependencies) and the git hook rev (the rev value currently pinned to
v0.14.10) with the corresponding newer v0.15.x values; ensure both entries match
the same 0.15.y tag you choose (e.g., ruff==0.15.z and rev: v0.15.z) so the
pre-commit dependency and the hook rev remain consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@tools/pre-commit-hooks/check_generated_files.py`:
- Around line 43-44: Remove the unused inline noqa comments: delete the "# noqa:
E402" suffixes on the "import warp" and "from warp._src.generated_files import
export_stubs, generate_exports_header_file" lines and also remove the "# noqa:
PLC0415" on the later import; leave the imports themselves unchanged (import
warp and export_stubs/generate_exports_header_file) so behavior is preserved and
commit the cleaned file.

In `@tools/pre-commit-hooks/check_version_consistency.py`:
- Line 1: Update the SPDX copyright year in the new file
check_version_consistency.py from 2025 to 2026; locate the file header comment
at the top of check_version_consistency.py (the SPDX-FileCopyrightText line) and
change the year to 2026 so it matches the companion check_generated_files.py.
- Line 150: Remove the unnecessary f-string prefix on the print call so it isn't
flagged as an f-string with no placeholders (F541); locate the print statement
that currently reads print(f"version.h:              regenerated from
VERSION.md") and change it to a plain string print("version.h:             
regenerated from VERSION.md") to eliminate the spurious f prefix.

---

Nitpick comments:
In @.pre-commit-config.yaml:
- Line 21: Update the pinned ruff version to the 0.15.x series in both places so
they stay in sync: replace the current additional_dependencies entry referencing
ruff==0.14.10 (the list item named additional_dependencies) and the git hook rev
(the rev value currently pinned to v0.14.10) with the corresponding newer
v0.15.x values; ensure both entries match the same 0.15.y tag you choose (e.g.,
ruff==0.15.z and rev: v0.15.z) so the pre-commit dependency and the hook rev
remain consistent.

The check_generated_files.py pre-commit hook regenerates exports.h,
version.h, and __init__.pyi from Python source without needing build
artifacts. Add it (along with a version-consistency hook) to
.pre-commit-config.yaml and run it in a new lightweight pre-commit CI
job in both GitHub Actions and GitLab CI.

This replaces the artifact-dependent git-diff checks:
- GitHub: remove check-generated-files job, remove stubs check from
  check-build-docs-output
- GitLab: slim check-generated-files down to docs-only, rename to
  check-generated-docs

Move pre-commit hook scripts to tools/pre-commit-hooks/ for
discoverability.

Signed-off-by: Eric Shi <ershi@nvidia.com>
@shi-eric shi-eric force-pushed the ershi/pre-commit-ci branch from a52f796 to 7291f89 Compare February 22, 2026 19:41
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

8 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

"""Read and validate version from VERSION.md."""
with open(path) as f:
version = f.readline().strip()
if not re.fullmatch(r"\d+\.\d+\.\d+(\.\w+)?", version):
Copy link

Choose a reason for hiding this comment

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

Version regex rejects valid PEP 440 dev versions with digits in the pre-release segment

The regex r"\d+\.\d+\.\d+(\.\w+)?" only allows a single dot-separated segment after the patch number. This works for the current 1.12.0.dev0 format, but won't match versions with multiple pre-release segments like 1.12.0.post1.dev0. If such versions aren't used in this project, this is fine — just noting the constraint for future reference.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@greptile-apps
Copy link

greptile-apps bot commented Feb 22, 2026

Additional Comments (1)

.pre-commit-config.yaml
Consider skipping warp-check-generated-files on pre-commit.ci

The ci.skip list is empty, so warp-check-generated-files will attempt to run on pre-commit.ci. This hook does import warp which pulls in the full Warp package (including transitive dependencies resolved only via sys.path manipulation). While additional_dependencies: [numpy] should install numpy, the hook relies on the repo root being importable as a package via sys.path.insert. The docstring in check_generated_files.py (line 25) explicitly notes this hook has "heavy dependencies" that the version-consistency hook avoids — suggesting it may not work reliably on pre-commit.ci.

If this hook isn't intended to run on pre-commit.ci, add it to the skip list:

  skip: [warp-check-generated-files]

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tools/pre-commit-hooks/check_version_consistency.py`:
- Line 1: The file check_version_consistency.py is marked executable but has no
shebang, triggering Ruff EXE002; remove the executable bit in git by running git
update-index --chmod=-x check_version_consistency.py, then commit the change
(e.g., git commit -m "remove executable bit from check_version_consistency.py");
ensure the file remains unchanged otherwise (no shebang added) so the pre-commit
hook runs via language: python as intended.

---

Duplicate comments:
In `@tools/pre-commit-hooks/check_generated_files.py`:
- Around line 43-44: Remove the unused inline `# noqa` suppressions from the
import lines that are causing RUF100 diffs: delete the trailing comments on the
`import warp` line and the `from warp._src.generated_files import export_stubs,
generate_exports_header_file` line so they no longer include `# noqa: E402`
(these suppressions are not needed because E402/PLC0415 are not enabled and are
auto-stripped by the ruff hook).

In `@tools/pre-commit-hooks/check_version_consistency.py`:
- Line 149: The print call in check_version_consistency.py uses an unnecessary
f-string (print(f"version.h:              regenerated from VERSION.md")) causing
F541; change the f-string to a regular string by removing the leading f (or, if
you intended to interpolate values, add the proper placeholder and variable) so
the print statement becomes a plain string print("version.h:             
regenerated from VERSION.md").
- Line 77: _replace the use of datetime.date.today().year in
_regenerate_version_header so it uses the same hardcoded year constant as
generate_version_header (i.e., call _c_copyright_header with the shared YEAR
constant instead of computing today().year); update or add the shared YEAR
symbol so both functions reference it, and remove the now-unused import datetime
at the top of the file.
- Line 1: Update the SPDX copyright year in the SPDX header string from "2025"
to "2026" so the line reading "# SPDX-FileCopyrightText: Copyright (c) 2025
NVIDIA CORPORATION & AFFILIATES. All rights reserved." becomes "... 2026 ...";
edit the SPDX header at the top of check_version_consistency.py (the SPDX header
string) to reflect 2026.

In `@warp/_src/generated_files.py`:
- Line 77: Replace the volatile datetime.date.today().year call passed to
_c_copyright_header in generated_files.py with a fixed year constant (e.g. 2022)
or a module-level YEAR constant and use that instead of
datetime.date.today().year; update the call site (new_content =
_c_copyright_header(...)) to pass the constant and, if datetime is no longer
used elsewhere, remove the import datetime at the top of the file.

@@ -0,0 +1,189 @@
# SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# Check if the file is tracked as executable in git
git ls-files -s -- tools/pre-commit-hooks/check_version_consistency.py | awk '{print $1}'

Repository: NVIDIA/warp

Length of output: 61


🏁 Script executed:

# Check the first few lines of the file for shebang
head -n 5 tools/pre-commit-hooks/check_version_consistency.py

Repository: NVIDIA/warp

Length of output: 326


🏁 Script executed:

# Check if file is Python and view its structure
head -n 20 tools/pre-commit-hooks/check_version_consistency.py | tail -n 15

Repository: NVIDIA/warp

Length of output: 757


Remove the executable bit from this Python pre-commit hook file.

For language: python pre-commit hooks, the executable bit is unnecessary — pre-commit invokes the script using python <script>. The file is currently tracked as executable (mode 100755) but lacks a shebang, triggering the Ruff EXE002 error. Remove the executable bit from git's index.

🧰 Tools
🪛 Ruff (0.15.1)

[error] 1-1: The file is executable but no shebang is present

(EXE002)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tools/pre-commit-hooks/check_version_consistency.py` at line 1, The file
check_version_consistency.py is marked executable but has no shebang, triggering
Ruff EXE002; remove the executable bit in git by running git update-index
--chmod=-x check_version_consistency.py, then commit the change (e.g., git
commit -m "remove executable bit from check_version_consistency.py"); ensure the
file remains unchanged otherwise (no shebang added) so the pre-commit hook runs
via language: python as intended.

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.

1 participant