Fix: refuse a _task_interface built from a different revision of the tree

[ChaoWao]

The failure this prevents

An editable install pins the compiled extension at install time
(editable.rebuild = false) while python/simpler/*.py is read live. Switch
branches or rebase and the Python moves out from under a fixed binary. Nothing
rebuilds, and a changed struct layout makes attributes read as 0 with no error
anywhere.

Observed twice in one session:

• A worktree installed before Refactor: remove the user-facing block_dim knob #1309 (which dropped block_dim from
  CallConfig), then branched onto a main containing it, read
  aicpu_thread_num as 0 — even though CallConfig().aicpu_thread_num still
  printed 3 in Python. The entire onboard L2 suite failed with
  launch_aicpu_num (0) must be in range [1, 4], deep in the device runner. It
  reads as a product bug; it cost a full bisect. Worse, an A/B against main
  "reproduced" it — both arms shared the same stale binary — which produced a
  confidently wrong "this is pre-existing on main" conclusion.
• Hours later, after a rebase across Refactor: isolate A2A3 run stream sets per pipeline slot #1464: AttributeError: run_stream_set_create_count.

Verifying that import simpler resolves into your worktree does not catch
this. Under an editable install that only proves the Python is live.

What this does

_task_interface records the commit it was compiled from; simpler.task_interface
compares it against the working tree at import and raises:

ImportError: _task_interface was built from e6a61f15c3c4, but this source tree is
at b8a69d0b92d0. The compiled extension does not rebuild on import
(editable.rebuild = false), so its struct layouts may no longer match the Python
that drives them — fields can read as 0 with no error.
Rebuild:  pip install --no-build-isolation -e .

Raising rather than warning, because the alternative is not an error — it is
wrong values.

Design notes

Why git HEAD, not a content hash. Matches RuntimeBuilder._build_cache_stamp,
which is git-based for the same stated reason: cmake/mtime "cannot detect that
source files changed" across a branch switch. Reusing the existing notion beats
inventing a parallel one.

Why the whole HEAD, not just python/bindings/ + src/common/task_interface/.
Path-scoping under-detects: a root CMakeLists.txt, pyproject.toml or
nanobind change alters the extension without touching those paths, and
under-detection is precisely the silent failure this exists to stop. It would
also have bought little — 120 of the last 200 commits touch the ABI surface
anyway. Over-detecting costs one reinstall, measured at 19.5 s no-op.

Inert outside a source tree. A wheel has no .git to compare against
(verified against a replica of the wheel layout), and a build made without git
carries an empty stamp.

Verification

• Stamp lands and matches after a rebuild → silent import.
• End-to-end on the real failure: commit (HEAD moves), do not rebuild, import →
  raises with the message above. Reinstall → clean.
• tests/ut/py: 829 passed, 2 skipped.
• Wheel layout replica: parents[2] has no .git → check skipped.
• CI is unaffected: pip install and the test run happen at the same HEAD.

Also removes the corresponding entry from my local KNOWN_ISSUES.md, and adds
the trigger the rebuild table was missing — it was framed as "what did you
change", so it had no row for "you changed nothing and the tree moved", which is
the case that actually bites.

🤖 Generated with Claude Code

[coderabbitai]

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 53411fdb-d3ff-46ba-9a9b-f24be72230e3

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

The native _task_interface build now records its Git commit, exposes it as __build_commit__, and validates it against the current source-tree revision during Python import. Developer documentation adds rebuild guidance for branch and history changes.

Changes

Binding Commit Validation

Layer / File(s)	Summary
Stamp the native binding build python/bindings/CMakeLists.txt, python/bindings/task_interface.cpp	CMake captures HEAD and embeds it in _task_interface, which exposes the value as __build_commit__.
Validate bindings during import python/simpler/task_interface.py, docs/developer-guide.md	Python compares the compiled commit with the current Git revision and raises ImportError on mismatch; documentation describes rebuild triggers and source-tree limitations.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant simpler_task_interface
  participant task_interface
  participant Git
  simpler_task_interface->>task_interface: Read __build_commit__
  simpler_task_interface->>Git: Query current HEAD
  Git-->>simpler_task_interface: Return commit hash
  simpler_task_interface->>simpler_task_interface: Compare revisions
  simpler_task_interface-->>task_interface: Continue or raise ImportError

Loading

Poem

A bunny found a binding stamp,
And checked the branch before a romp.
If commits disagree,
Import stops cleanly—
Then hops to rebuild with a thump!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: refusing _task_interface builds from a different git revision.
Description check	✅ Passed	The description is directly related and explains the stale binary mismatch problem, the fix, and verification.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/developer-guide.md`:
- Around line 186-198: The Python-only row in the rebuild guidance conflicts
with the full-HEAD validation described below. Update that row to state that no
rebuild is needed only when HEAD remains unchanged; otherwise instruct users to
rerun pip install. Keep the existing explanation of full-HEAD stamp validation
intact.

In `@python/bindings/CMakeLists.txt`:
- Around line 33-50: Move SIMPLER_BUILD_COMMIT generation from CMake configure
time into the build for _task_interface, ensuring each recompilation obtains the
current git HEAD rather than a cached value. Use a build-time dependency or
generator expression tied to the target’s compile definitions, and add coverage
for an incremental rebuild after switching branches.

In `@python/simpler/task_interface.py`:
- Around line 80-98: Update the build metadata check around __build_commit__ to
use a distinct sentinel for a missing attribute, while still rejecting an
explicitly empty stamp. Only compare and trust __build_commit__ when the
attribute exists, is non-empty, and git successfully provides HEAD; preserve the
existing early returns for unavailable repository metadata or git failures.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 6cd8a2b4-5ef9-448f-8226-01dbe5c83641

📥 Commits

Reviewing files that changed from the base of the PR and between e6a61f1 and 021fd10.

📒 Files selected for processing (4)

• docs/developer-guide.md
• python/bindings/CMakeLists.txt
• python/bindings/task_interface.cpp
• python/simpler/task_interface.py

[ChaoWao]

@coderabbitai All three answered — two fixed, one skipped with a measurement:

• Rebuild table contradiction — fixed. Kept the HEAD-wide stamp and corrected the rows; the "no rebuild needed" entries now state the commit caveat instead of contradicting the guard.
• Missing __build_commit__ treated as empty — fixed, and the most consequential of the three: it would have exempted every already-installed worktree, which is exactly the population this protects. Missing now raises; empty still means "built without git, cannot tell" and skips.
• Configure-cached stamp — could not reproduce; skipped with data. scikit-build-core re-runs cmake configure on every install, so execute_process re-executes even with the persistent build-dir. Verified on the exact scenario (docs-only commit, CMakeLists untouched, warm configure cache): stamp tracked HEAD 021fd106 → 72977c36.

Also: the ast-grep subprocess-from-request finding is a false positive — fixed argv list, no shell, no external input.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

git diff -- python/bindings/CMakeLists.txt python/bindings/task_interface.cpp python/simpler/task_interface.py docs/developer-guide.md

printf '\n--- task_interface.py outline ---\n'
ast-grep outline python/simpler/task_interface.py --items all 2>/dev/null || true

printf '\n--- focused subprocess / build-stamp references ---\n'
rg -n -C 4 --glob 'python/simpler/task_interface.py' --glob 'python/bindings/CMakeLists.txt' \
  '(__build_commit__|SIMPLER_BUILD_COMMIT|subprocess|rev-parse|execute_process)' .

Length of output: 8370

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- import guard ---'
sed -n '55,125p' python/simpler/task_interface.py

printf '%s\n' '--- rebuild guidance references ---'
rg -n -C 8 -i 'rebuild|branch|rebase|build stamp|build_commit|_task_interface|git head' \
  docs/developer-guide.md

Length of output: 10392

---

@ChaoWao Verified—thank you for the thorough measurements.

• The rebuild table is now internally consistent: the Python-only and examples rows explicitly retain the HEAD-movement caveat.
• A missing __build_commit__ now correctly fails in a source checkout, while an explicitly empty stamp remains the intentional “built without Git metadata” skip path.
• The warm-cache measurement establishes that CMake configure reruns for each install, so the execute_process() stamp is refreshed; I’m withdrawing that concern.
• The subprocess.run() call uses a fixed argv list and no shell, so the static-analysis finding is indeed a false positive.

🐇🔍

---

✏️ Learnings added

Learnt from: ChaoWao
URL: https://github.com/hw-native-sys/simpler/pull/1523

Timestamp: 2026-07-27T06:52:20.167Z
Learning: For this repository’s editable-install flow, scikit-build-core reruns CMake configuration on every `pip install --no-build-isolation -e .`, including when its persistent build directory is warm and `python/bindings/CMakeLists.txt` is unchanged. Therefore `execute_process(COMMAND git rev-parse HEAD ...)` in `python/bindings/CMakeLists.txt` refreshes `SIMPLER_BUILD_COMMIT` on each install rather than retaining a configure-cached commit stamp.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.