From c181f78117ec5d408e1ddd6e70b59852f7d910e5 Mon Sep 17 00:00:00 2001 From: Farhan Syah Date: Wed, 26 Nov 2025 08:48:07 +0800 Subject: [PATCH 1/5] fix(ci): use correct rust-toolchain action and venv for maturin - Replace dtolnay/rust-action with dtolnay/rust-toolchain - Create venv for maturin develop command - Use .venv/bin/python for test step --- .github/workflows/ci.yml | 16 ++++++++-------- .github/workflows/release.yml | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 91ea6ac..0095714 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,7 +22,7 @@ jobs: - uses: actions/checkout@v4 - name: Install Rust - uses: dtolnay/rust-action@stable + uses: dtolnay/rust-toolchain@stable - name: Install PCRE2 (Ubuntu) if: matrix.os == 'ubuntu-latest' @@ -51,9 +51,9 @@ jobs: - uses: actions/checkout@v4 - name: Install Rust - uses: dtolnay/rust-action@stable + uses: dtolnay/rust-toolchain@stable with: - components: rustfmt + components: rustfmt, clippy - name: Check formatting run: cargo fmt --check @@ -72,17 +72,17 @@ jobs: - name: Install PCRE2 run: sudo apt-get update && sudo apt-get install -y libpcre2-dev - - name: Install dependencies + - name: Install dependencies and build run: | + python -m venv .venv + . .venv/bin/activate python -m pip install --upgrade pip pip install maturin tiktoken - - - name: Build and install - run: maturin develop --release + maturin develop --release - name: Test Python bindings run: | - python -c " + .venv/bin/python -c " import splintr import tiktoken diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index c608c00..1130d21 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -17,7 +17,7 @@ jobs: - uses: actions/checkout@v4 - name: Install Rust - uses: dtolnay/rust-action@stable + uses: dtolnay/rust-toolchain@stable - name: Install PCRE2 dependencies run: sudo apt-get update && sudo apt-get install -y libpcre2-dev From 287ae9e0e285aa219d0661d24bfee51b14e8f3f2 Mon Sep 17 00:00:00 2001 From: Farhan Syah Date: Wed, 26 Nov 2025 08:50:51 +0800 Subject: [PATCH 2/5] fix(ci): run only on pull requests, not on push to main --- .github/workflows/ci.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0095714..2312e96 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,8 +1,6 @@ name: CI on: - push: - branches: [main] pull_request: branches: [main] From 618b912fe460527f6495336b96e4b3106f92b561 Mon Sep 17 00:00:00 2001 From: Farhan Syah Date: Wed, 26 Nov 2025 09:05:09 +0800 Subject: [PATCH 3/5] fix(ci): resolve CI failures and add pre-commit hook - Pin Python 3.12 in test job (runner default 3.14 exceeds PyO3 max 3.13) - Apply rustfmt formatting fixes - Fix clippy collapsible_if warning - Add pre-commit hook for local development --- .github/workflows/ci.yml | 5 ++++ README.md | 9 +++++- hooks/pre-commit | 60 ++++++++++++++++++++++++++++++++++++++++ src/core/tokenizer.rs | 5 +--- src/lib.rs | 4 ++- src/python/bindings.rs | 8 +++--- src/python/mod.rs | 2 +- 7 files changed, 82 insertions(+), 11 deletions(-) create mode 100755 hooks/pre-commit diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2312e96..defcd0f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,6 +19,11 @@ jobs: steps: - uses: actions/checkout@v4 + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: '3.12' + - name: Install Rust uses: dtolnay/rust-toolchain@stable diff --git a/README.md b/README.md index 0a4dfcd..4497a23 100644 --- a/README.md +++ b/README.md @@ -296,6 +296,10 @@ Contributions are welcome! Here's how you can help: git clone https://github.com/farhan/splintr.git cd splintr +# Install pre-commit hook (recommended) +cp hooks/pre-commit .git/hooks/pre-commit +chmod +x .git/hooks/pre-commit + # Build the Rust library cargo build --release @@ -305,9 +309,12 @@ maturin develop --release # Run tests cargo test # Rust tests -pytest python/tests/ # Python tests (if available) +cargo clippy --all-targets # Linting +cargo fmt --all --check # Format check ``` +The pre-commit hook automatically runs formatting, clippy, and tests before each commit. + ## License This project is licensed under the MIT License - see the LICENSE file for details. diff --git a/hooks/pre-commit b/hooks/pre-commit new file mode 100755 index 0000000..39e8bcf --- /dev/null +++ b/hooks/pre-commit @@ -0,0 +1,60 @@ +#!/bin/bash +# +# Pre-commit hook for splintr +# +# Install by copying to .git/hooks/: +# cp hooks/pre-commit .git/hooks/pre-commit +# chmod +x .git/hooks/pre-commit +# + +set -e + +RED='\033[0;31m' +GREEN='\033[0;32m' +YELLOW='\033[1;33m' +NC='\033[0m' + +echo "Running pre-commit checks..." +echo "" + +FAILED=0 + +# 1. Rust formatting +echo -n "Checking formatting... " +if ! cargo fmt --all --check > /dev/null 2>&1; then + echo -e "${RED}FAILED${NC}" + echo -e "${YELLOW}Fix with:${NC} cargo fmt --all" + FAILED=1 +else + echo -e "${GREEN}OK${NC}" +fi + +# 2. Clippy +echo -n "Running clippy... " +if ! cargo clippy --all-targets -- -D warnings > /dev/null 2>&1; then + echo -e "${RED}FAILED${NC}" + echo -e "${YELLOW}Run:${NC} cargo clippy --all-targets" + FAILED=1 +else + echo -e "${GREEN}OK${NC}" +fi + +# 3. Tests +echo -n "Running tests... " +if ! cargo test --quiet > /dev/null 2>&1; then + echo -e "${RED}FAILED${NC}" + echo -e "${YELLOW}Run:${NC} cargo test" + FAILED=1 +else + echo -e "${GREEN}OK${NC}" +fi + +echo "" + +if [ $FAILED -ne 0 ]; then + echo -e "${RED}Pre-commit checks failed.${NC}" + echo "Bypass with: git commit --no-verify" + exit 1 +fi + +echo -e "${GREEN}All checks passed!${NC}" diff --git a/src/core/tokenizer.rs b/src/core/tokenizer.rs index 62ccfb1..f9be129 100644 --- a/src/core/tokenizer.rs +++ b/src/core/tokenizer.rs @@ -332,10 +332,7 @@ impl Tokenizer { /// Get cache statistics (hits would require additional tracking). pub fn cache_len(&self) -> usize { - self.chunk_cache - .lock() - .map(|c| c.len()) - .unwrap_or(0) + self.chunk_cache.lock().map(|c| c.len()).unwrap_or(0) } } diff --git a/src/lib.rs b/src/lib.rs index 3035710..a912d62 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -3,7 +3,9 @@ mod python; use pyo3::prelude::*; -pub use core::{Tokenizer, TokenizerError, CL100K_BASE_PATTERN, O200K_BASE_PATTERN, StreamingDecoder}; +pub use core::{ + StreamingDecoder, Tokenizer, TokenizerError, CL100K_BASE_PATTERN, O200K_BASE_PATTERN, +}; /// splintr - Fast Rust BPE tokenizer with Python bindings /// diff --git a/src/python/bindings.rs b/src/python/bindings.rs index eca10ba..a74dbcd 100644 --- a/src/python/bindings.rs +++ b/src/python/bindings.rs @@ -456,10 +456,10 @@ impl PyStreamingDecoder { continue; } - if std::str::from_utf8(&bytes[..check_len]).is_ok() { - if Self::could_be_incomplete_sequence(&bytes[check_len..]) { - return check_len; - } + if std::str::from_utf8(&bytes[..check_len]).is_ok() + && Self::could_be_incomplete_sequence(&bytes[check_len..]) + { + return check_len; } } diff --git a/src/python/mod.rs b/src/python/mod.rs index 08ae76c..f4e63bf 100644 --- a/src/python/mod.rs +++ b/src/python/mod.rs @@ -1,3 +1,3 @@ mod bindings; -pub use bindings::{PyTokenizer, PyStreamingDecoder}; +pub use bindings::{PyStreamingDecoder, PyTokenizer}; From 4ccc55daf8b0e9fe41bc9a6d9207a196228b3c86 Mon Sep 17 00:00:00 2001 From: Farhan Syah Date: Wed, 26 Nov 2025 09:14:31 +0800 Subject: [PATCH 4/5] fix(ci): restructure workflow and add PyO3 configuration for all platforms - Consolidate clippy and rustfmt into single lint job - Add PyO3 Python environment configuration for Ubuntu, macOS, and Windows - Configure LIBRARY_PATH on macOS to resolve linker errors with Python framework - Restore full Python tests with tiktoken comparison This resolves macOS build failures caused by missing Python framework linking. --- .github/workflows/ci.yml | 64 +++++++++++++++++++++++++++------------- 1 file changed, 44 insertions(+), 20 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index defcd0f..4b2baeb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -8,8 +8,30 @@ env: CARGO_TERM_COLOR: always jobs: + # Lint and format check (single job) + lint: + name: Lint & Format + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Install Rust + uses: dtolnay/rust-toolchain@stable + with: + components: rustfmt, clippy + + - name: Install PCRE2 + run: sudo apt-get update && sudo apt-get install -y libpcre2-dev + + - name: Check formatting + run: cargo fmt --all --check + + - name: Run clippy + run: cargo clippy --all-targets -- -D warnings + + # Rust tests on multiple platforms test: - name: Test on ${{ matrix.os }} + name: Test (${{ matrix.os }}) runs-on: ${{ matrix.os }} strategy: fail-fast: false @@ -31,37 +53,39 @@ jobs: if: matrix.os == 'ubuntu-latest' run: sudo apt-get update && sudo apt-get install -y libpcre2-dev + - name: Configure Python for PyO3 (Ubuntu) + if: matrix.os == 'ubuntu-latest' + run: echo "PYO3_PYTHON=$(which python3)" >> $GITHUB_ENV + - name: Install PCRE2 (macOS) if: matrix.os == 'macos-latest' run: brew install pcre2 + - name: Configure Python for PyO3 (macOS) + if: matrix.os == 'macos-latest' + run: | + echo "PYO3_PYTHON=$(which python3)" >> $GITHUB_ENV + # Get Python framework path for linking + PYTHON_PATH=$(python3 -c "import sys; print(sys.prefix)") + echo "LIBRARY_PATH=${PYTHON_PATH}/lib:${LIBRARY_PATH:-}" >> $GITHUB_ENV + - name: Install PCRE2 (Windows) if: matrix.os == 'windows-latest' run: | vcpkg install pcre2:x64-windows echo "PCRE2_SYS_STATIC=1" >> $env:GITHUB_ENV - - name: Run tests - run: cargo test --verbose - - - name: Run clippy - run: cargo clippy -- -D warnings - - fmt: - name: Format check - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - - name: Install Rust - uses: dtolnay/rust-toolchain@stable - with: - components: rustfmt, clippy + - name: Configure Python for PyO3 (Windows) + if: matrix.os == 'windows-latest' + run: | + $pythonPath = (Get-Command python).Source + echo "PYO3_PYTHON=$pythonPath" >> $env:GITHUB_ENV - - name: Check formatting - run: cargo fmt --check + - name: Run tests + run: cargo test - python-test: + # Python bindings tests + python: name: Python tests runs-on: ubuntu-latest steps: From f4199c33ff662b72ac23dc7c4462e9dd66cb1330 Mon Sep 17 00:00:00 2001 From: Farhan Syah Date: Wed, 26 Nov 2025 09:17:13 +0800 Subject: [PATCH 5/5] fix(ci): use dynamic_lookup linker flag for macOS PyO3 extensions Update macOS PyO3 configuration to use the standard -undefined dynamic_lookup linker flag for Python extension modules. Add DYLD_LIBRARY_PATH environment variable alongside LIBRARY_PATH for proper runtime library resolution. --- .github/workflows/ci.yml | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4b2baeb..7b672f5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -65,9 +65,12 @@ jobs: if: matrix.os == 'macos-latest' run: | echo "PYO3_PYTHON=$(which python3)" >> $GITHUB_ENV - # Get Python framework path for linking - PYTHON_PATH=$(python3 -c "import sys; print(sys.prefix)") - echo "LIBRARY_PATH=${PYTHON_PATH}/lib:${LIBRARY_PATH:-}" >> $GITHUB_ENV + # Get Python library directory and set for linker + PYTHON_PREFIX=$(python3 -c "import sys; print(sys.prefix)") + echo "LIBRARY_PATH=${PYTHON_PREFIX}/lib" >> $GITHUB_ENV + echo "DYLD_LIBRARY_PATH=${PYTHON_PREFIX}/lib" >> $GITHUB_ENV + # Tell Cargo to link the Python framework + echo "CARGO_BUILD_RUSTFLAGS=-C link-arg=-undefined -C link-arg=dynamic_lookup" >> $GITHUB_ENV - name: Install PCRE2 (Windows) if: matrix.os == 'windows-latest'