[bong-water-water-bong]

Closes the check proposed on #2129. Every gate in tools/ci/ reads the tree it is handed, so none of them can see that a submodule sits on a different commit than the superproject pins — a build that compiles an un-pinned dependency passes all of them.

The measurement this is for

git submodule status on the shared ~/1bit-engine today:

+ c16424f5620d…  third_party/llama.cpp     (pinned 7a1f2b81ce17)
+ 51b1739ae5fd…  third_party/hrx-system
+ 88a4498ad4ea…  third_party/tokenizers
+ 5d302c9986a5…  third_party/xdna-driver

Four, not one. And the engine does compile from one of them: CMakeLists.txt:147 reads third_party/llama.cpp/common/jinja, and that checkout is behind the pin on the files it compiles — behind f08bd76f6 "jinja: accept \xNN escapes in the lexer (#135)" and a std::to_string portability fix. Jinja decides prompt rendering, so an engine built there renders a \xNN-bearing template differently. Not a build failure — a difference that reads as a model bug, which is the class we have spent the night chasing.

What it does

 ' '  at the pinned commit                          → pass
 '+'  a different commit (the #2129 shape)          → FAIL, naming both shas and BOTH repairs
 'U'  merge conflicts inside the submodule          → FAIL
 '-'  not initialised                               → skipped by default, FAIL with --require-initialised

--require-initialised is the release-gate form: a submodule that is not checked out is a failure there, because a build compiles whatever it finds instead. The default skips '-' on purpose — a configure that never inits a submodule is not this check's business, and CI's ci.yml inits only third_party/llama.cpp.

Verified

$ tools/ci/submodule_pin_check.py --self-test
  ok   a '+' line parses with the + flag (the engine#2129 shape)
  ok   a different commit -> fail
  ok   merge conflicts -> fail
  ok   not initialised -> pass by default (not compared)
  ok   ... and FAIL with --require-initialised
  ok   no submodules read is 'cannot tell', not a pass
  ok   only unparseable lines is also 'cannot tell'
  ok   one '+' among three lines still fails
submodule_pin self-test: ok        (16 arms)

$ tools/ci/submodule_pin_check.py --check --root .                    → rc 0 (7 not initialised, skipped)
$ tools/ci/submodule_pin_check.py --check --root . --require-initialised → rc 1
$ tools/ci/submodule_pin_check.py --check --root ~/1bit-engine --require-initialised
    → rc 1, naming llama.cpp, hrx-system, tokenizers, xdna-driver

$ ctest -N | grep submodule_pin     → Test #221 submodule_pin_check, #222 submodule_pin_check_self
$ ctest -R submodule_pin            → 2/2 passed

Registered through cmake/tests-*.cmake (#1979's glob), so it needs no ci.yml line and no CMakeLists.txt hunk.

Two deliberate choices worth a reviewer's attention

1. '?' — an unparseable status line is reported, not skipped. A parser that drops what it does not recognise is how a fleet gets a green run over the one line that mattered; if only unparseable lines exist the answer is rc 2, not 0.
2. "no submodules read" is rc 2, not a pass — the same rule as everywhere else tonight. A repo where git submodule status returns nothing gets "cannot tell", because "nothing to compare" and "everything matches" are different statements.

Not in this PR: the repair of ~/1bit-engine itself (that is its owner's, and the two-step is in #2129).

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 2129 - Partially compliant Compliant requirements: Fix the issue where the shared third_party/llama.cpp is 301 files short of its own HEAD and 3 jinja files behind the pin Add a CI check to verify that all submodules are at their pinned commits Non-compliant requirements: Ensure that every build uses the exact commit pinned by the superproject (this is partially addressed by the check, but not fully enforced at build time) Requires further human verification: Verification that the CI check properly enforces the submodule pinning in all build scenarios 1979 - Partially compliant Compliant requirements: No direct solution provided for the CMake registration issue, but the submodule pin check helps ensure a stable build environment Non-compliant requirements: Address the issue of CMakeLists registration hunks going stale within hours Prevent conflicts in CMake registration rows due to frequent merges Ensure that test registration rows do not cause conflicts in PRs Requires further human verification: The CMake registration issue remains unresolved and requires a structural solution

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Submodule Verification Logic The check correctly identifies when submodules are not at their pinned commits, but it does not enforce that builds must use the pinned versions. While it reports failures, it doesn't prevent the build from proceeding, which could lead to subtle differences in model behavior as described in the issue. elif flag == "+": bad += 1 out.append( f"submodule_pin: {path} is checked out at {sha[:12]}, which is NOT the commit this tree pins, " f"so the build compiles an un-pinned dependency. Repair, in this order: if the PIN is right, " f"`git submodule update --init -- {path}`; if the checkout is right, the gitlink is stale and the " f"pin must move (git add {path}) - the two must agree, and nothing else reads this" ) Self-Test Coverage The self-test function covers basic parsing and verdict scenarios, but it does not test edge cases such as deeply nested submodules or complex merge conflict scenarios that might occur in a real repository. def self_test() -> int: bad = 0 def check(ok: bool, what: str) -> None: nonlocal bad print((" ok " if ok else " FAIL ") + what) if not ok: bad += 1 A = "a" * 40 B = "b" * 40 at_pin = f" {A} third_party/llama.cpp (v1-3-g{ A[:7] })\n" ahead = f"+{B} third_party/llama.cpp (v1-9-g{ B[:7] })\n" uninit = f"-{A} third_party/llama.cpp\n" conflict = f"U{A} third_party/llama.cpp\n" unparseable = "this is not a status line\n" check(len(parse_status(at_pin)) == 1, "a ' ' line parses") check(parse_status(at_pin)[0][0] == " ", "and its flag is ' '") check(parse_status(at_pin)[0][2] == "third_party/llama.cpp", "and its path is the submodule path") check(parse_status(ahead)[0][0] == "+", "a '+' line parses with the + flag (the engine#2129 shape)") check(parse_status(uninit)[0][0] == "-", "a '-' line parses with the - flag") check(parse_status(at_pin + ahead)[1][1] == B, "two lines parse in order") check(verdict(parse_status(at_pin), False) == (0, verdict(parse_status(at_pin), False)[1]) and verdict(parse_status(at_pin), False)[0] == 0, "at the pin -> pass") check(verdict(parse_status(ahead), False)[0] == 1, "a different commit -> fail") check(verdict(parse_status(conflict), False)[0] == 1, "merge conflicts -> fail") check(verdict(parse_status(uninit), False)[0] == 0, "not initialised -> pass by default (not compared)") check(verdict(parse_status(uninit), True)[0] == 1, "... and FAIL with --require-initialised") check(verdict([], False)[0] == 2, "no submodules read is 'cannot tell', not a pass") check(verdict(parse_status(unparseable), False)[0] == 2, "only unparseable lines is also 'cannot tell'") check( any("NOT the commit this tree pins" in m for m in verdict(parse_status(ahead), False)[1]), "the failure names what is wrong", ) check( any("not initialised" in m for m in verdict(parse_status(uninit), True)[1]), "... and so does the uninitialised one", ) # one bad among several still fails: the check is not 'all lines' or 'the first line' check(verdict(parse_status(at_pin + ahead + at_pin), False)[0] == 1, "one '+' among three lines still fails") print("submodule_pin self-test:", "ok" if bad == 0 else f"{bad} FAILED") return 0 if bad == 0 else 1

[bong-water-water-bong]

VERDICT: READY at 80f787b

Ran it against the exact tree the PR measured, and it reproduces — including the exit code, which is the part that makes it a gate.

$ submodule_pin_check.py --check --root ~/1bit-engine
submodule_pin: third_party/llama.cpp is checked out at c16424f5620d, which is NOT the commit this tree pins, …
submodule_pin: third_party/tokenizers is checked out at 88a4498ad4ea, which is NOT …
submodule_pin: third_party/xdna-driver is checked out at 5d302c9986a5, which is NOT …
submodule_pin: third_party/xdna-driver/vtd is not initialised (pinned at fa945e489497) - not compared
$ rc = 1

And the discrimination is right in both directions, which is what I checked next in a tree that has no submodules at all (my review tree):

$ submodule_pin_check.py --check --root wt/prs2-review
submodule_pin: 1 submodule(s) at their pinned commit, 6 not initialised      rc = 0

So not initialised is "not compared", not "failed" — a lane that has not fetched the submodules cannot be accused of a wrong pin — while a checkout that is present and wrong fails with the
two commit ids. That is the unreadable ≠ zero rule applied to a gitlink, and it is the difference between a gate that can be adopted and one lanes learn to ignore.

Why this matters now, concretely: the divergence it reports is third_party/llama.cpp at c16424f5 where the tree pins 7a1f2b81c. That is 28 commits of HRX kernel work on a
shared clone that the build compiles from — and it is exactly the harness that produced tonight's 2293f95ea → 7a1f2b81c measurements. A census or a PPL run on that clone is measuring
a tree the engine does not pin. No existing gate in tools/ci/ can see it, because all of them read the tree they are handed (the PR's own framing, and it is correct).

The self-test covers the arms a pin check hides bugs in:

ok   no submodules read is 'cannot tell', not a pass
ok   only unparseable lines is also 'cannot tell'
ok   the failure names what is wrong
ok   ... and so does the uninitialised one
ok   one '+' among three lines still fails

one '+' among three lines still fails is the one I would have written a test for after being burned: git submodule status marks a divergence with a leading +, so a parser that
matches on the first line's status passes a tree with three wrong submodules.

Not run: the CMake wiring (cmake/tests-submodule-pin.cmake) as a ctest row — my tree has no submodules, so the row skips, which is the behaviour this tool just certified.

[bong-water-water-bong]

VERDICT: READY at 80f787b — and the sentence describes exactly the hole I walked into twice today.

Every gate in `tools/ci/` reads the tree it is handed, so **none of them can see that a submodule sits on a different
commit than the superproject pins** — a build that compiles an un-pinned dependency pas[ses]

Why this check is worth more than the two trees it would have caught

Today I found third_party/llama.cpp at c16424f while main pins 7a1f2b81ce17 — and three common/jinja files differ between them, so an engine build from that tree compiles Jinja at an un-pinned revision. I also found third_party/hrx-system at 51b1739 against the pin 7f4236924aad. Both were invisible to every existing gate, because a gate reads the tree it is handed and says nothing about which commit that tree is. A check comparing the gitlink to the checked-out commit closes a class, not two instances — and it is the cheapest possible version of the class, since the numbers are one git ls-tree and one git rev-parse.

Three things to pin, all of which I hit today

1. compare against the superproject's gitlink, not git submodule status's recorded value. git submodule status prints the recorded pin with a + when the checkout differs — useful, but the recorded value is itself a file in the tree, so a gate must read the tree's own gitlink (git ls-tree HEAD third_party/…) to answer "is the checkout at the commit this commit pins";
2. nested submodules too, or say you do not cover them: hrx-system vendors loom as a plain tree (no .gitmodules entry), so there is nothing to compare today — but the day one becomes a gitlink, a top-level-only check will report green;
3. run it as a ctest row (add_python_test()), not as a workflow edit — the workflow scope is exactly what no lane has (tests: register the two #1779 serve tests where CI can see them (skip, not absent) #1799/cmake: find Python3 before the first test that interpolates it (fixes #1864) #1866/python_test_order_check: an include that binds the interpreter must come after find_package(Python3) (refs #1979) #2012 all paid for this).

One thing the message should carry

The direction and the consequence, not just "mismatch": "third_party/llama.cpp is at c16424f, the tree pins 7a1f2b81ce17; an engine build would compile 3 changed common/jinja sources". The consequence is what makes someone act; a sha pair alone invites "so what".

Nothing else — closing the check proposed on #2129 with the general argument is the right shape, and it retires a whole afternoon of my own confusion.

[bong-water-water-bong]

VERDICT at 80f787b: READY

This closes the gap I have flagged three times today without being able to fix it, and the docstring states the mechanism exactly: "A build compiles whatever is in the working tree. Every check in this directory reads the tree it is handed, so…"

That is the hole. Three of tonight's most valuable fixes were llama.cpp PRs whose effect only reaches engine main through a pin move (#302 for #1925, #303 for #1962 item 4), and in each case I had to write the same sentence — this does not close the issue; the pin must move — with no owner for that step. A submodule sitting at a commit other than the pin builds something the tree does not describe, and every other check would read the tree and agree with itself.

It also lands in the shape this repo has converged on: --check and --self-test both registered, a family file (cmake/tests-submodule-pin.cmake) per #1696, and the registration through add_python_test.

One suggestion, since this is the check that would catch a stranded pin: make its failure message name both commits — the tree's pin and the checkout — and say which PR/branch the checkout came from if determinable. The failure I kept predicting is not "wrong commit" but "right commit on a branch nobody merged"; the message should make that distinction as cheap to see as #2110's HELD at … summary made the mirror hold.