contract: resolve ISS-V1-TAIL-RESIDUE — un-gate mint_for, default-on guid-v3-tail, aiwar mints V3#663
Merged
Conversation
… mints Executes the structural half of ISS-V1-TAIL-RESIDUE (operator ruling 2026-07-04, "default-on guid-v3-tail now so production mints route through mint_for in every build"). - canonical_node.rs: `mint_for` moved to an unconditional `impl NodeGuid` — V1 arm always available; V2/V3 arms feature-gated (`guid-v2-tail`) with a dead V1 fallback so `--no-default-features` still compiles. - Cargo.toml: `default = ["guid-v3-tail"]` — every normal build has the V3 mint path. - ocr.rs + aiwar.rs: route through `mint_for(classid_read_mode(c).tail_variant, …)` instead of hardcoded `NodeGuid::new`. ocr is classid-param-driven (V3- ready). aiwar stays on the V1 `CLASSID_OSINT` (see below), behavior-preserving. - .claude/commands/v3-audit.md: check #6 forbids `NodeGuid::new(` in non-test production code (mechanical guard). Discovered blocker (documented, deferred): flipping aiwar to `CLASSID_OSINT_V3` broke `projects_to_family_node_graph` — the V3 tail puts `family` at bytes 12..14 (u16) but `soa_graph::project_snapshot` reads it via the V1 `family()` u24 accessor. So aiwar's V3-classid flip is now gated on making that consumer tail-aware (I-LEGACY-API-FEATURE-GATED), not on the feature gate the issue originally named. ISSUES.md carries the progress + remaining work. Verification: 854 contract lib tests green (default); `--no-default-features` compiles; `lance-graph` + `lance-graph-planner` check clean (default-on propagates safely); fmt + clippy clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MLBnPuScZy6w9di2QEjsXM
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_ce8a3e4a-1ac6-4c8c-8921-6c21a7586a7a) |
📝 WalkthroughWalkthroughThis PR makes ChangesV1-tail-residue migration
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/lance-graph-contract/src/aiwar.rs (1)
112-125: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winClarify the V3 note in
crates/lance-graph-contract/src/aiwar.rs:112-125:mint_fordoes not narrowfamily/identityautomatically; the V2/V3 arm asserts both fit in 16 bits and panics otherwise, so the& 0x00FF_FFFFmasks must be tightened if this path ever flips.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/lance-graph-contract/src/aiwar.rs` around lines 112 - 125, The `NodeGuid::mint_for` call in `aiwar.rs` is annotated as if V3 would automatically narrow `family` and `identity`, but the `V2/V3` path actually asserts 16-bit fit and will panic if those values exceed it. Update the comment and the masking logic around the `mint_for` invocation so it reflects the real behavior, and tighten the `fam`/`identity` masks to 16 bits if this path can ever switch to V3.
🧹 Nitpick comments (2)
crates/lance-graph-contract/src/ocr.rs (1)
104-132: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRedundant
classid_read_mode(classid)lookup.
classid_read_mode(classid)is invoked twice — once at Line 105 for.value_schemaand again at Line 124 for.tail_variant— for the sameclassidwithin the same call. SinceReadModeappears to carry both fields, cache the result once and reuse it.♻️ Proposed refactor
pub fn to_node_row(&self, classid: u32, identity: u32) -> NodeRow { - let schema = classid_read_mode(classid).value_schema; + let mode = classid_read_mode(classid); + let schema = mode.value_schema; let mut value = [0u8; VALUE_SLAB_LEN]; @@ key: NodeGuid::mint_for( - classid_read_mode(classid).tail_variant, + mode.tail_variant, classid,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/lance-graph-contract/src/ocr.rs` around lines 104 - 132, The to_node_row method is doing the same classid_read_mode(classid) lookup twice for one call. Cache the returned ReadMode once at the start of to_node_row, then reuse its value_schema and tail_variant fields when building the NodeRow. This keeps the logic in to_node_row unchanged while removing the redundant lookup.crates/lance-graph-contract/src/aiwar.rs (1)
75-131: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winHoist
classid_read_mode(osint_classid)out of the per-row closure.
osint_classidis loop-invariant (fixed at Line 81), butclassid_read_mode(osint_classid).tail_variant(Line 117) is re-resolved inside.map()on every row. Hoist it once before the.map()call to avoid a redundant lookup per graph node.♻️ Proposed refactor
let view = AiwarClassView::from_graph(graph); let ids = graph.all_node_ids(); + let tail_variant = classid_read_mode(osint_classid).tail_variant; let fam_of = |id: &str| -> Option<u32> { graph.node(id).and_then(|n| view.family_of(&n.label)) }; ids.iter() .enumerate() .map(|(i, id)| { @@ key: NodeGuid::mint_for( - classid_read_mode(osint_classid).tail_variant, + tail_variant, osint_classid,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/lance-graph-contract/src/aiwar.rs` around lines 75 - 131, The per-row closure in the `ids.iter().enumerate().map(...)` pipeline is re-resolving `classid_read_mode(osint_classid).tail_variant` for every `NodeRow`, even though `osint_classid` is ثابت and invariant. Hoist the `classid_read_mode(osint_classid)` result (or at least its `tail_variant`) once before the `.map()` call in `aiwar.rs`, then reuse that value inside `NodeGuid::mint_for` so the lookup happens only once per batch instead of once per node.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@crates/lance-graph-contract/src/aiwar.rs`:
- Around line 112-125: The `NodeGuid::mint_for` call in `aiwar.rs` is annotated
as if V3 would automatically narrow `family` and `identity`, but the `V2/V3`
path actually asserts 16-bit fit and will panic if those values exceed it.
Update the comment and the masking logic around the `mint_for` invocation so it
reflects the real behavior, and tighten the `fam`/`identity` masks to 16 bits if
this path can ever switch to V3.
---
Nitpick comments:
In `@crates/lance-graph-contract/src/aiwar.rs`:
- Around line 75-131: The per-row closure in the
`ids.iter().enumerate().map(...)` pipeline is re-resolving
`classid_read_mode(osint_classid).tail_variant` for every `NodeRow`, even though
`osint_classid` is ثابت and invariant. Hoist the
`classid_read_mode(osint_classid)` result (or at least its `tail_variant`) once
before the `.map()` call in `aiwar.rs`, then reuse that value inside
`NodeGuid::mint_for` so the lookup happens only once per batch instead of once
per node.
In `@crates/lance-graph-contract/src/ocr.rs`:
- Around line 104-132: The to_node_row method is doing the same
classid_read_mode(classid) lookup twice for one call. Cache the returned
ReadMode once at the start of to_node_row, then reuse its value_schema and
tail_variant fields when building the NodeRow. This keeps the logic in
to_node_row unchanged while removing the redundant lookup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5b4fb986-befb-4ad7-ba34-da9ccfeae001
📒 Files selected for processing (6)
.claude/board/ISSUES.md.claude/commands/v3-audit.mdcrates/lance-graph-contract/Cargo.tomlcrates/lance-graph-contract/src/aiwar.rscrates/lance-graph-contract/src/canonical_node.rscrates/lance-graph-contract/src/ocr.rs
…AIL-RESIDUE resolved) Completes the migration: aiwar now mints the real V3 `CLASSID_OSINT_V3` in every normal build (V1 fallback only under `--no-default-features`). The mid-work diagnosis in the prior commit was WRONG and is corrected here. `soa_graph`'s read path is already tail-aware (`family_of`/`identity_of` route through `classid_read_mode(...).tail_variant` → `family_v2`/`identity_v2` for V3; proven by `v3_rows_decode_family_and_identity_via_tail_variant`). The actual blocker was a one-line domain-spec pin: `OSINT_GOTHAM.classid = CLASSID_OSINT` (V1), and `project_snapshot` filters rows by EXACT classid, so V3-minted aiwar rows were dropped (empty snapshot). Consumers were V3-ready; the straggler was the domain constant. - soa_graph.rs: `OSINT_GOTHAM.classid` -> `CLASSID_OSINT_V3` (via a cfg-selected `OSINT_GOTHAM_CLASSID`, V1 fallback for `--no-default-features`). The `node()` test helper mints via `mint_for` so its rows carry the domain's tail layout. - aiwar.rs: mint `CLASSID_OSINT_V3` (cfg fallback), u16 masks; test asserts the V3 classid. - ISSUES.md: ISS-V1-TAIL-RESIDUE marked RESOLVED with the corrected diagnosis. Verification: default build 854 lib tests green (V3), `--no-default-features` 840 green (V1 fallback), fmt + clippy clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MLBnPuScZy6w9di2QEjsXM
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_355a4716-ab6d-46ed-9488-c48dcfc9927c) |
AdaWorldAPI
changed the title
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.
What
Resolves
ISS-V1-TAIL-RESIDUE— the V1→V3 GUID-tail migration for the two production mint sites, plus the substrate wiring that unblocks it.Changes (2 commits)
44c67d12— un-gate + default-on + routecanonical_node.rs—mint_formoved to an unconditionalimpl NodeGuid: V1 arm always available, V2/V3 arms feature-gated with a dead V1 fallback so--no-default-featuresstill compiles.Cargo.toml—default = ["guid-v3-tail"].ocr.rs+aiwar.rs— route throughmint_for(classid_read_mode(c).tail_variant, …); no hardcodedNodeGuid::newleft..claude/commands/v3-audit.md— check feat(graph): add SPO triple store with bitmap ANN, TruthGate, semirin… #6 forbidsNodeGuid::new(in non-test production code.4bc25ba2— complete the aiwar V3 flipsoa_graph.rs—OSINT_GOTHAM.classid→CLASSID_OSINT_V3(cfg-selected, V1 fallback for--no-default-features); thenode()test helper mints viamint_for.aiwar.rs— mints the realCLASSID_OSINT_V3in every normal build.Corrected diagnosis (recorded in ISSUES.md)
Commit 1 had claimed the aiwar V3 flip was blocked because
project_snapshotreadsfamilyvia the V1family()accessor. That was wrong.soa_graph's read path is already tail-aware (family_of/identity_ofroute through the classid'stail_variant→family_v2/identity_v2for V3; a test proves it). The actual blocker was a one-line domain-spec pin —OSINT_GOTHAM.classid = CLASSID_OSINT(V1) — andproject_snapshotfilters rows by exact classid, so V3-minted rows were dropped. Consumers were V3-ready; the straggler was the domain constant. Commit 2 fixes it.Verification
--no-default-features(V1 fallback): 840 green.lance-graph+lance-graph-plannercheck clean; fmt + clippy clean.Follow-up (separate, non-blocking)
CLASSID_OSINT(V1,0x0700_0000) stays a registered legacy alias — retirement is corpus-proof-gated (W6), not part of this issue.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes