Skip to content

ci: validate split releases and retained smoke output - #781

Closed
luisleo526 wants to merge 3 commits into
mainfrom
ci/main-admission-1108
Closed

luisleo526 wants to merge 3 commits into
mainfrom
ci/main-admission-1108

Conversation

@luisleo526

Copy link
Copy Markdown
Collaborator

CI admission currently rejects the split hosted release layout and smoke
output collection. Accept the complete reviewed split release layout alongside
the legacy layout, and allow each smoke job's existing four steps with an
optional final output collector. Preserve credential isolation, main-only
toolchain access, runner permissions, ordered build and artifact checks, and
explicit repository context for release uploads.

Freeze and hash the legacy and split workflow fixtures so rollback coverage
survives the later workflow changes. When release dispatch inputs change,
update the admission template, split fixture, and its pinned SHA-256 together
in the same commit.

Validation: the prior checker failed the intended corrected-layout and
mutation controls; the candidate passed all 539 preflight and 289 release-gate
tests plus the source contract check. Real actionlint 1.7.12 accepted three
valid data roots and rejected the repository-context and credential mutations.
The upload command's repository context was checked with real gh against a
synthetic loopback API using two repository inputs. Hosted CI, the full fast
tier, and real release execution remain separate validation steps.

The actual workflow cutovers follow in separate changes.

🤖 Generated with Claude Code

Preserve the legacy release form while pinning split architecture jobs and
their verified handoff. Accept the existing four smoke steps or the exact
fifth output collector so the workflow can change after these rules land.
Pin split dispatch and fixture identities, add credential-withdrawal controls,
and retain immutable legacy inputs. Require repository-bound uploads and
main-only OIDC callers while preserving four-step smoke compatibility.
Comment on lines +11898 to +11899
'gke-pr.yml build-debug must retain exactly its four smoke steps '
'and optional pinned final output collector',

@graphify-labs graphify-labs Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Graphify reviewed this change.

Worth a look — the grounded gate found no coupling regressions or blocking issues, but 3 advisory finding(s) below merit a look before merge.

Not checked: tests were not run; formal verification proved 0 of 4 changed function(s) (1 sampled, not proven, 1 not verified).

Formal verification. 2 change(s) alter behavior, breaking input(s) attached. PR-changed functions: 3/4 verified (0 proven, 1 may-equivalent, 2 distinguished) · 1 not verified (1 vacuous).

Behavior changes: gke\_admit\_workflow\_findings changes behavior, here is the input that shows it.

