[abhirajadhikary06]

What does this PR do?

Enforces pre-commit in CI and adds ruff as the Python linter (#270). This is commit 1 of three — configuration only, touching no source file, so it does not conflict with the open 26.11.1.post1 PRs.

• pyproject.toml: [tool.ruff] with default rules E4/E7/E9/F at line length 88 (Black's default; E501 is intentionally not enabled, so the lines Black leaves long are not flagged).
• .pre-commit-config.yaml: adds ruff-check (check only), the basic hygiene hooks (trailing whitespace, end-of-file, merge-conflict markers, large files, private keys, check-yaml/toml/json/xml), prettier (Java, XML, Markdown, JSON; printWidth 160 for Java/XML, prose not hard-wrapped), shellcheck, and hadolint. Keeps black, isort, shfmt, pretty-format-yaml, actionlint.
• .prettierrc.json: printWidth 160 for *.java/*.xml, proseWrap: preserve.
• .github/workflows/pre-commit.yml: a CI job running uvx pre-commit run --all-files on every pull request and push to main.
• CONTRIBUTING.md: documents uvx pre-commit install and the Docker note for shellcheck/hadolint.

Related issues

Closes #270 .

Performance changes

None — configuration only.

Docs

CONTRIBUTING.md (the Pre-commit section). No README.md or docs/ change needed for config.

Checklist

• A regression test fails on the old code and passes on the new — N/A, no source change.
• The test suite that the change can affect passes — N/A, no source change.
• uvx pre-commit run --files <changed files> passes (run with SKIP=prettier,shellcheck,hadolint-docker,no-old-package-name: the prettier reformat is commit 3, the docker/python3 hooks are Linux-CI only).

Draft until release/26.11.1 merges to main. The Pre-commit CI job is expected to fail on the draft: ruff finds the 63 existing findings and prettier wants its first reformat. Commits 2 (ruff fixes) and 3 (prettier reformat) are added after the merge, each in its own commit; the PR then goes ready-for-review and the job passes.

Update:
Ran uvx pre-commit run --all-files on the rebased branch (rebased onto 7e05787):

• ruff-check: 58 errors across 15 files (25 safe-fixable with --fix, 10 more with --unsafe-fixes)
• prettier: 15 files modified
• shellcheck: findings in 4 files — 1 warning (SC2188 in scripts/run_examples.sh, fixed in this PR),
  rest are info-level (deferred to follow-up)
• hadolint: 17 findings in 1 file (scripts/Dockerfile.build, deferred to follow-up)
• shfmt, pretty-format-yaml, actionlint: Passed

[abhirajadhikary06]

@tae898 — opening this as a draft per your timing request.

This is commit 1 only: configuration that touches no source file, so it shouldn't conflict with the open 26.11.1.post1 PRs.

• pyproject.toml — [tool.ruff] (default rules E4/E7/E9/F, line length 88)
• .pre-commit-config.yaml — ruff-check, the hygiene hooks, prettier (Java/XML/Markdown/JSON), shellcheck, hadolint
• .prettierrc.json — printWidth 160 for *.java/*.xml, proseWrap preserve
• .github/workflows/pre-commit.yml — the uvx pre-commit run --all-files job on PR + push
• CONTRIBUTING.md — uvx pre-commit install + the Docker note

A couple of notes on choices, happy to change any:

• Ruff runs check-only (no --fix), so fixes land in their own commit; black/isort stay the formatters.
• For the 8 benign F811 fixture-shadowings in tests/, I used a tests/** per-file-ignores in ruff config. If you'd rather have inline # noqa: F811 in commit 2, I'll drop the ignore.
• Prettier uses types_or (OR) rather than upstream's types: [java, xml], since types is AND and would match no files.
• shellcheck and hadolint run as docker_image hooks (Docker pulls the images automatically, both locally and in CI).

As expected on the draft, the Pre-commit CI job fails — ruff finds the 63 findings and prettier wants its first reformat.

Commits 2 (ruff fixes) and 3 (prettier reformat) will be added after release/26.11.1 merges into main, each in its own commit; I'll mark it ready-for-review then. Please comment here when that merge has happened. Thanks!

[tae898]

Thank you, this is the shape I asked for: configuration only, actions pinned by SHA, uv pinned in the install URL, every hook pinned, and no source file touched.

One thing before it can go in. As you note in the description, the new Pre-commit job is expected to fail on the current code (ruff finds the 63 existing findings and prettier wants its first reformat). Merged alone, it would be red on every pull request and on main until the fix commits land, and I do not merge a pull request with a red check. So this pull request must not merge before the fixes do.

Please do two things:

• Convert this pull request to a draft now (it shows as ready for review at the moment). I will say here when release/26.11.1 has been merged into main; I expect the 26.11.1.post1 release to be out today and that merge to follow within a day or two.
• After that, add the fixes to this same pull request as separate commits on top of a rebased main: first the ruff fixes, then the prettier reformat of the Java and XML files, then whatever the whitespace and shell hooks change. The last commit is the one where uvx pre-commit run --all-files passes and the CI job goes green. It would help if you ran the hooks locally now and posted the number of failing files per hook, so that I can see the size of the fixes.

[abhirajadhikary06]

Converted to draft. Ran uvx pre-commit run --all-files locally — here are the counts:

• ruff-check: 59 errors across 15 files (25 auto-fixable with --fix, 10 more with --unsafe-fixes)
• prettier: 14 files modified (README.md, CLAUDE.md, SECURITY.md, examples/README.md, tests/README.md,
  .github/pull_request_template.md, .github/ISSUE_TEMPLATE/release.md, scripts/dev_engine_pin.json,
  native/wheel/README.md, native/wheel/THIRD_PARTY_NOTICES.md, two native/wheel/notice-texts/jdk/*.md,
  native/.../reachability-metadata.json, .prettierrc.json)
• shellcheck: findings in 4 files (scripts/run_examples.sh, scripts/build-native.sh, scripts/build.sh,
  native/scripts/build-image.sh) — all info/style level except one warning in run_examples.sh
• hadolint: 17 findings in 1 file (scripts/Dockerfile.build) — mostly DL4006 (pipefail) and DL3008 (pin apt versions)
• shfmt, pretty-format-yaml, actionlint: all Passed

Ready to start on the fix commits as soon as I get updates from you.

[tae898]

Thank you for the counts. The release line is on main now (#351, merge commit 6619f13), so you can rebase this pull request on it and start the fix commits. The numbers will move, because the merge brought in many files: please run uvx pre-commit run --all-files again on the rebased branch and put the new counts in the description.

A split that keeps the review small:

1. One commit with ruff check --fix only, the safe fixes. Please leave out --unsafe-fixes in this first pass.
2. One commit for the ruff findings that remain, by hand.
3. One commit for the prettier reformat, on its own, so that it can be skipped when I read the diff.
4. The one shellcheck warning in scripts/run_examples.sh. The info-level shellcheck findings and the hadolint findings in scripts/Dockerfile.build can be a follow-up pull request, so that this one stays reviewable.

You can take it out of draft when the rebase is done. If a finding looks like a change of behavior and not a style fix, ask here before changing it, and I will answer.

[abhirajadhikary06]

@tae898
Starting the manual fixes (commit 2). Two findings I want to confirm before touching:

1. E741 ambiguous I in src/arcadedb/_native/lib.py:223
P, L, I, S = ctypes.c_void_p, ctypes.c_longlong, ctypes.c_int, ctypes.c_char_p
I is used throughout the sigs dict below as a ctypes type shorthand. The fix is renaming it (e.g. CI for c_int). Pure rename, no logic change — but it's low-level ctypes. Should I fix it or add # noqa: E741?

2. F821 undefined name VectorIndex in src/arcadedb/core.py:1618
   The return annotation -> "VectorIndex" is a string annotation with no import. The fix is adding from .vector import VectorIndex under TYPE_CHECKING. Is that the right import path, or should this use # noqa: F821 instead?

[tae898]

Thank you for asking before touching them. Both can be fixed without noqa.

1. The P, L, I, S = ... line in src/arcadedb/_native/lib.py: rename it. I appears on 15 lines of that one function (the assignment and the entries of the sigs table), so the rename is small and changes no behavior. Please use N, so that the table keeps its one-letter aliases P, L, N and S, and extend the comment above the line so that it names the four aliases. The native tests load this table when the library starts, so the CI legs will catch a slip, and I will read the diff to check that only those 15 lines changed.

2. -> "VectorIndex" in src/arcadedb/core.py: yes, that is the right import path. VectorIndex is defined in src/arcadedb/vector.py, and core.py already imports it inside a function further down, which is how it avoids a circular import. So add from typing import TYPE_CHECKING and, under if TYPE_CHECKING:, from .vector import VectorIndex. The string annotation stays as it is, and noqa is not needed here. The line numbers on main are one off from yours (your branch predates the merge); the content is the same.

If a later finding is less clear than these two, ask here before changing it, and I will answer.

[abhirajadhikary06]

@tae898 Ruff commits are done. 11 findings remain, all marked unsafe by ruff. Before I touch them, confirming each:

examples/06_vector_search_recommendations.py:115 — F841 count = db.count_type("Movie"). Drop the assignment, keep the call. Safe?

examples/08_stackoverflow_tables_olap.py:1975 — E731 pid_provider = lambda: .... Rewrite as def pid_provider(): return .... Safe?

examples/download_data.py:2100 — F841 meta_path = out_dir / .... The path is computed but never read. Drop the assignment. Safe?

tests/test_bulk_insert.py:804 — F841 pd = __import__("pytest").importorskip("pandas"). Drop assignment, keep the importorskip call (it skips the test if pandas is absent). Safe?

tests/test_free_threading.py:350 — E731 gil = lambda: .... Rewrite as def gil(): return .... Safe?

tests/test_typed_rows.py:166 — F841 hash_map = jpype.JClass("java.util.LinkedHashMap"). Drop assignment, keep the call. Safe?

tests/test_vector.py:1084-1088 — F401 unused numpy import + F841 use_numpy never used. Remove the import and the try/except entirely. Safe?

tests/test_vector_sql.py:385 — F841 vec_data = arcadedb.to_python_array(...). Drop assignment, keep the call. Safe?

tests/test_vector_sql.py:804, 870 — F841 index_name assigned but never used. Drop assignment, keep the expression. Safe?

[tae898]

Thank you for confirming each one before touching it. I read the code on main for every item (nothing run), and all nine are fine, some with a better variant. In your order:

1. examples/06_vector_search_recommendations.py: delete the whole line, not only the assignment. count_type is a plain read inside a try/except Exception: pass, so a bare call does nothing useful.
2. examples/08_stackoverflow_tables_olap.py: yes. Use the shape that 07_stackoverflow_tables_oltp.py already has: def pid_provider() -> List[int]: with return list_postgres_pids(pgdata) on its own line.
3. examples/download_data.py: yes, drop the assignment. The path is never read and joining it has no side effect.
4. tests/test_bulk_insert.py: keep the guard as the first statement, written as pytest.importorskip("pandas") (pytest is already imported there). A bare call keeps the skip.
5. tests/test_free_threading.py: yes, def gil() -> bool: with the return on its own line.
6. tests/test_typed_rows.py: delete the whole line in the helper; the JVM is already needed on the line above. The hash_map further down is a different local and stays.
7. tests/test_vector.py: remove the numpy import and the use_numpy try/except in test_lsm_euclidean_distance only. In that test the block just sets a flag that nothing reads. The blocks with the same names in the other tests choose the numpy or the list path, so they stay.
8. tests/test_vector_sql.py, the vec_data line: delete the line and fix the comment next to the commented-out # assert vec_data[0] > 0.9. Please do not enable that assert in a lint pull request. If you want a check there, mirror the test for the binary vector right below it with assert vertex is not None, and say so in the commit message.
9. tests/test_vector_sql.py, the two index_name lines: at the first, delete index_name = indexes[0], because the next assert any(...) already covers an empty list. At the second, replace it with assert any("DocSql" in idx for idx in indexes), indexes, which passes exactly when today's code does.

Everything except the optional assertions in 8 and 9 is a deletion or an equivalent rewrite, so your ruff run and a normal CI run cover it. When CI is green, take the pull request out of draft and I will review it.

[abhirajadhikary06]

@tae898 please confirm the ones below too:
Four new E741 findings appeared in native/wheel/gen_notices.py (came in with the merge):

• line 85: l in a generator expression for l in s.split("\n")
• line 107: l in a list comprehension for l in root.findall(...)
• line 773: l in for l in diff[:80]
• line 789: l in for l in difflib.unified_diff(...)

All are loop variable renames (l → ln). Safe to fix?

[tae898]

Yes, ln is fine: that name is not used anywhere else in native/wheel/gen_notices.py. Please rename the uses too, not only the loop variables, because each l is also read in the body:

• the generator that strips trailing whitespace: l.rstrip() before the for;
• the list comprehension over the license elements: l.findtext(...) is on the line before the for l in root.findall(...) line, so it is easy to miss;
• the two debug loops over a diff: print(l[:300], ...) and print(l[:400], ...) inside the loop bodies.

A rename of the for clause alone would leave a NameError, in a script that runs when the native wheel is built. The Test native (arcadedb) run executes it (the notices drift check described in the release page), so CI will catch a missed one, but it is cheaper to see it in the diff: all four renames should be one commit with the l to ln change on every line that used it.

[abhirajadhikary06]

ruff, prettier, shfmt, pretty-format-yaml, actionlint, and all hygiene hooks now pass.

The remaining failures are shellcheck (info-level + 2 warnings in build-native.sh and build-image.sh) and hadolint (Dockerfile.build) — both deferred to the follow-up PR as agreed.
Now it's ready for review @tae898

[tae898]

Thank you, this is nearly there. Two things have to change before I approve the CI run, and one thing you should know:

1. The prettier pass reformats files that are generated or are legal text, and CI compares generated files byte for byte. native/wheel/THIRD_PARTY_NOTICES.md is generated from the build log of the native image (the release page describes the step), and the Test native (arcadedb) run compares it with the committed copy, so the first run would stop there. The two files under native/wheel/notice-texts/jdk/ are license texts and should not be reformatted either. The 5,000 line reformat of native/src/main/resources/META-INF/native-image/org.humemai/arcadedb-native-lib/reachability-metadata.json is a native-image configuration file that I would also leave as it is. Please restore these files to their content on main, and add a .prettierignore that lists the notices file, the native/wheel/notice-texts/ directory and the native/src/main/resources/META-INF/native-image/ directory, so that the hook stays green without touching them.

2. The new Pre-commit workflow runs every hook, so it would be red on every pull request and on main while shellcheck and hadolint still have findings. Until the follow-up pull request, set SKIP: shellcheck,hadolint-docker in the env: of the Run pre-commit on all files step, with a one-line comment that names the follow-up, and remove the line there.

3. No checks have run on this branch yet: the workflows of a fork wait for a maintainer to approve them. I will approve them as soon as you have pushed 1 and 2, so that the first run is meaningful.

The workflow itself matches the conventions of the repository (pinned action SHAs, read-only permissions, a pinned uv), and the ruff configuration matches what we agreed on #270. I am also checking the Python changes statement by statement, to confirm that the ruff commits change no behavior, and I will post the result here.

[tae898]

I checked the Python changes statement by statement: I compared the syntax tree of every changed .py file before and after. Of the 34 files, 5 have an identical tree (formatting only) and 29 differ. What differs is an unused name removed, a lambda turned into a def, or a renamed loop variable, apart from two points that need a change and one that I accept:

1. A regression. src/arcadedb/jvm.py no longer imports render_logging_config as _render_logging_config, because ruff sees it as unused, but tests/test_log_dir.py still calls jvm._render_logging_config(engine), so test_rendered_config_keeps_the_engine_settings_and_moves_the_file_log would fail with an AttributeError. Please fix it on the test side: import render_logging_config from arcadedb._logdir in the test and call that. The alias in jvm.py only existed as a hook for the test.

2. The new comment in pyproject.toml says that the duplicate TestInsertManyVertexType in tests/test_bulk_insert.py "is removed with the ruff-fixes commit", but both copies are still there. The two class bodies are identical, and pytest already collects only the second one, so please delete the first copy. No test that runs today changes.

3. In tests/test_vector_sql.py, dropping vec_data = arcadedb.to_python_array(...) removes the only call in that test that exercised the conversion. I accept that in a lint pull request, with the assert vertex is not None you added. Please say it in the commit message.

Everything else is behavior-neutral: the unused imports and assignments (nothing else uses them), the TYPE_CHECKING import of VectorIndex (the lazy import at runtime stays), the renames (l to ln and I to N; the old names are gone from the three files), and the Black re-wrapping. The large addition in tests/test_release_name_guards.py is only re-wrapping plus one removed unused import.

When you have pushed these two fixes and the two from my previous comment, I approve the workflows.

[abhirajadhikary06]

@tae898 All three fixes from your review are pushed:

1. tests/test_log_dir.py: import render_logging_config directly from
   arcadedb._logdir and call that instead of jvm._render_logging_config.

2. tests/test_bulk_insert.py: first (duplicate) copy of
   TestInsertManyVertexType deleted; pyproject.toml comment updated.

3. Trailing whitespace left after the deletion cleaned up.

Local hook run: ruff, black, isort, prettier, shfmt, pretty-format-yaml,
and actionlint all pass. shellcheck and hadolint remain failing on the
deferred info-level and Dockerfile findings — those go in the follow-up PR.

Ready for your review.

[tae898]

Thank you, those three are what I asked for. Two items from my earlier comment, the one about the generated files, are still open, and they decide whether the first CI run can be green:

1. native/wheel/THIRD_PARTY_NOTICES.md, the two files under native/wheel/notice-texts/jdk/ and reachability-metadata.json are still reformatted in the diff. Please restore them to their content on main (git checkout origin/main -- <the four paths>) and add a .prettierignore that lists the notices file, the native/wheel/notice-texts/ directory and the native/src/main/resources/META-INF/native-image/ directory. Run the hooks again afterwards: prettier should stay green without touching them.
2. The Run pre-commit on all files step of the new workflow still runs shellcheck and hadolint, so the job would fail on every pull request until the follow-up. Add SKIP: shellcheck,hadolint-docker to the env: of that step, with a one-line comment that names the follow-up pull request.

When both are pushed I approve the workflow runs and do the final read of the diff.

[abhirajadhikary06]

Both fixes pushed:

1. .prettierignore added — the four generated files (THIRD_PARTY_NOTICES.md,
   two JDK notice texts, reachability-metadata.json) are restored to main's
   content and excluded from prettier. The hook run is now fully green locally
   with SKIP=no-old-package-name (Windows python3 alias).

2. .github/workflows/pre-commit.yml: SKIP: shellcheck,hadolint-docker added
   to the CI step with a comment naming the follow-up PR.

Ready for final review.