CI: jeder Job auf dem Pin, jedes Member gegated, ein Ort pro Version - #984
Conversation
All seven workflows installed `stable` and set it as the rustup default. For the jobs that run cargo inside the repo that was merely misleading — `rust-toolchain.toml` (channel = 1.97.1, components rustfmt + clippy) wins over `rustup default`, so they already ran on the pin while the YAML said otherwise, and a `matrix.toolchain: [stable]` that nothing else referenced looked like a version axis while selecting nothing. For two jobs it was not merely misleading. `release.yml` and `rust-publish.yml` pass `toolchain: stable` to `actions-rust-lang/setup-rust-toolchain@v1`, which pins the environment to that input — so the SHIPPED artifact and the published crates were built with whatever `stable` was that week, not with the channel this repo pins. The fix deliberately does NOT restate `1.97.1` in seven files. `rust-toolchain.toml` says why in its own comment — "a bump edits `channel` and leaves the prose behind" — and seven restatements are seven places for that to happen. Instead: - `rustup toolchain install stable && rustup default stable` (and the matrix variant) → `rustup show`, which installs and activates whatever the file pins, including its components. - the two action-based setups → drop the `toolchain:` input so the action reads `rust-toolchain.toml`. That is the shape `style.yml:146` already uses for its rustfmt job, so this follows existing practice here rather than importing a convention. Result: one file names the version; every job follows it; a bump stays one edit. Verified locally: all seven parse under `yaml.safe_load`, and no `toolchain: stable` / `rustup default stable` / `toolchain install stable` remains anywhere under `.github/workflows/`. NOT verified locally, and it cannot be: whether the runner behaves as intended. GitHub Actions does not run in this sandbox, so the first real evidence is the first CI run on this branch — the `rustup show` step prints the selected toolchain, which is what to read there.
Follow-up to the toolchain commit on this branch, from the same measurement.
No workflow here runs `--workspace` or `--all`; every gate names one crate by
path. So `[workspace] members` gates nothing, and ELEVEN of 25 members were in
no job. Two of them (catalog, planner) are deps of lance-graph, so their libs
compiled inside a gated build while their tests ran nowhere; the rest compiled
nowhere at all. That is how `lance-graph-hydrate` reached `main` un-compiling —
`object_store 0.13.2`, a semver-compatible release already in the lockfile,
moved `get`/`put` onto `ObjectStoreExt`, and nothing could see it.
Every step added here was RUN LOCALLY on the pinned 1.97.1 first, and the count
in each comment is what it returned. Nothing is armed on hope:
catalog 12+15 · planner 368+4 · ontology 276+2+6 · rbac 23 · archetype 16
consumer-conformance 8 · sigma-tier-router 20 · neural-debug 11
cognitive-shader-driver 107+2 · hydrate 33
`lance-graph-benches` gets `cargo check --benches` instead of `cargo test`: it
is a benches-only crate with one `harness = false` target, so `cargo test` runs
zero tests and would be a gate that cannot fail. Same shape build.yml already
uses for lance-graph's own benches.
Formatting gates for the same ten. Nine were already rustfmt-clean and simply
had nothing holding them there; `sigma-tier-router` was NOT — 14 hunks in one
file, formatted here, rustfmt output only, its 20 tests unchanged.
Two things this deliberately does NOT do:
- No clippy gates for these crates. Not measured crate-by-crate, and
`causal-edge` alone already carries 7 pre-existing findings
(ISS-CAUSAL-EDGE-CARRIES-SEVEN-PRE-EXISTING-CLIPPY-FINDINGS). Arming a lint
gate on unmeasured crates fails PRs for defects they did not introduce.
- No `--workspace` job. That would catch every FUTURE member automatically and
is the better long-run answer, but it is a scope and cost decision, not a
repair. This closes the measured gap; the standing choice stays open.
Also here, because a PR that adds a gate must leave CI green: the two-line
`ObjectStoreExt` import that makes `lance-graph-hydrate` compile, its rustfmt
pass (4 files), and its live `trim_split_whitespace` clippy warning. Those same
fixes ride with the crate's own PR (claude/hydrate-from-zip) — identical hunks,
so a rebase after this merges drops them cleanly. Without them the hydrate gate
added here would be red on arrival.
Cargo.lock is NOT in this commit. It gained one line (`blake3`) from advancing
the OGAR sibling checkout #265 → #279, which is a deliberate dependency bump
and not part of a CI change.
Verified: all seven workflows parse under `yaml.safe_load`, and every one of
the 25 workspace members now resolves to at least one job.
Background: EPIPHANIES E-THE-GATE-IS-A-HAND-MAINTAINED-ALLOWLIST-NOT-THE-WORKSPACE-1
and ISSUES ISS-CI-GATE-IS-AN-ALLOWLIST-NINE-MEMBERS-UNGATED, both landing with
claude/hydrate-from-zip.
TWO CHANGES, one theme: the things that must not drift were declared in eight
places, and the gate that would have caught a drifting member did not exist.
[workspace.dependencies] — 33 declarations, 8 member manifests
`arrow`, `datafusion`, the lance family and `object_store` were spelled a dozen
ways across eight members (`arrow = "58"`, `{ version = "58" }`,
`{ version = "58", optional = true }`, …). A bump was eight edits and an
inconsistent one stayed invisible until a resolve produced two majors.
Members now inherit with `workspace = true` and keep their own `optional` and
`features`; `default-features = false` moved into the central entry for the
two crates that need it (`datafusion`, `lancedb`), since every member already
wanted it off.
WHAT THIS DOES NOT DO, stated because it is the thing most likely to be
misread: it does not pin the transitive closure. Cargo has no recursive
version pin — `[workspace.dependencies]` centralises OUR declarations, `=x.y.z`
pins only edges we declare, and `[patch]` redirects a source rather than a
version graph. What `lancedb` pulls in as ITS datafusion is governed by
lancedb's requirement. The artifact that freezes the whole graph is
`Cargo.lock` — which is why this workspace commits 29 of them, and why FLOAT
AND FIX (MedCare-rs, OGAR) does not transfer here.
The falsifier that matters: `Cargo.lock` is BYTE-IDENTICAL after the change.
Centralising moved the declarations, not the resolution. `cargo metadata`
exits 0; the resolved pins are unchanged (lance 9.0.0, lancedb 0.33.0,
datafusion 54.1.0, arrow 58.3.0, object_store 0.13.2) and datafusion is a
single major — deltalake was the only thing that pulled 53 and #962 removed it.
Two grep traps deliberately left alone: `lance-graph-planner`'s
`datafusion = []` and `holograph`'s `lancedb = ["dep:lance"]` are FEATURE
names, not dependencies. And `[workspace.dependencies]` reaches members only,
so the excluded crates (`holograph`, `lance-graph-python`, …) keep their own
declarations by necessity, not by oversight.
cargo build --workspace as the structural gate
Every other gate here names one crate by path, so `[workspace] members` gates
nothing — which is how `lance-graph-hydrate` reached `main` un-compiling. This
step needs no maintenance: a member added tomorrow is covered the moment it is
listed.
Measured on a clean tree with the manifest's `debug = 0`: exit 0, 6m18s,
target/ 3.5 GB.
It gates the COMPILE, not the tests, and the comment in the workflow says
plainly why that is provisional rather than principled: an earlier
`cargo test --workspace --no-run` hit ENOSPC, but that ran against a target
already holding 15 GB of debug-laden artifacts from builds predating the
manifest default — it measured the polluted tree, not the test scope. An
earlier draft of this commit cited that number as if it were a property of the
test scope; it is not, and the claim is removed rather than softened. When a
clean-tree measurement exists, the ten per-crate test steps collapse into one.
…of assumed
The previous commit landed `cargo build --workspace` as the structural gate and
said openly that the build-not-test choice was provisional: the only evidence
against a test-scope job was an ENOSPC that had measured a polluted 15 GB target
rather than the test scope itself.
Measured now, clean tree, manifest `debug = 0`:
cargo build --workspace exit 0, 6m18s, target/ 3.5 GB
cargo test --workspace --no-run exit 0, target/ 14 GB, 86 binaries
The test scope SUCCEEDS. What rules it out is not failure but size: 14 GB is the
same order as a GitHub runner's free disk, against 3.5 GB for the compile. So
the choice stands, for a reason that is now a measurement rather than an
artifact of my own polluted tree.
Also recorded, because it is a risk this branch INTRODUCED rather than found:
the ten per-crate test steps run in this same job and accumulate their test
binaries into the same target. They are a subset of those 14 GB, so this job's
disk headroom is not proven — only the workspace compile is. If it ever fails on
disk, the fix is to split those tests into their own job, not to drop the gate.
…my check could not see TWO THINGS, and the second is a correction to this branch's own claim. Own job, preventively Everything this branch added to `test` — the workspace compile gate plus the per-crate steps for members that had none — accumulates test binaries into one `target/`. `test` is a job with a MEASURED history at exactly that cliff; its own env block records "a hard `ld` SIGBUS (signal 7 = object file truncated when the runner partition fills mid-link)". Measured here on a clean tree with the manifest's `debug = 0`: `cargo build --workspace` costs 3.5 GB; `cargo test --workspace --no-run` costs 14 GB across 86 binaries — the same order as a runner's free disk. Adding a subset of that to a job already known to have filled its partition is a bet with no upside, so `test` goes back to exactly its pre-branch disk profile and every addition moves to `member-tests` with its own runner and its own cache key (sharing `lance-graph-deps` would have two jobs writing different contents under one key). The claim "every member is gated" was FALSE when I made it The member check extracted with `"crates/[a-z0-9-]+"` — no underscore. So `crates/surreal_container` was never in the list it checked, and `tools/dto-class-check` is not under `crates/` at all. Two of eighteen members were invisible to the check that reported full coverage, and the "25 members" figure in earlier messages came from the same broken extraction (it counted comment lines in a `sed` range). A membership check that cannot see two of its inputs is the same shape as a gate that cannot fire — the exact defect class this branch exists to close, one level up, in my own instrument. Both now gated, both measured first: `surreal_container` 5 + 5 green, `dto-class-check` 1 green. `surreal_container` was also rustfmt-dirty (6 hunks in one test file, formatted here, tests unchanged). Re-checked with a parser that reads the `members` array instead of a line range: 18 members, none without a job.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_0129dbd3-8ffd-4699-a562-cb7cee085b49) |
`build.yml`, `rust-test.yml`, `rust-publish.yml` and `style.yml` all set `RUSTFLAGS: "-C debuginfo=1 -C target-cpu=x86-64-v3"` at WORKFLOW level. RUSTFLAGS are appended after the flags cargo derives from the profile, so that `debuginfo=1` OVERRIDES `[profile.dev] debug = 0` in Cargo.toml — the manifest default was silently defeated in CI, which is the opposite of what it is for. Four jobs already carried a job-level `debuginfo=0` for exactly this reason, measured in b56bb2c: ~930 MB -> ~252 MB per binary at link, -73%. Three did NOT, and inherited the `=1`: style.yml clippy compiles the whole tree style.yml format rust-publish.yml publish builds what gets published clippy in particular compiles everything and never opens a debugger. Now `debuginfo=0` at workflow level in all four. The four jobs with an explicit job-level `=0` keep the identical string, so their Swatinem cache keys are unchanged; the three that inherited `=1` get a new key and repopulate once. `RUST_BACKTRACE=1` still yields a backtrace, without line numbers. NOT changed, and both deliberately: - lance-graph's own Dockerfiles build `--release` and this workspace declares no `[profile.release]` at all, so cargo's default (`debug = false`) already applies. There is nothing to strip there. - MedCare-rs's Railway image is a different question and is left to its owner: its `[profile.release]` sets `debug = "line-tables-only"` + `strip = "none"`, and its Dockerfile states why — "line-tables-only debug info is intentionally kept for symbolicating crash dumps". That is a documented reason, which is exactly the carve-out in the standing rule ("debug 0 by default unless there are reasons"), so it is surfaced rather than stripped.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ee3e7069dd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| run: rustup show | ||
| rustup component add clippy |
There was a problem hiding this comment.
Restore a multiline shell block for toolchain setup
Every Style Check run fails before reaching Clippy because YAML folds these lines into the single command rustup show rustup component add clippy. Checking rustup show --help shows the syntax rustup show [OPTIONS] [COMMAND], and executing the folded command returns error: unrecognized subcommand 'rustup'; use a run: | block or remove the now-redundant component command. This also prevents the dependent formatting job from running.
AGENTS.md reference: AGENTS.md:L51-L55
Useful? React with 👍 / 👎.
| - name: Run dto-class-check tests (missed by the first sweep) # 1 green | ||
| run: cargo test --manifest-path tools/dto-class-check/Cargo.toml |
There was a problem hiding this comment.
Trigger member gates for changes under tools
For a pull request that modifies only tools/dto-class-check, this new test never runs: the pull_request.paths list in this workflow includes crates/**, the root Cargo files, and the workflow itself, but no tools/**. The Style Check workflow has the same omission for its newly added dto formatting step, so such a PR can merge without either new gate and only discover failures after the unconditional push-to-main run; add the tool path to both workflow filters.
Useful? React with 👍 / 👎.
This reverts commit 85d5f46.
Live CI failure on this branch:
Run rustup show rustup component add clippy
error: unrecognized subcommand 'rustup'
The toolchain commit replaced a two-line `rustup toolchain install stable` /
`rustup default stable` block with the single-line form `run: rustup show`. In
`style.yml`'s clippy job that block had a THIRD line — `rustup component add
clippy` — which the replacement did not match. Left behind under a single-line
scalar, YAML folds the two into one command.
Now a block scalar with both lines. The extra `component add` is redundant with
`rust-toolchain.toml`'s `components = ["rustfmt", "clippy"]`, which `rustup
show` already installs; it stays because it is idempotent and because removing
it would be a second change in a step that just broke.
WHY MY CHECK MISSED IT, since that is the more useful half: I validated with
`yaml.safe_load`, which proves the file PARSES. `run: rustup show` followed by
an indented line is perfectly valid YAML — a folded plain scalar. It parses to
the wrong command. Parseability is not correctness, and a check that can only
see the former will pass a broken workflow every time.
The check is now on the parsed VALUE: it walks every step's `run`, splits it
into lines, and fails on any line carrying two `rustup` invocations. Run across
all seven workflows: the six other `rustup show` steps are single-line and
clean, and clippy's is the intended two.
Rebasing onto `main` after #984 merged dropped this branch's duplicate `ObjectStoreExt` / rustfmt / clippy hunks — they are in `main` now — but left its BOARD entries describing a world that no longer exists. Correcting them before they land, rather than landing false statements and fixing them after: - `ISSUES.md` `ISS-CI-GATE-IS-AN-ALLOWLIST-NINE-MEMBERS-UNGATED` is RESOLVED. The outcome is recorded ABOVE its original text, which is kept verbatim — it was accurate when filed, and an entry rewritten to match the present hides what was known when. - `LATEST_STATE.md` said `lance-graph-hydrate` is "gated for the first time" here and that "eight more members are still ungated". Both were true when written and are false now. - `EPIPHANIES.md` `E-THE-GATE-IS-A-HAND-MAINTAINED-ALLOWLIST-NOT-THE-WORKSPACE-1` gets an appended outcome paragraph; its analysis is untouched. What #984 actually did, against what these entries predicted: BOTH of the two options they offered — a workflow line per member (each measured locally before its gate was armed) AND `cargo build --workspace` as the net that covers future members without a line. And the count in all three was wrong: eleven ungated members, not nine. The check behind it extracted with `"crates/[a-z0-9-]+"` — no underscore — so `crates/surreal_container` was never in the list, and `tools/dto-class-check` is not under `crates/` at all. That is carried forward in the entries rather than quietly corrected, because a membership check blind to two of its inputs is the same defect class those entries describe, one level up: in the instrument instead of the workflow. Still open, unchanged: no `cargo test --workspace` job. Measured at 14 GB across 86 binaries versus 3.5 GB for the compile — the same order as a runner's free disk. A new member's TESTS still need a line. Verified after the rebase and the conflict resolution (both conflicts were the same import, resolved to `main`'s already-merged wording): hydrate 39 tests green, clippy `--all-targets -D warnings` clean, rustfmt clean.
Ausgelöst durch einen konkreten Befund:
crates/lance-graph-hydratekompilierte aufmain(#981) nicht.object_store 0.13.2— eine semver-kompatible Version, die längst im Lockfile stand — hatget/putauf einen Extension-Trait verschoben. Das Crate war zusätzlich fmt-schmutzig und trug eine lebende Clippy-Warnung.Die interessante Frage war nicht der Fehler, sondern warum ihn nichts sehen konnte.
Der Mechanismus
Kein Workflow in diesem Repo läuft
--workspaceoder--all. Jedes Gate nennt genau ein Crate per Pfad —build.yml×1,rust-test.yml×14,style.yml×9.style.yml:152sagt es selbst in einem Kommentar übercargo fmt --all. Ein Crate in[workspace] memberseinzutragen fügt es damit keinem Gate hinzu.Elf von achtzehn Membern waren in keinem Job. Zwei davon (
catalog,planner) sind Deps vonlance-graph, ihre Libs kompilierten also innerhalb eines gegateten Builds — ihre Tests liefen nirgends. Der Rest kompilierte nirgends.Ledger:
EPIPHANIES.mdE-THE-GATE-IS-A-HAND-MAINTAINED-ALLOWLIST-NOT-THE-WORKSPACE-1,ISSUES.mdISS-CI-GATE-IS-AN-ALLOWLIST-NINE-MEMBERS-UNGATED(landen mitclaude/hydrate-from-zip).Was der PR ändert
1 — Jeder Job auf der gepinnten Toolchain. Alle sieben Workflows installierten
stable. Für Jobs, die cargo im Repo ausführen, war das nur irreführend (rust-toolchain.tomlschlägtrustup default, sie liefen längst auf 1.97.1). Fürrelease.ymlundrust-publish.ymlnicht: die übergebentoolchain: stableanactions-rust-lang/setup-rust-toolchain@v1, und dieser Input pinnt die Umgebung — das ausgelieferte Artefakt und die publizierten Crates wurden mit dem gebaut, wasstablein der jeweiligen Woche war.Die Version steht bewusst nicht siebenmal im YAML;
rust-toolchain.tomlwarnt in seinem eigenen Kommentar davor („a bump editschanneland leaves the prose behind"). Stattdessenrustup show, und bei den zwei Action-Setups der weggelassenetoolchain:-Input, damit die Action die Datei liest — das Muster, dasstyle.yml:146hier schon benutzt.2 — Jedes Member gegated. Test- und fmt-Schritte für die elf, plus
cargo build --workspaceals strukturelles Netz, das keine Pflege braucht: ein morgen hinzugefügtes Member ist abgedeckt, sobald es in[workspace]steht.3 —
[workspace.dependencies]. 33 Deklarationen aus 8 Manifesten, in einem Dutzend Schreibweisen, auf eine Stelle.4 — Eigener Job für die Ergänzungen (
member-tests), präventiv.Gemessen, nicht behauptet
Jeder Test-Schritt wurde vorher lokal auf 1.97.1 gefahren; die Zahl im Kommentar ist, was er zurückgab. Kein Gate ist auf Hoffnung scharfgestellt:
Der Beleg für
[workspace.dependencies]ist nicht der Diff, sondern dassCargo.lockbyte-identisch blieb. Zentralisiert wurden die Deklarationen, nicht die Auflösung: lance 9.0.0, lancedb 0.33.0, datafusion 54.1.0, arrow 58.3.0, object_store 0.13.2 — unverändert, und datafusion bei genau einem Major (deltalake war das Einzige, das 53 zog, und #962 hat es entfernt).Warum das Workspace-Gate baut statt testet, auf sauberem Baum mit
debug = 0:target/cargo build --workspacecargo test --workspace --no-runDer Test-Scope scheitert nicht — er ist zu groß. 14 GB ist die Größenordnung des freien Plattenplatzes eines Runners.
Der eigene Job, und warum präventiv
testträgt in seinem eigenenenv-Block die Erinnerung an genau diesen Abgrund: „a hardldSIGBUS (signal 7 = object file truncated when the runner partition fills mid-link)". Meine Ergänzungen akkumulieren ihre Test-Binaries in dasselbetarget/und sind eine Teilmenge jener 14 GB.testhat deshalb wieder exakt sein Disk-Profil von vor diesem Branch; alles Neue liegt inmember-testsmit eigenem Runner und eigenem Cache-Key (lance-graph-depszu teilen hieße, zwei Jobs schreiben unterschiedliche Inhalte unter einen Schlüssel).Zwei Korrekturen an diesem PR selbst
Die Behauptung „alle Member sind gegated" war falsch, als sie gemacht wurde. Mein Prüf-Regex war
"crates/[a-z0-9-]+"— ohne Unterstrich.crates/surreal_containerwar nie in der Liste,tools/dto-class-checkliegt nicht untercrates/. Zwei von achtzehn waren für die Prüfung unsichtbar, die Vollständigkeit meldete; die „25 Member" früherer Fassungen stammten aus derselben Extraktion. Eine Mitgliedsprüfung, die zwei ihrer Eingaben nicht sieht, ist dieselbe Defektklasse wie ein Gate, das nicht feuern kann — eine Ebene höher. Beide sind jetzt gegated, nachgeprüft mit einem Parser über dasmembers-Array: 18 Member, keiner ohne Job.Die erste Begründung für Build-statt-Test war ein Artefakt. Ein früheres
cargo test --workspace --no-runlief in ENOSPC — gegen einentarget/, der bereits 15 GB Debug-Ballast aus Läufen vor dem Manifest-Default trug. Das hat den verschmutzten Baum gemessen, nicht den Test-Scope. Die Entscheidung steht, aber jetzt auf der Messung oben.Nicht enthalten, bewusst
causal-edgeallein trägt sieben vorbestehende Befunde (ISS-CAUSAL-EDGE-CARRIES-SEVEN-PRE-EXISTING-CLIPPY-FINDINGS). Ein Lint-Gate auf ungemessenen Crates lässt PRs für Defekte scheitern, die sie nicht verursacht haben.Cargo.lock. Er gewann eine Zeile (blake3) durch das Nachziehen des OGAR-Siblings feat: ScenarioBranch facade + LF integration mapping v1 #265 → feat: DeepNSM grammar parser — Markov ±5 bundler, role keys, thinking styles #279 — ein bewusster Dependency-Bump, kein Teil einer CI-Änderung.[workspace.dependencies]zentralisiert unsere Deklarationen,=x.y.zpinnt nur unsere Kanten,[patch]lenkt eine Quelle um. Waslancedbals sein datafusion zieht, bestimmt lancedb. Der rekursive Pin ist die Lockfile — deshalb die 29 hier, und deshalb überträgt sich die Regel aus MedCare-rs/OGAR nicht.Kollateral, benannt
sigma-tier-router(14 Hunks) undsurreal_container(6 Hunks) waren rustfmt-schmutzig und sind hier formatiert — reine rustfmt-Ausgabe, Tests unverändert (20 bzw. 5+5). Sie waren schmutzig, weil nichts sie dort hielt.Dazu der
ObjectStoreExt-Zweizeiler, derlance-graph-hydratewieder kompilieren lässt, plus dessen fmt-Lauf und Clippy-Warnung — sonst wäre das hydrate-Gate rot bei Ankunft. Dieselben Hunks reiten mitclaude/hydrate-from-zip; ein Rebase danach lässt sie sauber wegfallen.Was offen bleibt
Ein
--workspace-Test-Job statt der Einzelschritte, sobald jemand misst, dass der Runner die 14 GB hält — dann fallen zwölf Schritte zu einem zusammen. Und die Platten-Reserve vonmember-testsist nicht bewiesen, nur die des Workspace-Compiles; wenn der Job an der Platte scheitert, ist die Reparatur eine weitere Aufteilung, nicht das Fallenlassen des Gates. Beides steht so im Workflow.Generated by Claude Code