bank/gui + ci: clear the eleven bank-GUI sources' clang-tidy debt, and gate the Q_OBJECT header split that no build catches early (fixes #656, fixes #659)

[Yaraslaut]

Two tickets, one commit each, branched from 7ab4c7a9.

• 22951eb1 — 97 unreported clang-tidy findings in the eleven bank-GUI sources morph#651 brings into reach #656: clear the 97 clang-tidy findings -DMORPH_BUILD_BANK_GUI=ON brought into the clang-tidy job's reach.
• aa84b126 — No gate catches a Q_OBJECT header split from its TU; the first signal is a link error in six CI legs #659: a drift-guard gate that catches a Q_OBJECT header split from its translation unit, before the link does.

Both accepted as framed. Neither was rejected, and #659 was not concluded wontfix — see "Why not wontfix" below.

---

#656 — the bank GUI's clang-tidy debt

My own re-measurement

The ticket's 97 was measured on 4563aff3 and had not been reproduced since. Re-measured here on 7ab4c7a9, before touching anything — clang-tidy 22.1.8 (ci.yml pins CLANG_VERSION: "22"), Qt 6.11.2, the clang-tidy job's own configure flags plus its own -extra-arg pair, cold build directory:

$ cmake --preset clang-debug -DMORPH_BUILD_NET=ON -DMORPH_BUILD_QT=ON \
    -DMORPH_BUILD_FORMS_QML=ON -DMORPH_BUILD_OFFLINE_SQLITE=ON \
    -DMORPH_BUILD_LOAD_TESTS=ON -DMORPH_BUILD_HMAC_EXAMPLES=ON \
    -DMORPH_BUILD_FUZZERS=ON -DMORPH_BUILD_LADDER=ON \
    -DMORPH_BUILD_BANK_EXAMPLE=ON -DMORPH_BUILD_BANK_GUI=ON \
    -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++
-- Configuring done (67.6s)

$ clang-tidy -p build/clang-debug --extra-arg=-std=c++23 \
      --extra-arg=-Wno-missing-include-dirs --quiet <each of the eleven>

total in-source findings: 97

file	before	after
gui/controllers/CardController.cpp	20	0
gui/controllers/LoanController.cpp	19	0
gui/controllers/TransactionController.cpp	12	0
gui/controllers/AccountController.cpp	11	0
gui/controllers/PayeeController.cpp	11	0
gui/main.cpp	10	0
tests/gui/test_bank_qml_surface.cpp	6	0
tests/gui/test_bank_gui_qml_behaviour.cpp	6	0
gui/controllers/AppController.cpp	2	0
gui/BankClient.cpp, gui/controllers/BankController.cpp	0	0
total	97	0