The verifier found a concrete input on which gke\_admit\_workflow\_findings 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 \{"workflow":"'a'","name":"''"\}, the old code produced \[': trigger must be exactly pull\_request\_target', ': trigger must restrict branches to main', ': trigger types are incomplete or reordered', ': run\-name must bind the PR number and head SHA', ':… but the new code produces \[': trigger must be exactly pull\_request\_target', ': trigger must restrict branches to main', ': trigger types are incomplete or reordered', ': run\-name must bind the PR number and head SHA', ':…. Paste that input straight into a regression test.

Behavior changes: release\_prebuilt\_workflow\_findings changes behavior, here is the input that shows it.

The verifier found a concrete input on which release\_prebuilt\_workflow\_findings 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 \{"workflow":"'\\\\t\\\\n'"\}, the old code produced \['release\.yml prebuilt must retain needs: release before build', 'release\.yml prebuilt must retain workflow permissions: contents: write', 'release\.yml prebuilt must retain job permissions: conte… but the new code produces \['release\.yml prebuilt must retain exactly release, prebuilt and notify\-hub jobs', 'release\.yml prebuilt must retain needs: release before build', 'release\.yml prebuilt must retain workflow perm… (outputs first differ at character 37). Paste that input straight into a regression test.

Not verified on this run: runner\_findings (vacuous: never exercised).


Graphify review — findings

Adds preflight rules for a split release workflow (prebuilt-x86, prebuilt-arm, upload) with pinned runners and a narrowly scoped registry-reader token exception for the x86 job. Tightens the GKE PR smoke check so each job must match the reviewed four-step body, with an optional pinned final always-upload output collector. Because any split job name switches runner_findings off the legacy prebuilt matrix and Eigen checks, a partial or mixed split attempt loses those pins, and malformed five-step smoke jobs or missing reviewed split steps can fail the check instead of reporting findings.

Worth a look

  • Adding any split job name disables the legacy prebuilt matrix, Eigen and runner pins — scripts/ci_preflight.py:3253 · Escalate · medium
    • NOT verified (no proof, no reproducing execution) — needs human review
  • Malformed five-step GKE smoke job can crash instead of returning findings — scripts/ci_preflight.py:2014 · Escalate · medium
    • NOT verified (no proof, no reproducing execution) — needs human review
  • A missing reviewed step may crash the split-release check instead of producing a finding — scripts/ci_preflight.py · 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 — 955 functions depend on the 927 functions this change touches.

Health — this change adds coupling hotspots:

  • worse: ci_workflow_findings() — 88 callers, 24 callees
  • worse: gke_admit_workflow_findings() — 18 callers, 12 callees
  • worse: _job_values() — 33 callers, 3 callees
  • worse: admit_rule_findings() — 8 callers, 9 callees
  • worse: release_prebuilt_workflow_findings() — 6 callers, 10 callees
  • worse: _job_block() — 20 callers, 3 callees
  • worse: runner_findings() — 2 callers, 27 callees
  • worse: _holds_credentials() — 5 callers, 7 callees
  • …and 5 more — each is listed as a finding

Verification — 955 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: 955 function(s) in the blast radius were not formally verified this run

Docs that may be stale (advisory)

…and 10 more.

Formal verification

Behavior changes: gke\_admit\_workflow\_findings changes behavior, here is the input that shows it.

The verifier found a concrete input on which gke\_admit\_workflow\_findings 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 \{"workflow":"'a'","name":"''"\}, the old code produced \[': trigger must be exactly pull\_request\_target', ': trigger must restrict branches to main', ': trigger types are incomplete or reordered', ': run\-name must bind the PR number and head SHA', ':… but the new code produces \[': trigger must be exactly pull\_request\_target', ': trigger must restrict branches to main', ': trigger types are incomplete or reordered', ': run\-name must bind the PR number and head SHA', ':…. Paste that input straight into a regression test.

Behavior changes: release\_prebuilt\_workflow\_findings changes behavior, here is the input that shows it.

The verifier found a concrete input on which release\_prebuilt\_workflow\_findings 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 \{"workflow":"'\\\\t\\\\n'"\}, the old code produced \['release\.yml prebuilt must retain needs: release before build', 'release\.yml prebuilt must retain workflow permissions: contents: write', 'release\.yml prebuilt must retain job permissions: conte… but the new code produces \['release\.yml prebuilt must retain exactly release, prebuilt and notify\-hub jobs', 'release\.yml prebuilt must retain needs: release before build', 'release\.yml prebuilt must retain workflow perm… (outputs first differ at character 37). Paste that input straight into a regression test.

No difference found (not proven): No behavior difference found in release\_credential\_findings (not a proof).

The verifier ran both versions of release\_credential\_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

· 6 grounded finding(s) anchored inline below; 7 more finding(s) on lines outside this diff (see the check run).

Comment thread scripts/ci_preflight.py
'''


def gke_admit_workflow_findings(workflow: str, name: str = 'gke-pr.yml') -> list[str]:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Health regression — gke_admit_workflow_findings()

fans out to 12 callees (efferent coupling); 18 callers depend on it (afferent coupling).

Grounded coupling-delta finding (deterministic), not an LLM guess.

Comment thread scripts/ci_preflight.py
return [step.rstrip('\n') for step in _steps(template)]


def _release_split_job_findings(job: str, body: str) -> list[str]:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Health regression — _release_split_job_findings()

fans out to 6 callees (efferent coupling).

Grounded coupling-delta finding (deterministic), not an LLM guess.

Comment thread scripts/ci_preflight.py
return findings


def _release_split_toolchain_reader_is_pinned(workflow: str, body: str) -> bool:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Health regression — _release_split_toolchain_reader_is_pinned()

fans out to 9 callees (efferent coupling).

Grounded coupling-delta finding (deterministic), not an LLM guess.

Comment thread scripts/ci_preflight.py
return not _holds_credentials(workflow, token_only_body, strict=True)


def _release_split_workflow_findings(workflow: str) -> list[str]:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Health regression — _release_split_workflow_findings()

fans out to 8 callees (efferent coupling).

Grounded coupling-delta finding (deterministic), not an LLM guess.

Comment thread scripts/ci_preflight.py
return findings


def release_prebuilt_workflow_findings(workflow: str) -> list[str]:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Health regression — release_prebuilt_workflow_findings()

fans out to 10 callees (efferent coupling); 6 callers depend on it (afferent coupling).

Grounded coupling-delta finding (deterministic), not an LLM guess.

'gke-pr.yml: admission workflow is missing'])


def main_admission_data_findings(workflows):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Health regression — main_admission_data_findings()

6 callers depend on it (afferent coupling).

Grounded coupling-delta finding (deterministic), not an LLM guess.

luisleo526 added a commit that referenced this pull request Nov 8, 2026
…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)
@luisleo526

Copy link
Copy Markdown
Collaborator Author

This change is merged as part of #788 (squash commit 0c8e1a8), which combined it with the other sources listed there. Closing in favour of that merge.

@luisleo526 luisleo526 closed this Nov 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant