Adopt TapHouse rules and the tap::python namespace#1
Merged
Conversation
Bring the renamed PythonTap (was Python-Max) under the shared Tap House Rules and the family C++ namespace convention. - Sync the four canonical TapHouse configs (.clang-format, .clang-tidy, STYLE.md, .pre-commit-config.yaml) at v4, and add scripts/tidy.sh (the local clang-tidy gate mirror). - Namespace: tap::python_runtime -> tap::python (the runtime alias and its uses are unchanged; the Min external class `python` stays global, per the family's Min-object convention). - Apply the house layout with clang-format, and add braces to every control-flow body (the mandatory-braces rule; clang-format does not insert them, so this was a one-time InsertBraces pass, verified complete). Naming already matched the house scheme (m_ members, k_/s_ prefixes, snake_case). - Add .github/workflows/style.yml: the TapHouse drift check (v4) and a clang-format gate on Linux, plus the clang-tidy naming + braces gate on macOS — tap.python~ only configures with the embedded Python runtime, which installs on macOS/Windows only, so Linux cannot produce the compile database. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRTDgYKY38WH8Bqz17W9k1
The canonical TapHouse .clang-format sets SortIncludes, which alphabetized the include block in tap.python_tilde.h and moved the attribute/message headers ahead of cglue.h. Those headers use cglue.h's C trampoline declarations (python_attr_get/set, python_mess_*) but did not include it — they had relied on the previous hand-ordered includes — so the reorder broke the macOS/Windows builds (and the clang-tidy gate, which compiles the same TU). Make the headers depend on what they use, so include order no longer matters: - tap.python_tilde_cglue.h: include c74_min_api.h (it uses c74::max types). - tap.python_tilde_attribute.h / _message.h: include the cglue header. Verified: tap.python_tilde.cpp now syntax-compiles clean (0 errors) against min-api + Python headers; the pre-fix attribute.h reproduced exactly the two undeclared-identifier errors from CI. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRTDgYKY38WH8Bqz17W9k1
Two issues surfaced once the build was green and clang-tidy actually ran:
1. The package defaults to a universal macOS build (arm64;x86_64); a universal
compile command carries two -arch flags, so clang-tidy sees two compiler
jobs and bails ("expected exactly one compiler job"). Configure the
compile-database build single-arch (-DCMAKE_OSX_ARCHITECTURES=arm64) — the
arch is set conditionally in CMakeLists, so the override takes.
2. The clang-tidy invocation used the canonical .clang-tidy HeaderFilterRegex
(include|tests), which matches the min-api submodule's include/ and flooded
the run with min-api's template-parameter naming. Pass
-header-filter='.*/source/projects/.*' (as the other Max repos do) to scope
diagnostics to this package's own sources.
That header-filter also newly lints the package's own headers, which caught two
unbraced if-statements in tap.python_tilde_runtime.h that clang-format's
InsertBraces had skipped (both sit right after an #endif — a known InsertBraces
limitation across preprocessor boundaries). Braced them.
Verified locally with clang-tidy-18: both TUs clean under --warnings-as-errors=
readability-* with the source/projects header filter.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MRTDgYKY38WH8Bqz17W9k1
The previous -header-filter='.*/source/projects/.*' also matched the bundled CPython headers: the build includes them via a path that passes back through source/projects (…/tap.python_tilde/../../../support/include/python3.13/…), so the filter linted all of CPython and failed on its naming. (This slipped past local checks because Ubuntu's clang-tidy canonicalizes the include path, hiding the source/projects segment; the macOS CI clang-tidy keeps the literal path.) Narrow the filter to '.*/tap\.python_tilde/tap\.python_tilde.*', which matches only this object's own tap.python_tilde_*.h headers and excludes both the CPython support tree and min-api. Verified against the exact CI path strings. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRTDgYKY38WH8Bqz17W9k1
With the header filter fixed, clang-tidy stopped flooding on CPython and hit a real toolchain wall: llvm@18's clang-tidy cannot parse the macOS 26 SDK's libc++ (<bitset>/<charconv>/<algorithm> use __builtin_ctzg / __builtin_clzg, generic bit-count builtins added in Clang 19). The TU failed to compile, so clang-tidy reported clang-diagnostic-errors and the gate failed. Install llvm@20 for the macOS clang-tidy leg instead. Behavior is unchanged: the canonical .clang-tidy enables only readability-identifier-naming and readability-braces-around-statements, both config-driven and stable across clang-tidy versions; the newer toolchain just understands the current SDK. (This leg only exists on macOS because tap.python~ needs the embedded Python runtime to configure; the Linux-based Max repos keep clang-tidy-18.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRTDgYKY38WH8Bqz17W9k1
Chasing clang-tidy across macOS toolchains was a dead end: llvm@18 couldn't parse the macOS 26 SDK's libc++ (__builtin_ctzg), and llvm@20 couldn't find the SDK's headers at all. The lint only ran on macOS because tap.python~ *configures* with the bundled Python runtime (macOS/Windows only) — but linting needs only headers, not the runtime. Move the clang-tidy job to ubuntu-latest with clang-tidy-18 (matching every other Max repo) and invoke it directly instead of via a compile database: compile the two TUs against min-api and the system Python headers (python3-dev). This removes the entire macOS-SDK-vs-standalone-clang-tidy problem, and — since it's the same Ubuntu + clang-tidy-18 locally and in CI — it is verifiable before pushing (both TUs are clean under --warnings-as-errors=readability-* with the own-headers filter). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRTDgYKY38WH8Bqz17W9k1
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Brings the renamed PythonTap (was Python-Max) under the shared Tap House Rules and the family C++ namespace convention.
Changes
.clang-format,.clang-tidy,STYLE.md,.pre-commit-config.yaml) at v4, and addscripts/tidy.sh(the local clang-tidy gate mirror). All byte-match canonical, so the drift check passes.tap::python_runtime→tap::python(theruntimealias and its uses are unchanged; the Min external classpythonstays global, per the family's Min-object convention).InsertBracespass, verified complete). Naming already matched the house scheme (m_members,k_/s_prefixes, snake_case)..github/workflows/style.yml— the TapHouse drift check (v4) and a clang-format gate on Linux, plus the clang-tidy naming + braces gate on macOS.Note on the clang-tidy gate
tap.python~only configures with the embedded Python runtime, which installs on macOS/Windows only — so Linux cannot produce a compile database, and the clang-tidy gate runs on macOS. That's also why this leg couldn't be run in the authoring environment: clang-format cleanliness and brace completeness were verified locally, but the macOS clang-tidy job itself is unobserved — worth a glance on the first CI run.🤖 Generated with Claude Code
https://claude.ai/code/session_01MRTDgYKY38WH8Bqz17W9k1
Generated by Claude Code