AI review

From Kimi-K2.6: :robot: Here's my comprehensive review of the repository, organized by category: --- ## 🔴 Bugs / Logic Errors | File | Issue | Details | |------|-------|---------| | `src/scikit_build_core/build/wheel.py:216` | `.format()` on user-controlled string | `settings_reader.settings.messages.after_failure.format()` — if the message contains `{` or `}`, it raises `KeyError`/`IndexError`, masking the original error. **Fix:** remove `.format()` | | `src/scikit_build_core/builder/builder.py:234,241` | Missing format argument in logger | `logger.info("py-api {} requires CPython, ignoring")` — `{}` has no argument; raises `IndexError` on PyPy. **Fix:** pass `py_api` | | `src/scikit_build_core/cmake.py:90` | Mutable class-level default | `single_config: bool = not sysconfig.get_platform().startswith("win")` evaluated once at import time; stale after mocks. **Fix:** use `field(init=False)` + `__post_init__` | | `src/scikit_build_core/settings/skbuild_read_settings.py:220-222` | Unsafe dict access | `pyproject["build-system"]["requires"]` crashes with `KeyError` if `[build-system]` is missing. **Fix:** use `.get("build-system")` with helpful error | | `src/scikit_build_core/settings/sources.py:772,785` | Overly broad `except Exception` | Catches `AttributeError` etc. from programming bugs. **Fix:** narrow to `TypeError, ValueError` | --- ## 🟠 Performance Issues | File | Issue | Recommendation | |------|-------|---------------| | `src/scikit_build_core/build/wheel.py:410-468` | Dead `build_args` variable | Always empty list; remove it | | `src/scikit_build_core/builder/builder.py:132-145` | Entry points loaded 3× per configure | Cache `_get_entry_point_search_path` with `@lru_cache` | | `src/scikit_build_core/builder/builder.py:326-328` | `get_archs` recomputed | Already computed in `wheel.py`; pass the result instead | | `src/scikit_build_core/program_search.py:41-59` | `_macos_binary_is_x86` subprocess overhead | Cache with canonicalized `Path` | | `src/scikit_build_core/program_search.py:129-173` | Duplicate subprocess for CMake version | Consider defaulting to `--version` if JSON path often fails | | `src/scikit_build_core/program_search.py:186-212` | Ninja version probe uncached | Add `@lru_cache(None)` | | `src/scikit_build_core/settings/skbuild_read_settings.py:262-264` | Settings conversion runs twice | Reuse intermediate TOML-derived object | --- ## 🟡 Simplifications & Best Practices | File | Issue | Recommendation | |------|-------|---------------| | `pyproject.toml:48-50` | Empty `[project.optional-dependencies]` `pyproject` | **Remove the empty group** | | `pyproject.toml:143-148` | `dev` group redundantly includes `test` | Since `cov` → `test` → `test-core`, remove `{ include-group = "test" }` from `dev` | | `src/scikit_build_core/cmake.py` | 8× repeated `.replace("\\", "/")` | Extract `_cmake_path_str()` helper | | `src/scikit_build_core/builder/builder.py:66,289-292` | Inconsistent platform detection | Standardize on `sys.platform.startswith("win")` | | `src/scikit_build_core/program_search.py:215-224` | `get_make_programs` not in `__all__` | Add it or prefix with `_` | | `src/scikit_build_core/build/sdist.py:115` | `copy.deepcopy(metadata)` workaround | Check if upstream bug (python-pyproject-metadata#49) is fixed; remove if so | --- ## 🟢 CI / Packaging Best Practices (from sp-repo-review) **Passed all checks** — sp-repo-review reports zero failures. The repo is well-configured for Scientific Python standards. Minor observations: | Observation | Note | |-------------|------| | `FORCE_COLOR` | Already set in workflows | | `uv` backend | Nox correctly uses `uv\|virtualenv` | | Dependency groups | Properly structured (just redundant `test` in `dev`) | | Pre-commit | `prek` is used; good |