Per check, before: 44 cppcoreguidelines-pro-bounds-avoid-unchecked-container-access, 23 performance-unnecessary-value-param, 9 misc-const-correctness, 8 readability-identifier-length, 2 each of readability-static-accessed-through-instance / readability-implicit-bool-conversion / readability-avoid-nested-conditional-operator / modernize-use-auto, and one each of readability-function-cognitive-complexity, readability-container-size-empty, cppcoreguidelines-pro-bounds-constant-array-index, concurrency-mt-unsafe, and one site reported under both cppcoreguidelines-avoid-c-arrays and modernize-avoid-c-arrays (which is why the issue's per-check list sums to 98 while the finding count is 97).

It did not come back empty, so the ticket stands rather than closing as an artefact of the filer's local Qt. The count is identical file-for-file and check-for-check to 4563aff3, which is itself a small result: nothing between the two revisions touched these sources.

The two mechanical passes, then the residual

67 of 97 were two passes, as the ticket predicted:

• 44 map[QStringLiteral("k")] = v on a QVariantMap become map.insert(QStringLiteral("k"), v). QMap::operator[] on a non-const map inserts a default and returns a reference to assign through; insert does the same lookup and assignment in one call, so the map is byte-identical and no bounds-unchecked accessor remains.
• 23 by-value continuation parameters become const references. Completion<T>::then takes std::function<void(const T&)>, so each of these was copying a DTO out of a reference the caller already held.

The residual 30 needed judgement one at a time, and the reasoning is in the commit message. Briefly: id parameters renamed for what they identify (headers too — QML binds Q_INVOKABLE arguments positionally, so no .qml sees it); CardController's two nested conditional operators replaced by one if/else if over CardStatus; in main.cpp, setApplicationName/exec called statically so app can be const, std::getenv → qgetenv (which the two seed variables twenty lines below already used), and the const char* names[5] → a QStringList read with .at(), which removes the C-array pair and the non-constant index together.

Cognitive complexity: two directives, and a correction to what the first revision of this PR claimed

The first revision of this PR suppressed readability-function-cognitive-complexity on the second behaviour TEST_CASE and wrote, in the tree, that "the sibling case above scores under the threshold and stays covered". It does not, and never did. clang-tidy-diff went red on exactly that sibling:

examples/bank/tests/gui/test_bank_gui_qml_behaviour.cpp:115:1: error: function 'dummyFunction72'
    has cognitive complexity of 87 (threshold 25)
    [readability-function-cognitive-complexity,-warnings-as-errors]

Both cases are now measured, with the clang-tidy job's own configure and its own -extra-arg pair, clang-tidy 22.1.8, threshold lowered to 1 so that both report rather than only the one over:

case	function	score	assertions
MoveMoneyPage's picker keeps naming …	dummyFunction72	87	21 REQUIRE/CHECK
Main.qml confirms a posted transaction …	dummyFunction76	45	11 REQUIRE/CHECK

Which story is true: it was already over, the earlier measurement missed it

The branch's edits inside the first case are a lambda-parameter rename, QObject* → auto*, and an added INFO. None of them moved the number. Checking out origin/master's copy of the file into the same build and re-running gives the same two scores:

… test_bank_gui_qml_behaviour.cpp:115:1: error: function 'dummyFunction72' has cognitive complexity of 87 (threshold 1)
… test_bank_gui_qml_behaviour.cpp:210:1: error: function 'dummyFunction75' has cognitive complexity of 45 (threshold 1)

(75 rather than 76 because Catch2 names these off __COUNTER__, and this branch adds one more macro expansion earlier in the file.)

So the 87 is not new. What is new is that it is reported. clang-tidy-diff filters findings to changed lines, and ClangTidyDiagnosticConsumer accumulates that filter over the finding and each of its notes — so a finding surfaces when any one of its notes lands on a changed line. On origin/master none of this case's notes was on a changed line; renaming the balanceOf lambda's parameter put one there. The earlier claim was read off the finding not being reported, which is a different fact from it being under the threshold.

What the 87 is made of

REQUIRE/CHECK expand to do { … try { … } catch (…) { … } } while ((void)0, (false) && …) — a loop (+1), a handler (+2 at nesting level 1) and a && (+1): four points per assertion. 21 × 4 = 84 and 11 × 4 = 44, so 3 of the 87 and 1 of the 45 are everything the tests' own shape contributes (the nested lambdas here, the six-controller range-for there). Measured rather than arithmetic: commenting a single CHECK out of the first case moves it 87 → 83.

Why a directive rather than a split, and what the split would have cost

At four points an assertion, a threshold of 25 permits six assertions per TEST_CASE. The first case's prologue alone — register a user, open two accounts, stand up a QQmlEngine, load MoveMoneyPage.qml, drive the picker onto the savings account — is eight assertions, 32. Every fragment of a split carries that prologue, so no split of this case reaches the threshold; it would take four fragments and the prologue hoisted into a helper, and the helper then scores what the fragments no longer do. The score moves, it does not go away. And morph#296's defect is the sequence — pick, deposit, still picked, deposit again, the money followed the label — which is the thing a split would scatter. tests/.clang-tidy makes the same argument for the framework's own suite and subtracts the check there; this file argues it per case instead.

Two per-case directives, each with its own measured reason, rather than:

• an entry in examples/bank/tests/.clang-tidy — that would subtract the check from every bank test including the ones not written yet, which is examples/common/testkit/.clang-tidy also governs the testkit *library* sources, contradicting its own prose #652's mistake one directory over. Still no new .clang-tidy anywhere and that file is still untouched;
• a NOLINTBEGIN/NOLINTEND span — which would also cover whatever is added between them;
• raising the threshold — which is not this PR's to do.

Each reason sits above its directive, never wrapped onto it (#631); scripts/check_nolint_directives.sh passes with 173 directives, all annotating code.

Verified, including the second directive's anti-vacuity

The clang-tidy-diff job was reproduced locally, not inferred: clang-tidy-diff.py with this job's -path/-p1/-extra-arg set, over git diff -U0 origin/master for the branch's whole diff, exits 1 with exactly the CI error above before this correction and exits 0 after it.

The second directive turns out not to be exercised by this branch's own diff at all — with it deleted, the full branch diff still reports only the 87. It is armed rather than decorative, which was checked separately: over a one-line diff on that case's REQUIRE(pumpUntil([&app] { … })) line, deleting the directive reports dummyFunction76 … 45 (threshold 25) and exits 1, and restoring it exits 0. A one-line diff on a non-lambda assertion line in the same case reports nothing either way — the gate's condition is a changed line carrying a user-code note, not merely a changed line inside an over-threshold case.

The 97 → 0 claim was also re-checked whole-file against the runner's own Catch2 series rather than the workstation's: all eleven sources, full check set, 0 findings inside them and 0 clang-diagnostic-error.

Why the first revision's local run agreed with it — filed as #666

Reproducing any of this locally needed Catch2 3.5.x headers ahead of the workstation's on -isystem. With the workstation's Catch2 3.16.0, clang-tidy computes the same 87 and 45 and then drops both findings as non-user code, so a local clang-tidy-diff run is green on a diff CI fails, silently and in both directions a reviewer would look. That is filed as #666 (reproduced; the mechanism inside ClangTidyDiagnosticConsumer is inferred and labelled as such). #667 is the smaller one it turned up: tests/.clang-tidy and its nine copies state the job installs Catch2 3.4.0, which cannot be right — 3.4.0 has no dummyFunctionNN to name.

How this landed in the history

Folded into 22951eb1 (#656) by amend rather than stacked on top, so the PR does not ship one commit asserting something false and a second retracting it; the commit message carries the same correction and the same measurements. aa84b126 (#659) was replayed unchanged — the two commits touch disjoint files. Head is now 0a04b0de.

scripts/check_tidy_suppression_scope.sh and scripts/check_rung_filters.sh both pass.

The claim the branch's safety rests on

That QVariantMap::insert(key, value) is indistinguishable from map[key] = value for these 44 sites. Every key is a distinct string literal written once per map, so there is no overwrite case where the two could differ, and both insert-or-assign. If that is wrong, the QML sees different property bags and the GUI silently shows nothing — which is exactly why the three bank GUI tests, all of which read those bags back through a live QML engine, were run.

Verification

• Measured: 97 → 0, same command, same build directory.
• Anti-vacuity: the run still reports 1743 diagnostics in include/morph/** headers from these eleven TUs and zero clang-diagnostic-error, so the analysis is live rather than skipping the files. And reinstating one map[...] = ... in PayeeController.cpp brings the finding straight back:

  PayeeController.cpp:52:20: error: possibly unsafe 'operator[]', consider bounds-safe alternatives [cppcoreguidelines-pro-bounds-avoid-unchecked-container-access,-warnings-as-errors]
  

  The zero is a measurement, not an empty walk.
• Measured: builds clean under clang 22.1.8 (clang-debug, -Weverything -Werror) and under gcc (gcc-debug), both configured from empty with -DMORPH_BUILD_BANK_GUI=ON. Both compilers because linux-all-features runs both.
• Measured: bank_gui_tests and bank_gui_qml_tests pass, all three cases, under QT_QPA_PLATFORM=offscreen.
• Not verified: the Emscripten leg. wasm-demo.yml builds bank_gui_wasm from these same seven controller sources (main.cpp is not in that target) and no Qt-WASM toolchain is installed here. The changes are Qt Core only and introduce no new API, so the risk is low — but it is unverified, not low-risk-therefore-verified.

---

#659 — no gate caught a Q_OBJECT header split from its TU

scripts/check_qobject_moc_pairing.py, wired into drift-guard.yml as its own job (self-test first, then the gate).

AUTOMOC finds a Q_OBJECT header two ways and only two: beside a translation unit of the same basename, or named in a target's own source list. When neither holds it generates nothing and says nothing; the .cpp compiles, the static library archives, and the first signal is a linker error about a missing vtable in every leg that links the target — six red legs on #657, the fastest at 4m03s. cmake/morph_add_rung.cmake describes this failure and names the case it was diagnosed on; #652 hit it again in a different CMakeLists anyway.

For every tracked header carrying an AUTOMOC macro, the gate requires one of three mechanisms, each checked rather than assumed:

1. a translation unit of the same basename in the same directory;
2. the header named in a target's source list — parsed out of the CMake corpus by balanced-paren command extraction, with # comments blanked first (examples/common/CMakeLists.txt names fault_proxy.hpp five times in the paragraph explaining why it is listed, and counting those would let the prose about the coverage stand in for the coverage) and FILE_SET argument blocks dropped (morph_qt's installed-header set names qt_websocket_server.hpp but drives no moc; morph_qt_impl's source list is what does);
3. the header under examples/<rung>/include/ for a rung in examples/rungs.txt — morph_add_rung()'s glob. This gate does not resolve CMake globs, so it asserts that one: it fails if morph_add_rung.cmake stops carrying a file(GLOB_RECURSE _lib_headers ... include/*.hpp) whose result reaches an add_library(). Four of the tree's six split headers are covered by nothing else.

Measured on this tree:

walked 349 tracked header(s) across 41 CMake file(s)
40 carry an AUTOMOC macro:
    34 paired with a same-directory translation unit
    2 named in a target's source list
    4 under a ladder rung's include/, globbed by morph_add_rung()
    0 with no moc pairing at all
ok: morph_add_rung() globs include/*.hpp into ladder_<rung>_lib
Q_OBJECT moc-pairing lint OK.

Why not wontfix, and why drift-guard.yml is the right host

The ticket's escape hatch was: if the check needs a configured build to know which headers are reachable, it does not fit drift-guard.yml's contract ("Every job here is fast and dependency-free; none of them compiles anything"), and saying so is a legitimate outcome.

It does not need one. The scoping problem the issue raised — a build tree only holds what its configure enabled, so an output check false-positives on everything behind an off-by-default option — applies to checking the moc output. Checking the pairing dissolves it: a header behind an off-by-default option still has to be listed in its conditionally-added target, and which options a configure turned on does not enter into it. So there is no configure, no compiler, no Qt, and the gate runs in under a second. The wontfix condition is not met.

It also does not check that the target owning the source list has AUTOMOC on. That is a second way to get no moc output; it has never happened here, and resolving target properties means a configure. Recorded in the script's header rather than left implicit.

The mutation the issue asked for

Both vacuity traps are closed, and the tree is clean today, so the gate ships already green.

• It prints what it examined — headers walked, headers carrying a macro, and which mechanism covered each — and exits 1 when the macro-bearing set is empty. A scan that stops recognising Q_OBJECT is a failure, not a pass.

• --self-test drives nine fixtures, two of them mutations of this repository's real files:

  fixture 6b — the issue's stated close condition. A copy of the real examples/common/, with testkit/fault_proxy.hpp deleted from morph_ladder_testkit's source list in the real examples/common/CMakeLists.txt. That is the exact examples/common/testkit/.clang-tidy also governs the testkit *library* sources, contradicting its own prose #652 regression:

  --- fixture 6: the real examples/common/, unmutated ---
  2 carry an AUTOMOC macro:
      1 paired with a same-directory translation unit
      1 named in a target's source list
      0 with no moc pairing at all
  Q_OBJECT moc-pairing lint OK.
  
  --- fixture 6b: fault_proxy.hpp removed from the source list ---
  2 carry an AUTOMOC macro:
      1 paired with a same-directory translation unit
      0 named in a target's source list
      1 with no moc pairing at all
  

  exit 1, naming examples/common/testkit/fault_proxy.hpp.

  fixture 7b — the file(GLOB_RECURSE _lib_headers ...) line deleted from a copy of the real cmake/morph_add_rung.cmake, with a rung header credited to it. Exits 1 rather than keeping the credit. This is what stops mechanism 3 above from being a decorative assertion.

  Each mutation asserts that it changed something, so a rename upstream turns the self-test red instead of quietly making it a no-op. The other seven fixtures cover the paired arm, the listed arm, an unlisted split, a header "covered" only by a comment and a FILE_SET, and a tree with no macro headers at all.

self-test OK: 9 fixture(s), including the #652 mutation of the real
examples/common/CMakeLists.txt and a mutation of the real
cmake/morph_add_rung.cmake.

The claim this half rests on

That AUTOMOC has exactly two ways of finding a Q_OBJECT header, so a header satisfying neither gets no moc output. That is Qt's documented behaviour and it is what both in-tree remedies (morph_add_rung.cmake's glob, examples/common/CMakeLists.txt's explicit listing) are written against. If a third mechanism exists, this gate is over-strict — it would report a header that is in fact mocced, which is a false positive on a clean tree and would be found immediately, not a silent hole.

Not verified

The gate against the pre-fix revision d380895c itself. Fixture 6b reconstructs that state from the current file rather than checking the old commit out, so it proves the gate fires on the shape, not that it would have fired on that commit's whole tree.

---

Findings filed, not folded

Both found while clearing #656; neither belongs to either change.

• bank GUI: parseMinor casts an unbounded double to int64_t; 1e30 in any amount field is UB #663 — bankgui::fmt::parseMinor casts an unbounded double to std::int64_t. Every amount the GUI accepts goes through it, the QML fields have no validator, and QString::toDouble accepts 1e30. Reproduced with UBSan on the extracted arithmetic: runtime error: 1e+32 is outside the range of representable values of type 'long', exit 1. No sanitizer leg covers bank_gui, so nothing would have caught it. Includes a second, explicitly weaker observation about + 0.5 rounding on the same line.
• HeaderFilterRegex excludes every examples/** header from clang-tidy: 49 findings from five of 737 TUs #664 — .clang-tidy's HeaderFilterRegex: "include/morph/.*" discards every finding in every examples/** header, from every job. Reproduced: 49 distinct findings across 12 headers from five of the eleven bank-GUI TUs, against a database of 737 entries — a lower bound, and a small one. This is the same shape as clang-tidy-diff's configure omits the application ladder, so no rung is in the compile database it lints against #481/tests/lint/ fixtures are inside clang-tidy-diff's scope, and nothing tells the author that until CI is red #650/60 of 470 tracked C/C++ sources are outside clang-tidy-diff's reach, and 11 of them only because one CMake flag is missing #651/97 unreported clang-tidy findings in the eleven bank-GUI sources morph#651 brings into reach #656 one level down, and it is why bank GUI: parseMinor casts an unbounded double to int64_t; 1e30 in any amount field is UB #663 sat unreported.

No triage: label on either; that verdict is the triage-issue skill's, not the author's.

Gates run locally before pushing

check_automoc_includes.sh (against build/clang-debug), check_tidy_suppression_scope.sh, check_rung_filters.sh, check_bidi_controls.py, check_workflow_job_banners.py, check_nolint_directives.sh, check_qobject_moc_pairing.py and its --self-test, and clang-format --dry-run -Werror over exactly the twelve files touched. All clean. The workflow YAML parses.

Re-run after the cognitive-complexity correction: check_tidy_suppression_scope.sh, check_nolint_directives.sh, check_qobject_moc_pairing.py and its --self-test, check_bidi_controls.py, and clang-format --dry-run -Werror over the one file touched. All clean.

CI has not been consulted. This PR was pushed and handed back immediately; whatever check counts are visible on it are incomplete.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW

[Yaraslaut]

Runner verification

I drove #659's close condition myself, on the real tree rather than through a fixture. Removing testkit/fault_proxy.hpp from morph_ladder_testkit's source list in the actual examples/common/CMakeLists.txt:

::error::examples/common/testkit/fault_proxy.hpp declares an AUTOMOC macro but AUTOMOC will
never see it: no translation unit of the same basename sits beside it, and no target's source
list names it. Nothing will fail until the first link that needs its vtable, in every leg at
once. Either move the .cpp back next to the header, or list the header among its target's
sources … header entries are not compiled, they only join the AUTOMOC scan.
    1 with no moc pairing at all
exit=1

restored → exit=0. Self-test: 9 fixtures pass. Clean tree: 349 headers walked, 40 with an AUTOMOC macro, 0 uncovered, exit 0. So the gate fails on exactly the regression that cost six red legs three hours ago, and the message tells the next person both remedies rather than just the verdict.

Rejecting wontfix was right, and for the reason given rather than by preference. My escape hatch was "if it needs a configured build". Checking moc output would; checking the pairing does not — a header behind an off-by-default option still has to be listed in its conditionally-added target. No configure, no compiler, no Qt, under a second. That fits drift-guard.yml's stated contract instead of straining it.

Fixture 7b is the part I would have missed. Four of the six split headers are credited to morph_add_rung.cmake's file(GLOB_RECURSE _lib_headers …), which the gate cannot resolve, so it asserts the glob's existence instead — and 7b mutates that line to prove the assertion is load-bearing. Asserting a precondition you cannot evaluate, and then testing that the assertion itself can fail, is the right handling of an unresolvable dependency.

#656's zero is a measurement, not silence. 97 reproduced exactly on 7ab4c7a9, file-for-file and check-for-check, then cleared to 0 — with the control that the same eleven TUs still report 1743 diagnostics in include/morph/** and zero clang-diagnostic-error. A post-fix run reporting nothing because it analysed nothing is the failure this repository keeps hitting; that control rules it out.

On the safety claim, which is stronger than argued. The branch rests on QVariantMap::insert(k, v) being indistinguishable from map[k] = v at all 44 sites, justified by every key being distinct per map. I checked and there are repeated key literals — "id" appears 8 times across the diff, twice in PayeeController.cpp alone — but those are different maps (account.id at :35, payee.id at :50). More to the point, QVariantMap is QMap, whose insert replaces an existing value, so the two forms agree even where a key does repeat. The only shape that would differ is QMultiMap, and there is none in examples/bank/gui/. So the claim holds with room to spare; the distinctness argument was a stronger condition than the change needs.

On the two filed issues

#664 confirmed structurally — only the root .clang-tidy sets HeaderFilterRegex: "include/morph/.*", and none of the ten other configs overrides it. clang-tidy inherits rather than merges that key, so findings in every examples/** header are discarded from every job. That is the same class as #481/#650/#651/#656, one level down, and it is the reason #663 sat unreported — which makes it the more valuable of the two.

#663 carries a reproduction (runtime error: 1e+32 is outside the range of representable values of type 'long') and the observation that no sanitizer leg builds bank_gui. Labelling the + 0.5 rounding note as explicitly weak, separately from the reproduced part, is the right discipline.

Not verified by me: the 97→0 measurement itself and the three bank GUI test runs (both need the Qt configure), and the Emscripten leg — wasm-demo.yml builds bank_gui_wasm from the same seven controllers and no Qt-WASM toolchain exists locally. The changes are Qt-Core-only with no new API, so the risk is low and stated rather than implied.

Worktrees pruned. Not merged: CI incomplete at hand-off.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[Yaraslaut]

Runner: red on clang-tidy-diff, and the finding contradicts this PR's own NOLINT reason

From the run's clang-tidy-report artifact:

examples/bank/tests/gui/test_bank_gui_qml_behaviour.cpp:115:1: error: function 'dummyFunction72'
    has cognitive complexity of 87 (threshold 25)
    [readability-function-cognitive-complexity,-warnings-as-errors]

Line 115 is TEST_CASE("MoveMoneyPage's picker keeps naming the account the next deposit will land in", …) — the sibling of the case suppressed at line 219. The reason attached to that suppression says:

"Here it is one case, so it is one directive rather than a suppression over every bank test — the sibling case above scores under the threshold and stays covered."

It scores 87.

The red leg is the smaller problem. A reasoned suppression whose justification rests on a measurably false claim about a neighbouring case reads as verified when it is not — the same defect this sweep filed as #652 (a .clang-tidy misdescribing its own reach) and corrected in backend.md (a spec line asserting parkIfInFrame is called from the dispatching frame when it is not). The wording is what makes a suppression auditable; wrong wording is worse than none.

Likely mechanism, stated as likely: this branch edits inside the case at 115 — hunks at 122, 155 and 207-208 — so either those edits pushed it over after the measurement, or the measurement predated them. Which one is true changes what the corrected comment should say, so it needs establishing rather than assuming.

Dispatching a lane whose whole batch is this PR, with three requirements: measure both cases on the current head with the job's own flags; decide the sibling on its merits and argue it (two suppressions in one file starts to look like widening a gate, and 87 against 25 is not marginal — splitting the test is a legitimate answer); and correct the reason at 219 either way, because as written it cannot stand.

Everything else on this PR stands — I verified the #659 gate by driving its close condition on the real tree, and #656's 97→0 with its anti-vacuity control, in the comment above.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW

[Yaraslaut]

Runner verification

Amend integrity checked, not assumed:

#659 commit byte-identical after amend+cherry-pick (0a04b0de vs aa84b126):  True
false sentence still in the tree:                                           0 occurrences

Folding the correction into the commit that made the false claim, rather than stacking a retraction on top, is the right shape — a PR that ships one commit asserting something and a second withdrawing it leaves the assertion in the history for git log -S to find.

The finding that changes the story

Both scores are identical on origin/master: 87 and 45. The branch's three hunks inside case 1 moved neither. So neither my framing nor the original lane's was right — I suggested its edits pushed the sibling over; they did not. What changed is only that the finding became reportable, because the line filter accumulates over a finding and its notes, and the lambda rename at line 125 put a changed line under one of case 1's notes. The measured control — a diff containing only case 2's changed line reports nothing — is what makes that mechanism credible rather than plausible.

That means the original false sentence was not carelessness. It was the predictable output of a local run that could not see the finding at all.

#666 is the systemically important one, and I verified its premise

local Catch2:        3.16.0
ci.yml installs:     sudo apt-get install -y ... catch2      (three jobs, unpinned)

With 3.16.0 clang-tidy computes the same 87/45 and then discards both as non-user code, so a bare local clang-tidy-diff is green on the exact diff CI fails. The lane only reproduced CI by putting 3.5.3 headers ahead of it on -isystem.

This undermines a class of evidence I have been accepting all session. Several lanes have reported "clang-tidy clean locally" and been contradicted by CI, and at least this one is explained: the local tool was blind, not lenient. And CI's Catch2 is unpinned — apt-get install -y catch2 takes whatever Ubuntu ships that day, so the divergence can widen without any change to this repository. That is worth more than the red leg it surfaced.

On the decision to suppress both

The derivation is the part that earns it: REQUIRE/CHECK expands to a do { try … catch … } while costing 4 points, verified by mutation (commenting one CHECK out of case 1 moves 87 → 83). 21 × 4 = 84 of the 87, and 11 × 4 = 44 of the 45 — so 3 and 1 points respectively are everything the tests' own shape contributes. A threshold of 25 permits six assertions per TEST_CASE, and case 1's prologue alone is eight. The split genuinely cannot reach the threshold; it would relocate the suppression into a shared helper and scatter the sequence that morph#296 is about. That is a measured argument, not a preference.

And the lane reported a finding against its own work: case 2's directive is inert for this branch's diff — deleting it changes nothing here — so it separately proved the directive is armed, over a one-line diff on that case's lambda-bearing assertion. Writing "this directive is armed rather than decorative, which the diff that introduced it did not by itself show" into the reason is exactly the standard the previous wording failed.

Not verified by me: the 87/45 measurements, the Catch2 3.5.3 reproduction, and the whole-file re-check of #656's 97 → 0 against the runner's Catch2 series — all need the Qt configure. Nor was bank_gui_tests built; the change is comment-only in one .cpp, which bounds the risk but is not the same as compiling it.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW

[Yaraslaut]

Runner: rebased onto 7a343e6f — rebase, not merge, and for the second limb of the test

This PR was green at 52/52, but #661 landed in the meantime and the staleness test comes out differently from the last several sweeps:

master-side delta since this branch's base:
  docs/spec/concurrency_and_lifetimes.md
  docs/spec/core/{backend,bridge}.md
  docs/spec/testing_strategy.md
  include/morph/core/bridge.hpp          <-- this one
  scripts/branch_partial_allowlist.json
  tests/bench/{CMakeLists.txt,bench_dispatch_allocations.cpp}
  tests/test_async_registration.cpp

and the sources this PR touches include it directly:

$ grep -rhoE '#include <morph/core/[a-z_]+\.hpp>' examples/bank/gui/ examples/bank/tests/gui/
#include <morph/core/backend.hpp>
#include <morph/core/bridge.hpp>
#include <morph/core/executor.hpp>

So this is "a source file it touches, or a header those include, changed → rebase. The combination was never tested." Not the reporting-only case that let #653, #654, #657 and #661 merge without one.

It matters more here than the rule's general form suggests. This PR's central claim is that eleven bank-GUI translation units now analyse to zero clang-tidy findings. #661 changed bridge.hpp — a defaulted constructor parameter and an extracted publishLateBindReply helper — and those eleven TUs compile against it. Whether the zero survives the new header is exactly what no run has measured: the green 52/52 was against the old one.

Rebased cleanly, two commits replayed unchanged, pushed as f498055f. Gates on the rebased tree, all green:

Q_OBJECT moc-pairing lint OK.
self-test OK: 9 fixture(s), including the #652 mutation of the real examples/common/CMakeLists.txt
tidy-suppression-scope OK
NOLINT directive lint OK: 177 NOLINTNEXTLINE directive(s), all annotating code.
bidi-control lint OK: 1246 text file(s) scanned, 12 codepoint(s) searched for, 0 raw occurrence(s).
ok: 15 structured citation(s) in scripts/mutation_survivors.json resolve to the line they name.
All 58 rung-filter checks passed.

Those are the cheap gates only. The one that matters — clang-tidy-diff over the eleven TUs against the new bridge.hpp — needs the Qt configure and is what the fresh run will answer. And per #666, a local run of it would not be trustworthy anyway: this workstation's Catch2 3.16.0 makes clang-tidy discard the TEST_CASE findings CI reports, so for this particular check CI is the only honest measurement available until that is pinned.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW