No strings in the i4 gate hot path; record the GateDecision ordinal collision - #1045
Merged
Merged
Conversation
…ollision
gate_decision_i4 — i4-quantised qualia, the name says hot path —
allocated five Strings per call for GateDecision::{Hold, Block}
reasons. The module header above it already claimed 'no heap
allocation' and claimed this type carried &'static str; neither was
true. A separate scalar batch path exists precisely because the
Strings cannot be SIMD-packed, so the workaround was built before
the cause was removed.
The strings were also derived: each rendered the (TrustTexture,
FlowState) pair computed two lines earlier and already typed
Copy + repr(u8). Nothing in the workspace read the field.
GateDecision now carries that pair. It is Copy + PartialEq + Eq,
heap-free, every payload byte a repr(u8) enum. reason() renders the
prose on demand as &'static str, derived from the payload so it
cannot drift from the decision. The batch/scalar test asserts full
equality instead of the discriminant — a divergence in texture or
flow was previously masked.
Also in this commit, both discovered while auditing plan citations:
- ISS-GATEDECISION-ORDINAL-COLLISION: two live types named
GateDecision with inverted locked mappings (mul: 1=Hold 2=Block;
collapse_gate, the crate-root export: 1=Block 2=Hold). Recorded,
not resolved — realigning supersedes a documented lock and needs
an owner.
- elevation/cycle.rs cited KanbanMove::libet_offset_us, retired.
Corrected to libet_window_us with the pointer to
LIBET_COMMIT_WINDOW_US.
- plans/kognitionswirtschaft-v1.md marked NOT CANONICAL: historical
extracts re-imported against an architecture that had moved, with
citations never re-verified against their head. The staleness class
board/SUPERSESSION-INDEX.md measures.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HKHLL6NTnRr1XGvKFA1j2L
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_1a140ff4-a43a-4166-ae53-5aeabe2d0d13) |
|
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 |
CI clippy (-D warnings) failed on dead_code: the helper existed only because GateDecision carried String fields and could not be compared directly. With the typed Copy payload the batch/scalar test asserts full equality, leaving the helper unreferenced. The local gate that missed this was weaker than CI's: clippy was run without -- -D warnings, and the output filter looked for 'warning: unused' while rustc emits 'function ... is never used'. Verified with the CI invocation this time: cargo clippy -p lance-graph-contract --all-targets -- -D warnings exits 0, tests green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HKHLL6NTnRr1XGvKFA1j2L
AdaWorldAPI
marked this pull request as ready for review
August 26, 2026 07:57
…gate
The entry assumed collapse_gate's comment ('matches ndarray CollapseGate
ordinals') was true and concluded mul's locked mapping was what would
fall. Measured in the fork instead of taken on trust:
ndarray::hpc::qualia_gate::QualiaGateLevel Flow=0 Hold=1 Block=2 (repr(u8))
ndarray::hpc::bnn_cross_plane::CollapseGate Flow, Hold, Block
Both match mul::to_disc. collapse_gate is the side that disagrees with
the substrate, and its justification cites the substrate incorrectly —
the comment is the defect, not just the constants. deepnsm-v2 already
asserts mul's ordering against live evidence values.
Blast radius measured and recorded: all consumers read through
is_flow/is_hold/is_block or the named constants, no raw gate byte is
compared, none is persisted, and lance-graph-java names the type only
in a plan. Correction appended under storno; the original line stays.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HKHLL6NTnRr1XGvKFA1j2L
![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: 337425e5b5
ℹ️ 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".
Comment on lines
+162
to
+166
| Hold { | ||
| /// The trust texture that produced the hold. | ||
| texture: TrustTexture, | ||
| /// The flow state that produced the hold. | ||
| flow: FlowState, |
There was a problem hiding this comment.
Replacing the public reason field leaves existing repository consumers uncompilable: crates/lance-graph/examples/graph_self_reasoning.rs:237-239 still constructs GateDecision::Hold { reason: ... }. Cargo's own cargo test --help states that cargo test builds examples, so the mandatory cargo test --manifest-path crates/lance-graph/Cargo.toml gate will reject this change with an unknown-field error. A repo-wide search also finds unmigrated constructors in deepnsm-v2/src/evidence.rs and the cognitive-shader example; migrate those consumers or retain a compatible construction API.
AGENTS.md reference: AGENTS.md:L51-L59
Useful? React with 👍 / 👎.
| #[must_use] | ||
| pub const fn reason(&self) -> &'static str { | ||
| match self { | ||
| GateDecision::Flow => "calibrated trust in flow: full autonomy", |
There was a problem hiding this comment.
For inputs classified as (TrustTexture::Underconfident, FlowState::Transition), gate_decision_i4 returns Flow at lines 635-638, but this new method reports "calibrated trust in flow." Thus a decision produced by the crate itself can display both the wrong trust texture and the wrong flow state, contradicting the method's guarantee that its explanation cannot drift. Flow must retain enough typed payload to distinguish these cases, or its reason must avoid claiming a specific texture/state.
Useful? React with 👍 / 👎.
…ming a texture Both findings from the codex review on 337425e, verified before fixing. P1 — three constructors still used the removed field, and none was covered by the checks run before the first push: crates/lance-graph/examples/graph_self_reasoning.rs:237 crates/cognitive-shader-driver/examples/probe_revision_kanban_hinge.rs:803,806 crates/deepnsm-v2/src/evidence.rs:91,98 The gate that missed them was scoped wrong, not merely weak: '-p lance-graph-contract --all-targets' builds that crate's targets, not other crates' examples, and deepnsm-v2 is workspace-excluded so no -p invocation reaches it at all. Verified now per crate, examples included, plus deepnsm-v2 through its own manifest. deepnsm-v2's gate formatted the U value into the reason. That value is not carried into the decision: it belongs to the evidence that computed it, and uncertainty() hands it to any caller that wants to report it. The pair chosen for each site is one gate_decision_i4 would itself produce for that classification, so a hand-built decision cannot encode a state the classifier never emits. P2 — reason() claimed 'calibrated trust in flow' for Flow, but Flow is returned for (Calibrated | Underconfident) x (Flow | Transition): four input classes, and the text was wrong for three. Flow carries no payload, so its text now claims no specific texture or state. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HKHLL6NTnRr1XGvKFA1j2L
AdaWorldAPI
added a commit
that referenced
this pull request
Aug 26, 2026
…ground Plan the gate OUTCOME vs producer-GROUND separation (follow-up arc to #1045)
This was referenced Aug 26, 2026
AdaWorldAPI
pushed a commit
that referenced
this pull request
Aug 27, 2026
Discharges the second half of F-MUL-6 — the half a grep cannot reach. A classification tells you a symbol is mentioned; only a compiler tells you a change is source-compatible. ada-rs was COMPILED against the combined arc head (#1065 merged, plus #1066, #1067, #1068, #1069) using a temporary `paths` override, reverted afterwards. The override was VERIFIED to bind rather than assumed: a probe calling `KanbanColumn::veto()` — a method that exists only on the head under test — compiled without E0599. Without that check the entire gate could have run against `main` and reported a meaningless pass. Result: ada-rs does not compile, with exactly three errors, and all three are the pre-existing #1045 `reason:` break that predates every deliverable in this arc. Zero errors are attributable to the arc: - `PlannerContract::gate_check` removal -> ada-rs never implemented it - `MulProvider::gate_check` deprecation -> a warning by construction - `KanbanColumn::{advance, veto}` -> purely additive - D-MCAL-3 and D-MCAL-5 -> docs and tests only So the arc is source-compatible with its one live external implementor, and the single red consumer is red for a reason the arc exists to explain rather than one it caused. The stopgap stays unpushed. The three errors are trivially silenced by supplying a texture and a flow at each site, and that fix is refused: inventing two calibration coordinates ada-rs never measured would reproduce exactly the defect the census found in MedCare-rs. The honest fix is D-MCAL-4's route, which now exists on this head — a consent veto is domain evidence, so it calls `veto()` and constructs no `GateDecision` at all. Two of the adapter's three arms are genuine MUL and keep working through `assess()`; only the consent arm moves. MedCare-rs is covered at symbol level plus the in-tree pins rather than by a compile, and that asymmetry is recorded as a LIMITATION in the report rather than reported as a pass. The report's final section states what the gate does not certify at all, including the fifteen other dependent repos whose non-involvement rests on a grep — which, by this deliverable's own argument, is not a build. Board hygiene in the same commit: STATUS_BOARD row, INTEGRATION_PLANS prepend, supersession index regenerated last. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HKHLL6NTnRr1XGvKFA1j2L
This was referenced Aug 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rule applied: no strings in a hot path.
The allocation
gate_decision_i4— i4-quantised qualia, the name says hot path — allocated fiveStrings per call forGateDecision::{Hold, Block}reasons (3× Hold, 2× Block).Three things made it more than a missed optimisation:
(TrustTexture, FlowState)pair computed two lines earlier and already typedCopy + repr(u8). Searched the workspace for a read of the field: none. The type's own batch test compared discriminants only.Storing a second, unpackable projection of state that already exists in typed form is the defect; the allocation is the symptom.
The fix
The payload is the typed pair.
GateDecisionis nowCopy + PartialEq + Eq, heap-free, every payload byte arepr(u8)enum.GateDecision::reason()renders prose on demand as&'static str, derived from the payload so it cannot drift from the decision it describes.Matchers were unaffected — every call site already used
{ .. }. Four test constructors updated.Falsifier, both legs:
grep -c "to_string()" crates/lance-graph-contract/src/mul.rs→0textureorflowis observable. It was masked before.Age at removal: 6 days (
b67f195, PR #969 — the commit whose message declares an "intentionally stupid substrate … all semantics as interpretations layered above it"). Prose reasons inside the substrate's own decision type are semantics baked in, not layered above.Recorded, deliberately not fixed:
ISS-GATEDECISION-ORDINAL-COLLISIONTwo live types are named
GateDecision, with inverted locked mappings:mul::GateDecision::to_disccollapse_gate::GateDecision.gatelance_graph_contract::GateDecisionat the crate root (lib.rs:211) is the collapse_gate one;kanban.rs:26imports the mul one. Anything packing one and reading the other swaps hold and block — caution becomes rejection.Not fixed here because realigning supersedes a documented lock: that is a decision with an owner, not cleanup. The asymmetry worth noting for whoever takes it — ndarray is the substrate, so if one of the two is wrong it is the one disagreeing with the substrate.
Two stale citations, corrected
elevation/cycle.rsnamedKanbanMove::libet_offset_usas the live Libet carrier, twice. That field is retired (contract::kanbansays so in as many words); magnitude isLIBET_COMMIT_WINDOW_US, direction is carried by the transition. Corrected with the pointer.plans/kognitionswirtschaft-v1.mdmarked ⊘ NOT CANONICAL: historical extracts re-imported against an architecture that had since moved, citations never re-verified against their head — it names the retired field as live and diagnoses "the MUL gate" throughmul::GateDecisionwithout noting the collision above. Exactly the staleness classboard/SUPERSESSION-INDEX.mdmeasures: plan-mentions outliving the code. Regenerated the index after the edit — byte-identical, so the plan header does not perturb it.Left open, deliberately
Holdis the_ =>catch-all and 3 of 5 match arms reach it. Whether a state that fires on most inputs is a state or a default with a name is a distribution question over the qualia space — unmeasured, and not answerable by inspection. Flagged in the board entry, not decided here.Gates
cargo fmt,cargo clippy -p lance-graph-contract --all-targetsclean,cargo test -p lance-graph-contract1220 lib + all suites green,cargo check -p lance-graph-plannerclean.Generated by Claude Code