CI: gate the OFF half of the activation contract - #259
Conversation
#257 shipped the plug-and-play activation but tested it in only ONE direction, which I noticed after it merged. `cargo test --workspace` cannot exercise the off path: `ogar-ro` dev-deps `ogar-blockly`, so feature unification turns ogar-vocab's `blocks` feature ON for the entire workspace run. Every test asserting "activated" therefore ran, and nothing ever asserted "not activated" — the exact vacuous shape of a guard nobody watched stay silent. The one proof I had was a throwaway crate outside the workspace, which CI never sees. Two tests under `cfg(all(test, not(feature = "blocks")))`, so they VANISH when any activating feature is on rather than passing hollowly (measured: 2 tests off, 0 tests on). They assert a default build activates nothing, that 0x1717 answers UnknownClassid, that the domain still routes on the reserved byte alone, and that no 0x17XX row ever reached `class_ids::ALL` — the surface mirrored into lance-graph under the compile-time fuse, caught on THIS side first. Plus the CI step that can reach them: `cargo test -p ogar-vocab`, crate-scoped so `ogar-blockly` is not in the graph. The gate is verified to fail when it should: seeding one row into the `not(blocks)` arm of `activated_concepts` turned the first test red, then restored.
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_36464670-2839-496f-a720-9d3de097c8f3) |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6fe8845b7d
ℹ️ 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".
| # direction, which is the vacuous shape of a guard nobody watched stay | ||
| # silent. | ||
| - name: cargo test -p ogar-vocab (default features — the OFF half) | ||
| run: cargo test -p ogar-vocab |
There was a problem hiding this comment.
If blocks is accidentally added to [features] default, this command enables it, the module's cfg(all(test, not(feature = "blocks"))) removes both OFF tests, and Cargo still exits successfully after running the remaining suite; the exact regression this gate claims to prevent would therefore be invisible. Cargo documents --no-default-features as disabling the default feature, so explicitly verify that defaults contain no activating feature and/or force a no-default build while also checking that the named OFF tests actually ran.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct, and fixed in c530436. This is the hole I had actually named in a check-in note — "a gate that compiles out looks identical to success" — and then didn't defend against, so thanks for catching the gap between the note and the code.
Two defenses, because --no-default-features alone is necessary but not sufficient — it only forces the build a human remembered to write that way:
1. CI forces the off build and proves the tests ran. --no-default-features, plus a grep asserting both test names appear in the output, since a filtered-out suite exits 0 exactly like a passing one.
2. the_off_gate_cannot_be_switched_off — always compiled, in every feature configuration. It reads the crate's own Cargo.toml through include_str! and fails if any activating feature reached the default set. This is the part that cannot be forgotten or cfg'd away.
Falsified by simulating your exact scenario (default = ["blocks"]):
plain `cargo test -p ogar-vocab` -> exit 101, manifest guard fires:
`blocks` is in the DEFAULT feature set (default = ["blocks"]). Every build
would then carry that codebook, and the OFF-half gate would silently compile
out. Activation must stay opt-in, turned on by the consumer that owns it.
`--no-default-features` -> both OFF tests still RUN (feature forced
off), manifest guard still fails
Worth noting defense 2 closes the old command too, not just the new one — the regression is caught even by a plain cargo test -p ogar-vocab, which is what I actually wanted from "cannot disappear."
Then restored to default = []. OFF 156 pass / ON 158 pass / workspace 0 failures.
Generated by Claude Code
Codex is right, and it names the hole I had flagged in a check-in note
without actually closing: `default_build_carries_no_activated_rows` is
`cfg(not(feature = "blocks"))`, so putting `blocks` into
`[features] default` compiles it out, `cargo test -p ogar-vocab` exits
0, and the regression the gate exists to catch becomes invisible. A gate
that can disappear is not a gate.
Two defenses, because the obvious one is necessary but not sufficient:
1. CI now uses `--no-default-features`, forcing the off build regardless
of what `default` contains — and asserts the named tests actually RAN
by grepping the output, since a filtered-out suite exits 0 too.
2. `the_off_gate_cannot_be_switched_off` — ALWAYS compiled, in every
feature configuration. It reads the crate's own Cargo.toml via
`include_str!` and fails if any activating feature reached the
default set. Defense 1 only forces the build a human remembered to
write that way; this one cannot be forgotten or cfg'd away, and it is
what makes the property hold under the plain command too.
Falsified by simulating the exact regression (`default = ["blocks"]`):
- plain `cargo test -p ogar-vocab` -> exit 101, manifest guard fires
with the diagnostic naming the cause. Note this ALSO closes the old
step, not just the new one.
- `--no-default-features` -> both OFF tests still run (the feature is
forced off), and the manifest guard still fails.
Then restored; `default = []`.
OFF 156 pass / ON 158 pass / workspace 0 failures / clippy -D warnings
clean / fmt scoped with -p.
2 participants
Follow-up to #257, which shipped the plug-and-play activation but tested it in only one direction. I noticed after it merged.
The gap
cargo test --workspacecannot exercise the off path:ogar-rodev-depsogar-blockly, so feature unification turnsogar-vocab'sblocksfeature ON for the entire workspace run. Every test asserting "activated" therefore ran, and nothing ever asserted "not activated" — the exact vacuous shape of a guard nobody watched stay silent.The only proof I had of the negative half was a throwaway crate outside the workspace. CI never sees that.
The gate
Two tests under
cfg(all(test, not(feature = "blocks"))), so they vanish when any activating feature is on rather than passing hollowly:They assert a default build activates nothing, that
0x1717answersUnknownClassid, that the domain still routes on the reserved byte alone (so a consumer can branch on0x17XXwith no concept minted), and that no0x17XXrow ever reachedclass_ids::ALL— the surface mirrored into lance-graph under the compile-time fuse, caught on this side before it can break the mirror.Plus the CI step that can actually reach them:
cargo test -p ogar-vocab, crate-scoped soogar-blocklyis not in the graph.Verified to fail when it should
Seeding one row into the
not(blocks)arm ofactivated_concepts:Then restored. A gate that has never been observed failing is not a gate.
Verification
cargo test -p ogar-vocab(OFF) — 2 gate tests passcargo test -p ogar-vocab --features blocks(ON) — gate absent, 157 others passcargo test --workspace— 0 failurescargo clippy -p ogar-vocab --all-targets -- -D warnings— cleancargo fmtscoped with-pGenerated by Claude Code