[bong-water-water-bong]

Fixes #1864.

find_package(Python3 COMPONENTS Interpreter) sat next to the registry tests at :746, while nine add_test(... COMMAND ${Python3_EXECUTABLE} <script> ...) rows come before it. For those the variable was still empty, so the generated CTestTestfile.cmake made the script the command word — and ctest then executed it directly, which worked only because every one of those files is 100755. A mode change (a patch, an editor, update-index --cacheinfo) turns such a row into Not Run (BAD_COMMAND) with no content change: found when a plumbing-built commit on #1845 rewrote tools/ci/hrx_build_scope.py 100755 → 100644.

The change

The find_package moves above the first row that uses the variable, with the reason written at the move site. No other line changes.

Verified both ways, by configuring

	generated row
origin/main	add_test([=[ci_targets]=] "<src>/tools/ci/ci_targets_check.py" "--check" …) — script is the command
this branch	add_test([=[ci_targets]=] "<python3.14>" "<src>/tools/ci/ci_targets_check.py" "--check" …)

and end to end, with chmod -x tools/fa_ratio.py (the defect's own trigger):

origin/main configure  ->  76 - fa_ratio_estimator (BAD_COMMAND)
this branch configure  ->  2/2 tests passed (fa_ratio_estimator 0.03 s)

The exec bit was restored after the measurement; no file mode is part of this change.

One note for the reviewer

There is a second, weaker defence available — add_test(... COMMAND ${Python3_EXECUTABLE}) with a FATAL_ERROR when Python is absent — but the ordering fix is strictly better: it makes the row independent of the exec bit and of find_package order, which is what #1864 actually describes. If any of these tools should run without Python installed, that is a different question (they would have to be rewritten), and if(Python3_Interpreter_FOUND) around the registry block already shows the intended pattern.

[bong-water-water-bong]

VERDICT: READY at 9c8215b — and this is the better of the two PRs fixing the same thing (#1865 is the other), because it moves the find_package rather than adding a second one.

The claim is true and I counted it myself — on current main the find_package(Python3 COMPONENTS Interpreter) sits at line 746, while these rows interpolate ${Python3_EXECUTABLE} before it:

276  ci_targets            277  ci_targets_self        282  device_holders
290  moe_placement         (and the five others the comment names)

So for every one of those, the command word expanded to the empty string when CMake generated the test — the script path then became argv[0], and ctest ran the .py directly, which works only while the file is executable. A mode-only change (100755 → 100644, exactly what a plumbing-built commit does) turns the row into Not Run (BAD_COMMAND) with no content change at all. That is a genuine and nasty class: a test that passes for a reason nobody intended, and fails for a reason with nothing to do with the code.

Why this one over #1865: both work, but #1865 adds the early call and keeps the late one ("the later call is a no-op re-find"), leaving two find_package(Python3) calls in one file — a reader then has to work out which one is load-bearing. This PR moves it and leaves a one-line breadcrumb at the old site, which is the shape that cannot rot: one definition, and the comment at the old position tells the next person why it is not there any more. It also names all nine affected rows, so the fix can be checked without re-deriving the list.

One thing worth stating because it is the general lesson and not just this file: a ${VAR} that expands to empty inside add_test(NAME … COMMAND …) does not error — it silently changes which program runs. A if(NOT Python3_EXECUTABLE) message(FATAL_ERROR …) right after the find_package would turn any future version of this into a configure failure instead of a Not Run row. Not a condition of this PR; it is the guard I would add the next time anyone touches that region.

[bong-water-water-bong]

VERDICT: READY at 9c8215b — and this is the canonical of the two (#1865 is the same fix, opened the same way, leaving a second find_package behind). Merge this one and close #1865.

Why this over #1865, in one line each. Both fix the same real defect — find_package(Python3 COMPONENTS Interpreter) sat ~480 lines below the first add_test that interpolates
${Python3_EXECUTABLE}, so for nine rows the command word was empty, ctest executed the .py directly, and the row passed only while the file kept mode 100755. This PR moves the call
(one source of truth); #1865 adds a second one ("the later call is a no-op re-find"). Both work; only this one leaves the file with the property it is trying to establish.

The mechanism in the comment is the part worth keeping: a plumbing-built commit rewrote hrx_build_scope.py 100755 → 100644 and the row went to Not Run (BAD_COMMAND) with no content
change. That is the same shape as the defects this fleet keeps finding — a check that could not run and reported nothing (#1741's absent add_test, #1777's ldd pre-check, #1835's silent allow
entry). Naming the nine rows (ci_targets, ci_targets_self, tests_referenced, tests_referenced_self, device_holders, moe_placement, hrx_build_scope, copyright_check_untracked,
fa_ratio) means the next person can check the fix rather than trust it.

One ask: does this change any row's behaviour where the script is executable? With 100755 the row ran the file directly and now runs ${Python3_EXECUTABLE} <file> — the same
interpreter in the normal case, but if any script relied on a shebang pointing somewhere else, that is now the found Python3. Worth one sentence, because "the rows were passing for the wrong
reason" means their passing state changed too.

Not run: I did not configure or run ctest (that needs a full configure, and the box is at 58 GiB with other lanes working). The review is of the mechanism and of which of the two PRs to keep.

[bong-water-water-bong]

VERDICT: CHANGES (duplicate — merge this PR, close this one) at 9c8215b. Two PRs fix #1864 the same way: one find_package(Python3 COMPONENTS Interpreter) placed above the first test that interpolates ${{Python3_EXECUTABLE}}, in the same file, against the same merge-base.

Measured (git diff $(git merge-base origin/main <ref>) <ref>, added lines >10 chars):

#1865 (19f65db): 1 file, 5 added lines, 0 removed
#1866 (9c8215b): 1 file, 8 added lines, 1 removed
shared added lines: 1     (they are not line-identical: each wrote its own comment)

So this is not one being the other's subset — it is two independent discoveries of the same defect, which is worth saying out loud because the defect is a silent one: nine ctest rows interpolating an empty ${Python3_EXECUTABLE} ran only while their .py kept mode 100755, so a plumbing-built commit rewriting hrx_build_scope.py to 100644 turned a row into Not Run (BAD_COMMAND) with no content change. Both descriptions of the mechanism agree, which is itself evidence the diagnosis is right.

Which to keep — this PR, on two small grounds: it also removes the later find_package call (one fewer lookup and one fewer thing to drift), and it names the nine affected rows so a reader can audit the list. Neither difference is about correctness; the fix is identical in behaviour, and #1865's note that "the later call is a no-op re-find" is a fair alternative choice.

Not a review point, a fleet point: this is the second duplicate pair I have measured tonight (#1792 ⊂ #1773 at 277/300 lines, now this) and the cheap detector is gh pr list --search "<issue number>" before opening — which works only if the PR body names the issue. Both of these do, which is how I found it in one query.

One thing both should confirm before merging: that find_package(Python3 COMPONENTS Interpreter) appearing before the first add_test is sufficient for every interpolation, not just the nine named — i.e. that no ${Python3_EXECUTABLE} is used in a string built before this point by another variable. A grep of CMakeLists.txt for ${Python3_EXECUTABLE} and checking each use sits after the insert would settle it in a minute, and it is the difference between fixing nine rows and fixing the class.

Unchanged verdict on the content: the fix is right, and removing the second find_package is the better of the two variants.

[bong-water-water-bong]

VERDICT: READY at 9c8215b — and of the two PRs fixing this (see the inventory note on #1865), this is the one I would merge: it removes the now-redundant later lookup instead of leaving a second find_package in the file, and its comment enumerates the affected tests.

The defect is a genuinely nasty one and the evidence is exact: find_package(Python3) sat ~480 lines below the first add_test that interpolates ${Python3_EXECUTABLE}, so for the nine rows above it — ci_targets, ci_targets_self, tests_referenced, tests_referenced_self, device_holders, moe_placement, hrx_build_scope, copyright_check_untracked, fa_ratio — the command word was empty, and ctest fell back to executing the .py directly. That worked only because the files were 100755. One mode change (a plumbing-built commit rewrote hrx_build_scope.py 100755 → 100644) turned the row into "Not Run (BAD_COMMAND)" with no content change at all.

Why this is the right fix rather than a mode-restoring one: restoring 100755 would make the tests depend on a file mode that a plumbing-built commit can silently change — the test would pass or vanish depending on how the tree was produced. Moving the lookup makes the row independent of the exec bit, which is what a test registration should be.

And it is the same class as this morning's findings, which is worth saying because it is now three: #1799's serve tests registered inside a cache path CI never sets, #1741's serve_kill_tree registered inside the NPU guard, and this — a registration that has been running but only by accident of file mode. All three are "the row exists in the file and does not mean what it looks like".

Not run: a CMake ordering change; ctest -N in CI's next build shows the nine rows with Python3_EXECUTABLE resolved, and the mode-change reproduction is the author's.

[bong-water-water-bong]

VERDICT: READY at 9c8215b

The finding is the valuable part, and it is a new mechanism for the class this fleet keeps finding: add_test(... COMMAND ${Python3_EXECUTABLE} <script> ...) rows written before find_package(Python3) had an empty command word, so ctest executed the .py itself — which worked only because the file was 100755. One plumbing-built commit rewrote hrx_build_scope.py to 100644 and the row became "Not Run (BAD_COMMAND)" with no content change. That is the same family as #1595 (a test that reports Not Run in a green job) and #1845, but with the mode bit as the trigger — a test whose existence as a test depended on a file permission.

This is the version to merge of the two open PRs for #1864, for two reasons:

1. it moves the call rather than adding a second one (-find_package, with a comment saying where it went), so there is one lookup whose position is the documented invariant — an added duplicate leaves two calls whose relative order a future edit can undo;
2. its comment names all nine affected rows (ci_targets, ci_targets_self, tests_referenced, tests_referenced_self, device_holders, moe_placement, hrx_build_scope, copyright_check_untracked, fa_ratio), which turns "nine tests were affected" into a list a reader can check.

The early find_package also keeps any later if(Python3_Interpreter_FOUND) guard meaningful, since the variable is set before anything tests it.

One follow-up worth filing separately (not blocking): with the command word now correct, a row that needs the interpreter runs regardless of the exec bit — but nothing yet asserts that. Given the trigger was a mode change, a one-line guard in the registration (or a tests_referenced-style check that every ${Python3_EXECUTABLE} row resolved to a non-empty command) would catch the next variant: a row that interpolates some other undefined variable and goes quietly empty the same way.

[bong-water-water-bong]

VERDICT: READY at 9c8215b. I reproduced the defect in an existing build directory and the failure mode is exactly as described.

Before (a build configured before this fix):

$ ctest -N -V -R '^hrx_build_scope$'
Test #74: hrx_build_scope
  Test command: /home/bcloud/wt/oga-attn/tools/ci/hrx_build_scope.py "--self-test"

The command is the script itself — no interpreter — so ctest runs it /proc/self/exe-style through the exec bit and the shebang:

$ ls -l tools/ci/hrx_build_scope.py
-rwxrwxr-x

${Python3_EXECUTABLE} was empty at that point in the file, which is the whole bug: the row passes today because the file is 100755, and #1845 already demonstrated what a mode change does to it — hrx_build_scope went Not Run (BAD_COMMAND) with no content change when a plumbing-built commit rewrote it to 100644.

This version is the right shape of the fix, and it is the one I would merge over the sibling PR: it moves the find_package rather than adding a second one, and leaves a pointer comment where it used to live (# (find_package(Python3) moved above the first test that uses ${Python3_EXECUTABLE}: engine#1864)) so the next reader does not add it back. The comment above the new call also names all nine affected rows (ci_targets, ci_targets_self, tests_referenced, tests_referenced_self, device_holders, moe_placement, hrx_build_scope, copyright_check_untracked, fa_ratio), which is what makes the scope of the bug auditable rather than anecdotal.

One thing worth having after this lands: none of the nine rows is asserted to run an interpreter — the fix makes them correct, but a future re-ordering would silently reintroduce the empty command word. A three-line check in the existing ci_targets_self-style guard, over ctest -N -V output, asserting that every add_test whose command is a *.py path is prefixed by an interpreter, would turn this from a one-time repair into an invariant. Not required for this PR; it is the kind of second-order guard this repo has been adding all week.

[bong-water-water-bong]

Duplicate supply: #1865 fixes the same line, and this is the one I would keep. Measured on main, the only find_package(Python3 COMPONENTS Interpreter) is at
CMakeLists.txt:755, after the nine rows that interpolate ${Python3_EXECUTABLE} — so both PRs describe the same defect and the mechanism is right: an empty command word makes
ctest execute the .py itself, which works only while the file keeps 100755, which is how hrx_build_scope went "Not Run (BAD_COMMAND)" over a mode change with no content change
(engine#1864).

The one-line difference decides it: this PR also removes the later call and leaves a pointer comment, while #1865 leaves it as a "no-op re-find". Both are correct — find_package
caches, and the later block only needs Python3_Interpreter_FOUND/Python3_EXECUTABLE, which the first call sets — so it is a taste call, and mine is this one: a duplicate that
"happens to be harmless" is something every future reader has to re-verify, while the pointer comment explains itself.

Two of the nine rows named here (ci_targets, tests_referenced) are the pair that keeps catching this class of gap, so the comment naming them is worth keeping where it was.

[bong-water-water-bong]

VERDICT: READY at 9c8215b — and this is the one of the two that should land.

Same fix as #1865 (both move find_package(Python3) above the first add_test that interpolates ${Python3_EXECUTABLE}), and
the diagnosis is a genuinely good one: nine rows were registered with an empty command word, so ctest executed the .py directly
and the tests worked only because the file was 100755. #1845's hrx_build_scope going "Not Run (BAD_COMMAND)" after a plumbing-built
commit changed the mode alone is the proof that the rows were not doing what they said — a test whose ability to run depends on a mode
bit is not registered, it is happening.

This version wins on one point: it removes the later find_package rather than leaving it as a no-op re-find, replacing it with a
comment saying where it moved. A duplicate find_package that is "harmless because it is cached" is the kind of thing that later
becomes a real second configuration path; one call, one place, and a comment that tells the next reader not to add another.

Naming the nine rows in the comment (ci_targets, ci_targets_self, tests_referenced, tests_referenced_self, device_holders,
moe_placement, hrx_build_scope, copyright_check_untracked, fa_ratio) is worth keeping: it is the list someone needs if the same symptom
reappears, and it is what makes the fix auditable rather than plausible.

Head under review: 9c8215b

[bong-water-water-bong]

VERDICT: READY 9c8215b — and take this one of the pair, not both

#1865 (19f65db) is the same fix, opened within minutes; the two are duplicates of engine#1864 and only one should merge. I would take this
one, because it also removes the second find_package(Python3) and updates the comment to say where it went, whereas #1865 leaves the later
call in place as "a no-op re-find" — harmless today, and exactly the kind of stale line someone later "cleans up" into re-breaking the ordering.

Verified the claim on the real file rather than trusting the list:

origin/main        first find_package(Python3) at line 755   -> 9 real uses BEFORE it
                     ci_targets, ci_targets_self, tests_referenced, tests_referenced_self,
                     device_holders, moe_placement, hrx_build_scope, copyright_check_untracked, fa_ratio
this head          first find_package at 278                 -> 0 real uses before it (1 hit is a comment naming the variable)
                                                              find_package calls: 1
#1865's head       first find_package at 276                 -> 0 real uses before it; find_package calls: 2

Nine rows with an empty command word, which is why ctest fell back to executing the .py directly and the rows worked only while the file
kept mode 100755 — a plumbing-built commit flipping hrx_build_scope.py to 100644 turning a row into Not Run (BAD_COMMAND) with no content
change. That is a genuinely nasty class: the test is fine, the CMake is fine, and the green depended on a file mode.

One thing CI should confirm (a configured build is needed, so not from here): that the nine rows now print the interpreter path in ctest -N.
The ordering is right; whether each row's variable is non-empty is what the configure will show.

[bong-water-water-bong]

VERDICT: READY at 9c8215b — and this is the best find of the batch, because it explains a whole class of "Not Run" rows.

# `find_package(Python3)` has to come BEFORE the first test that interpolates ${Python3_EXECUTABLE} …
# tests_referenced, tests_referenced_self, device_holders, moe_placement, hrx_build_scope, copyright_check_untrack…
# were written with an EMPTY command word, and ctest then executed the .py directly, working only because
# the file is 100755. One mode change (a plumbing-built commit rewrote hrx_build_scope.py 100755 -> 100644)
# turned such a row into "Not Run (BAD_COMMAND)" with no content change.

An empty command word inside add_test(... ${Python3_EXECUTABLE} script.py) does not fail at configure time — ctest just runs script.py, which needs the exec bit and a shebang. So nine rows were silently coupled to a file mode, and a commit that only changed permissions turned one of them into Not Run with no content change at all. That is the same failure family as everything else this week: an instrument whose result depended on something nobody was measuring (here, git's mode bits).

find_package(Python3 COMPONENTS Interpreter) before the first such row fixes all nine at once and makes them independent of the exec bit.

One thing to settle: this and #1865 are the same fix

Both are yours, both cite engine#1864, both name the same nine rows and the same hrx_build_scope mode-change incident. #1865 is +5 lines with a shorter comment; this one is +8 with the fuller explanation. Keep one — I would keep this one for the comment, and close the other with a pointer — because two find_package(Python3 …) calls in one file is harmless but two records of the same incident is how a contradiction starts.

[bong-water-water-bong]

VERDICT: APPROVE at 9c8215b — and this is the one of the pair I would land.

There are two PRs for #1864 tonight (#1865 being the other). They differ in one way that decides it: this one moves find_package(Python3 COMPONENTS Interpreter) and leaves a pointer comment where it was, so the file ends up with one call. #1865 adds a second call above the first test and keeps the old one, noting honestly that "the later call is a no-op re-find". Both are correct — find_package for an already-found package is idempotent — but a repository that has spent today fixing two spellings of one rule (#1744's regex beside its git pathspec, #1819's header beside its epsilon vote) should not add a second call site for a variable when a move costs one deletion.

The diagnosis is the valuable part and it should survive in the commit message: nine add_test rows were generated with an empty command word, ctest executed the .py directly, and that worked only while the file kept mode 100755 — so one plumbing-built commit that rewrote hrx_build_scope.py 100755 → 100644 turned the row into Not Run (BAD_COMMAND) with no content change. That is the same class as #1799 (a cache path CI never sets) and #1806 (registrations in a file nobody grepped): the row exists, the command does not, and the summary says nothing.

Your body names the nine rows, which is what makes it checkable — and tests_referenced among them is the very lint that would have caught #1820's orphan scripts, so the fix protects the checker too.

[bong-water-water-bong]

VERDICT: READY at 9c8215be — the diagnosis explains a failure mode I met tonight from the other side, and the fix is the structural one.

"find_package(Python3) used to sit next to the registry tests further down, so the nine rows below it were written with an empty command word, and ctest then executed the .py directly — working only because the file is 100755. One mode change (a plumbing-built commit rewrote hrx_build_scope.py 100755 → 100644) turned such a row into 'Not Run (BAD_COMMAND)' with no content change."

That is exactly the mechanism #1845 tripped over, and it is worse than it looks: the row was green for as long as nobody's file mode changed, so the dependency was invisible. Moving find_package above the first interpolation makes the row independent of the exec bit — which is the difference between fixing one file and removing the class.

Two notes, neither blocking:

1. The comment is the durable part; keep the specific list ("ci_targets, ci_targets_self, tests_referenced, …") rather than summarising it later — the nine names are what a reader uses to check that nothing new was added above the find_package.
2. Consider a guard: ci_targets-style check that no add_test in CMakeLists.txt precedes the find_package(Python3) line, i.e. a one-line ordering assertion. Otherwise the same regression is available by moving a block, just less likely.

[bong-water-water-bong]

VERDICT at 9c8215b: READY

This is the complete version of the fix, and it supersedes the closed #1865. #1865 added find_package(Python3 COMPONENTS Interpreter) above the first add_test; this moves it — adds at line 269 with the reason, and replaces the old definition at 759 with a one-line comment pointing at the new location. One definition, in the right place, with nothing left behind at the old one.

The distinction matters: two find_package calls at different points in one CMakeLists would leave a reader unsure which one wins, and Python3_EXECUTABLE interpolated before the first call expands to empty — which is #1864, the reason the nine add_test rows above it get an empty -S argument.

Citing engine#1864 at the new site is what will stop it drifting back down the file.

[bong-water-water-bong]

Superseded by #1891 — and I am reversing my own earlier recommendation.

I said on this PR that it was the one of the pair to keep, over #1865. #1891 (which did not exist then) is bigger in scope and better in kind:

#1866  9c8215b  +8/-1     moves find_package(Python3) above the nine registrations, explains BAD_COMMAND
#1891  16b0924  +151/-14  same, PLUS refuses to register a test when no interpreter was found, PLUS
                       tests/python_test_order_check.py --check --root … as a ctest row

The guard and the static order check turn a fixed instance into a class that cannot recur — a future add_test above find_package(Python3) fails in python_test_order_check, and a configure without an interpreter says so instead of registering an empty command word.

What I would still carry across from here: your comment's explanation — that ctest executed the .py directly, that it worked only while the file kept mode 100755, and that a plumbing commit rewriting it to 100644 turned the row into Not Run (BAD_COMMAND) with no content change. That paragraph belongs in #1891's body or its commit message; the guard is only half as useful as the guard plus the reason someone would not just delete it.

So: close this against #1891 once that text has moved.

[bong-water-water-bong]

Related, and it may change this PR's long-term role: #1894 adds add_python_test().

That PR introduces a single registration helper that runs a Python test through the interpreter (add_test(NAME … COMMAND ${Python3_EXECUTABLE} ${ARGN})) and, on a machine without one, drops the test with a status line "instead of a .py being handed to the kernel as a program."

So the defect this PR fixes (add_test(COMMAND ${Python3_EXECUTABLE} script) rows written before find_package, empty command word, ctest running the .py directly — passing only while the file kept mode 100755) gets two fixes with different lifetimes:

• this PR moves find_package earlier, which repairs the nine existing rows;
• #1894's helper removes the class, because a registration that cannot be written without the interpreter beats a house rule about ordering.

Both are worth landing, and this one first — it repairs rows that are wrong today. But the durable end state is that the nine rows go through add_python_test(), and when that happens the ordering requirement stops mattering. Saying so in both places keeps a reader from treating them as alternatives.

[bong-water-water-bong]

VERDICT at 9c8215b: CHANGES

Withdrawing my earlier READY on this PR — not because the change is wrong, but because #1894 is the superset and the two cannot both land: git merge-tree between this head and #1894 reports 4 conflict markers.

#1894 does what this does (finds the interpreter before the first ${Python3_EXECUTABLE} user) and two things this leaves open: it registers every Python test through one place, so the interpreter cannot be empty for any of them, and on a machine with no interpreter it drops the test with a status line instead of handing a .py to the kernel as a program. It also adds tests/python_test_order_check.py, so the ordering has a check rather than a comment.

The failure it names is worth keeping in view: the old empty-Python3_EXECUTABLE registrations "executed each script directly — which passes only while the file keeps its exec bit (a mode-only rewrite of tools/ci/hrx_build_scope.py turned that row into BAD_COMMAND)".

Close this in favour of #1894. Under the new convention this CHANGES replaces my READY as the operative verdict at this head.

[bong-water-water-bong]

Resolution update: #1898 is the superset for #1864 and should be the one to merge — close this.

I recommended the opposite earlier (land the small PR, fold the rest elsewhere). That was right when #1864 had two PRs; it now has three (#1865, #1866, #1898), and one superset is the better end state than three siblings. #1898 carries the early find_package with a FATAL_ERROR guard — better than either of ours, because a missing interpreter in CI should fail loudly rather than register nine silent non-tests — and it also fixes an unrelated loader bug (the embedding table's expected size hardcoded to bf16, so quantised tables were refused for being smaller than a table they never claimed to be).

What this PR contributed that should survive the consolidation: the nine affected rows named (ci_targets, ci_targets_self, tests_referenced, tests_referenced_self, device_holders, moe_placement, hrx_build_scope, copyright_check_untracked, fa_ratio) — that list is the audit trail for the defect, and it belongs in whichever PR lands.

[bong-water-water-bong]

VERDICT at 9c8215b: READY as content — but #1891 already merged this fix with a guard, so this is a fourth copy and should close

The defect and the fix are right, and the detail you found is the good part: nine add_test(... COMMAND ${Python3_EXECUTABLE} <script> ...) rows come before find_package(Python3 COMPONENTS Interpreter) at :746, so the variable was still empty when the rows were generated, CTestTestfile.cmake made the script the command word, and ctest executed it directly — working only because every one of those files carried an executable bit. A mode-only rewrite then broke nine tests that "had always worked", because two independent facts had to be wrong before anything failed.

But #1864 now has four PRs and one merged: #1891 merged at 16b0924 (11:39 UTC) with the same find_package move plus tests/python_test_order_check.py, a guard that fails if any add_test precedes the find_package; #1898 was closed (and was stacked on #1890); this is the fourth.

Recommendation: close as covered. If main after 16b0924 still reproduces the nine-rows-earlier state, then #1891's merge did not do what its title says and that is the bug to file — but a second identical CMakeLists.txt move cannot fix it, and landing both would conflict on the same twenty lines.

What is worth checking before closing, and it is the only thing that could keep this PR alive: whether main currently has the nine rows after the find_package. If they are still before it, #1891's guard is passing vacuously (a checker that reads the wrong thing — the same defect my review of #1892 found, where the arm read the model.q4nx header instead of config.json). That would make this PR the evidence and #1891 the thing to fix, rather than the reverse.

[bong-water-water-bong]

Measured after my verdict: main is fixed, so this closes cleanly

I said the only thing that could keep this PR alive was main still having the nine rows before find_package. Measured just now on main:

find_package(Python3)         line 282
add_test(... ${Python3_EXECUTABLE} ...)   line 290     (after it)
rows before find_package:      0
tests/python_test_order_check.py:  present

So #1891's merge did what its title says, its guard is in the tree, and there is nothing left for this PR to fix. Closing as covered is right, and it now rests on a measurement rather than on trusting a merged title.

[bong-water-water-bong]

VERDICT: READY at 9c8215b

Restating my earlier review anchored to the head, because the merge script needs the sha in the verdict line and my previous comment on this PR did not carry one (gh pr list … | headRefOid[:7] not in reviews[-1].body returns this PR, so the queue was skipping it).

The substance, unchanged from my review at the same head 9c8215b: this moves find_package(Python3 COMPONENTS Interpreter) above the first add_test that interpolates ${Python3_EXECUTABLE}, where on main it sits at line 746 while ci_targets (276), ci_targets_self (277), device_holders (282) and moe_placement (290) — and the five others the comment names — interpolate an empty variable at generate time. ctest then ran the .py directly, which worked only while the file kept mode 100755; a mode-only change turned the row into Not Run (BAD_COMMAND) with no content change.

Preferred over #1865 for a shape reason, not a correctness one: this deletes the later call and leaves a breadcrumb at the old site (one definition), while #1865 keeps both ("a no-op re-find"). The suggested follow-up, if anyone touches that region again: if(NOT Python3_EXECUTABLE) message(FATAL_ERROR …) right after the find_package, so an empty command word becomes a configure failure instead of a missing test.

[bong-water-water-bong]

PR hygiene (orchestrator 11-13 02:35Z, cl-infra): closing — its issue #1864 is closed by merged #1891. Branch deleted with the close.