Repository navigation
ci: teach main's admission rules and classifier the arm PR leg - #704
Conversation
…as two exact variants The PR workflow's admit job runs main's admission rules on the pull request's own gke-pr.yml before any privileged leg runs. A PR that adds the arm64 leg (a build-arm job, an arm_relevant admit output, and matching legs and result checks) is refused by those rules, and the base checker's ci_docs_only.py does not publish arm_relevant, so the new result check would also fail with an unanswered classification. This change is the main-side half of that PR and does not touch gke-pr.yml: - ci_docs_only.py publishes arm_relevant (true for sources, build inputs and test scripts and for any path it cannot classify; false for established docs-only and .github-only changes), with its tests. - ci_preflight.py admits exactly two shapes of gke-pr.yml. The shape that has no build-arm job is checked by the unchanged rules. The shape that defines build-arm is checked by the full new rule set: the job inventory, the arm_relevant output, the pinned arm64 label, the path policy, the exact setup and Debug steps, and the complete legs and result gate bodies. The two sets are selected together by the job definition and never mixed, so a hybrid, a second arm job, an extra job in either shape, another label, or a container, services, credentials or timeout on the arm job match neither shape and are refused. In the arm shape, continue-on-error anywhere in the arm job and env, shell or working-directory on any arm step are refused too. - test_ci_preflight.py runs the real rules against main's current workflow and the same workflow with the arm leg applied, and asserts the exact findings of each refusal control. - docs/ci.md documents the classifier and both admission variants without changing the file's line count. Its rewritten sentences are named here, as the documentation check for unnamed sentence deletions (introduced by f85ce18) requires: "First merge the checker or allow-table change while the existing workflows pass." and "Then merge the workflow change that uses the newly admitted rule or label." and "Rule and table changes land in two steps." and "This order prevents one pull request from changing both sides of admission." are now one sentence (rule and table changes land first while existing workflows pass, workflow changes follow, so one PR cannot change both sides of admission); "The classifier uses exact `.github/` and `docs/` prefixes, so lookalike directories and the classifier itself cannot create a skip." and "Renames contribute both names; empty, missing, malformed, truncated or unreadable lists are neither class." are folded into the shorter paragraph that describes the path file and the exact prefixes, with the new arm_relevant output described beside it. Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.
Not checked: tests were not run; formal verification proved 0 of 6 changed function(s) (1 sampled, not proven, 4 not verified).
Formal verification. 1 change(s) alter behavior, breaking input(s) attached. PR-changed functions: 2/6 verified (0 proven, 1 may-equivalent, 1 distinguished) · 4 not verified (3 vacuous, 1 unsupported).
Behavior changes: classify\_paths changes behavior, here is the input that shows it.
The verifier found a concrete input on which classify\_paths behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"paths":"\['a', 'a'\]"\}, the old code produced \(False, False\) but the new code produces \(False, False, True\) (outputs first differ at character 14). Paste that input straight into a regression test.
Not verified on this run: classify (unsupported), decide (vacuous: never exercised), main (vacuous: never exercised), runner\_findings (vacuous: never exercised).
Graphify review — findings
Adds an arm_relevant output to the docs-only classifier, set true for source/build inputs under tests/, runner/, include/, src/ and docker/, C/C++ and CMake files, and scripts/test_*. It is also true for unknown paths and for empty, unreadable or malformed lists, and false only for established docs-only or .github/-only changes. The docs-only override can hide a source-named file under docs/, and a mixed docs/GitHub PR can still come out arm-relevant when a docs/ path has a source name. Tightens gke-pr.yml admission to exactly two variants. OLD keeps the current jobs, outputs and gate bodies, and defining build-arm selects the NEW checks together: the arm classifier output, a fixed Debug job on bare gke-16cpu-32gb-arm64, an arm-aware legs needs/check and an independent result check. Hybrids and extra jobs, outputs, needs or arm steps are refused, but the arm variant may drop the light-runner pins that gke_admit_workflow_findings normally enforces. Adds a separate arm job inventory (GKE_PR_ARM_JOB_LABELS, GKE_PR_ARM_JOB_COMMANDS) so main keeps its old table, and the preflight tests cover the new arm rules. docs/ci.md is rewritten to match, and it states that the unchanged main workflow neither publishes the output nor schedules a PR arm leg.
Worth a look
- Arm variant may drop the light-runner pins that gke-pr.yml normally enforces —
scripts/ci_preflight.py:1762· Escalate · medium- NOT verified (no proof, no reproducing execution) — needs human review
Review partial — this diff was larger than one review pass covers, so later files were not reviewed; some findings may be missing.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 882 functions depend on the 851 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
gke_admit_workflow_findings()— 14 callers, 11 callees - worse:
admit_rule_findings()— 7 callers, 9 callees - worse:
runner_findings()— 2 callers, 22 callees
Verification — 882 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 879 function(s) in the blast radius were not formally verified this run
Docs that may be stale (advisory)
benchmarks/results/selection.md§ Manifest (lines 50-255): references changed symbolsclassify
Formal verification
Behavior changes: classify\_paths changes behavior, here is the input that shows it.
The verifier found a concrete input on which classify\_paths behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"paths":"\['a', 'a'\]"\}, the old code produced \(False, False\) but the new code produces \(False, False, True\) (outputs first differ at character 14). Paste that input straight into a regression test.
Could not verify: Could not verify classify.
The verifier did not have enough to check classify, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: synthesized inputs (no suite coverage): POSSIBLE DIFFERENCE on 4 constructed input(s) — both versions returned but disagreed on the return (e.g. old=["tuple", [["bool", false], ["bool", false], ["list", [["str", "not docs-only: an unnamed event runs every job"]]]]], new=["tuple", [["bool", false], ["bool", false], ["bool", true], ["list", [["str", "not docs-only: an unnamed event runs every job"]]]]]); ADVISORY, unverified — a synthesized input may violate an unstated precondition, so this is NOT a sound refutation and does NOT gate the merge; confirm with a real test, or let carve/property refute on real inputs; property tier: not verifiable here: def-time exec: NameError (name 'ROOT' is not defined) — the function cannot be defined in the harness because a name its module provides (decorator / default / annotation / import) did not resolve in the harness namespace; a harness artifact, not evidence about the edit
Could not verify: Could not verify decide.
The verifier did not have enough to check decide, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: not verifiable here: def-time exec: NameError (name 'ROOT' is not defined) — the function cannot be defined in the harness because a name its module provides (decorator / default / annotation / import) did not resolve in the harness namespace; a harness artifact, not evidence about the edit
Could not verify: Could not verify main.
The verifier did not have enough to check main, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: not verifiable: all 12 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly NameError — names the real obstacle, not a sampling gap)
No difference found (not proven): No behavior difference found in gke\_admit\_workflow\_findings (not a proof).
The verifier ran both versions of gke\_admit\_workflow\_findings on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
Could not verify: Could not verify runner\_findings.
The verifier did not have enough to check runner\_findings, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: non-vacuity: domain too small (only 2 distinct inputs exercised, need 3) — 'no divergence' would be near-vacuous
· 1 grounded finding(s) anchored inline below; 2 more finding(s) on lines outside this diff (see the check run).
| return name == 'gke-pr.yml' and 'build-arm' in jobs | ||
|
|
||
|
|
||
| def gke_admit_workflow_findings(workflow: str, name: str = 'gke-pr.yml') -> list[str]: |
There was a problem hiding this comment.
gke_admit_workflow_findings()
fans out to 11 callees (efferent coupling); 14 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
The strict ruleset requires the branch to contain current main. The only change main gained since the branch base is README.md; no file of this branch is touched by the merge. Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
…n, trade text and RCI changes (#788) ## Summary This branch combines seven source heads into one change on top of main (`755e6658`), so that the build, the parity sweep and the review each run once. Every source head was merged with an ordinary merge commit: nothing was rebased, nothing was squashed inside the branch, no conflict occurred in these merges and nothing was resolved by hand. Every source head contains main (`755e6658`) except the trade text channel's, which contains the main before one documentation-only commit (`63f3978a`); the combined branch contains main. When this change merges, pull requests 781, 758, 780 and 783 are closed with a link to it. A moving average change (head `4c5c65f4`, "Compute SMA from a correctly rounded window sum") was merged into this branch between the trade text channel and #783 and is taken out again by the last commit of the branch, a plain revert of that merge (`git revert -m 1`, no conflict). The parity measurement of the branch with it showed 40 probes whose graded tier falls against their committed TradingView tapes, every one of them calling `ta.sma`; a measurement of main plus only that change gave the same 40 results, and no other probe changed. The change is not part of this merge; whether it returns, and in what form, is decided separately. The tree of this branch equals the tree of the seven heads below merged on main without it. PR type: code (engine sources, tests, CI scripts and documentation). Fixes #779 ## Sources | Source | Title | Source head | |---|---|---| | #781 | ci: validate split releases and retained smoke output | `130bda2db570bd63bdec57b6b5fdd3cabacab7fc` | | branch | Validate corpus records and gitlink results in the pull-request checks | `46bb21fb169091722d49d3b2f2e4f1f589148166` | | #758 | test(ci): register rerun-preempted controller tests | `348a89766e0ff23f88b4837bce80957f7635ef8e` | | branch | Add an opt-in trade text companion channel | `5930d7f8d12010d67c77a098804eb6cd295095db` | | #783 | Add generated CLI coverage for map pair limits | `163abf58f1a73c7ec9160120774954fb29f4bd2d` | | branch | Avoid copying RCI price history during ranking | `022750f8451aadd93066ffad3b54695e23956116` | | #780 | Make nested consumer state construction portable to Clang | `000e2dac0e8e5e73fe53551adec7fbdf3f251af6` | The head listed for the corpus validation change already contains the heads listed for #781 and #758, so merging it brings both along and the separate merge of the #758 head changes nothing (it is listed because its pull request closes with this change). The merges that this branch makes of the source heads resolve no conflict by hand. The source histories themselves carry six merges that were resolved by hand in their own branches, before the heads were handed over: three in the history of the corpus validation head and three in the history of the trade text channel head. Corpus validation: `d482eb37` (`scripts/test_ci_preflight.py`: the stage-inventory counts between the registration of the rerun-controller tests and the corpus validation job), `f7519bb8` (`scripts/ci_preflight.py` and `scripts/test_ci_preflight.py`) and `cecedc0e` (`CHANGELOG.md`, `ci/cr150-source-bounds.yaml`, `scripts/test_cr150_source_gate.py` and `tests/private_fixture_reads.evidence`). Trade text channel: `09c9fb52` (`CHANGELOG.md` and `runner/main.cpp`), `8c17d14e` and `5930d7f8` (both in `ci/cr150-source-bounds.yaml` and `scripts/test_cr150_source_gate.py`). The head listed for #758 is that pull request's branch merged once with main; the head published on #758 is older. The head listed for #780 is the head of its branch (the construction fixes and the refreshed source citations in three documentation pages). ## How it was built The merges were made in the order of the table, with the moving average change between the trade text channel and #783 (see above), except that the separate merge of the #758 head was skipped because the corpus validation head contains it. Relative to main, 52 files change. Besides the files that #781 and #758 share with the corpus validation head, three files are touched by more than one source and were merged automatically without a conflict: `docs/ci.md` (the corpus validation change and #783), `docs/design/native-feature-parity.md` (the trade text channel and #780) and `scripts/test_ci_preflight.py` (#781, #758, the corpus validation change and #783). For every source, the changed lines of the files that only it touches equal the changed lines of this branch, and for every shared file the union of the sources' changed lines equals the branch's; an independent chain of the same merges in the reverse order gives the same tree. docs/ci.md: the slow-label bound paragraph and the slow-label inventory paragraph, introduced by 69cddc9 (#679), now state the size of the pinned set (90 names, including test_cr559_map_capacity_cli) instead of the retired 43 + 28 = 71 breakdown. docs/ci.md: the three sentences of 42527b6 (#704) that said gke-pr.yml admits exactly two variants, that OLD is the shape without the arm job kept so earlier workflow files still pass, that NEW requires the exact arm classifier output, and that hybrids and extra jobs, outputs, needs or arm steps are refused while OLD retains its original exact rules and templates for a workflow without the arm job, are rewritten to cover the arm and the corpus variants. ## What was done before merge Hosted CI was green on this head. One parity sweep against the active baseline measured this exact tree: 7,989 probes compared, no probe changed tier, no leaver, no regression, and the gate returned its neutral pass. One review of the combined diff, which cites each source, and of the revert commit gave a GO on this head and tree. The results already recorded on the individual pull requests and branches are not repeated here and do not stand in for these. The moving average change is not in this merge: it was merged into this branch and taken out again by the last commit, a plain revert. The full post-merge workflow was also run on this head (run 42452226570). It failed exactly the twelve jobs that fail on main `755e6658` (run 42435140307): the eight sanitizer shards and the live-sanitizer job, each stopping at the final link with "No space left on device", and their aggregate jobs. Every other job passed, and no job failed that passes on main. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Main-side half of the arm PR leg. The GKE PR workflow runs main's admission rules and main's docs-only classifier against a pull request's workflows (pull_request_target), so a workflow change that adds a job can only pass once main already knows that job's exact shape. This change teaches main both halves, ahead of the workflow change itself:
Security: the new rules admit only those exact shapes; no wider pattern lets a later pull request add other jobs to the privileged workflow.
Known limit (follow-up): the admission entry does not yet check env, shell, working-directory or continue-on-error on build and build-debug.
Deferred to the workflow change that follows: the real-file arm proof tests, the actual-workflow allowlist and actionlint tests, the arm inventory and aggregate expansions, independent arm-read execution tests, and the docs rows for the build-arm leg itself.
🤖 Generated with Claude Code
Replaces #702 as one commit whose message names the six docs/ci.md sentences it removes (the docs-revert check judges each commit by its own message). Same tree as #702.