Repository navigation
Conversation
|
Verified against the current branch. The |
|
Verified the codex blocking finding against commit |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
567ad2aa· Trigger:schedule - Thinking:
240s wall
BLOCKING (2)
Root Cause
src/v4/extdeps/languages/rust.dagRustIntegerRangeBound.value is introduced as a target-spec fact, but the canonical Node projection has no value-carrying edge → preserve bound values structurally in rust_integer_range_node or keep the range-axis scaffold gated until that carrier exists.src/v3/compiler/tests/integration/v4_compiler_emit_translate_smoke_test.rsThe PR expands the SG-0 hand-Rust test surface without pairing it to the P5 receipt authority → add exactly one receipt in the PR body or the required planning/census artifact.
| children: [ | ||
| rust_named_edge( | ||
| name: rust_facts_field_range_min, | ||
| target: rust_inhabitant_atom(id: facts.range_min.identity) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| ); | ||
| } | ||
|
|
||
| #[test] |
There was a problem hiding this comment.
BLOCKING: INVARIANTS P5 requires a PR-body or changed-planning-artifact receipt for expanded hand-written Rust tests under src/v3; the provided diff adds new tests here but no receipt artifact accompanies them.
567ad2a to
6f632ee
Compare
|
Review metadata
1. Story of the diffThis PR turns 2. Invariant categories
Finding — coproduct dissolution / substrate modeling discipline:
Finding — Practice 4 / Practice 7 / Practice 10: same blocking issue.
Compliant — the Rust-side additions are confined to tests, use free helper functions (
Compliant — the PR adds behavior-named ratchets in the existing v4 parse-surface smoke harness, with separate tests for record-field shape, row population, bound-value projection, and range-axis binding. Given the current v4 single-file harness constraints, parse-surface structural tests are the right level for this change.
Compliant — the diff does not alter a locked design doc; it removes the
Finding — untracked scaffold shape: if 2.5. Top-down PM intent reviewCompliant with one blocking modeling-discipline exception above. The PM-level intent is preserved in direction: the PR removes the 3. VerdictREQUEST_CHANGES — the PR’s core direction is right, and the range axis is now wired into the Rust primitive fact bundle. The blocker is the new unclassified |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
2bc17ee5· Trigger:schedule - Thinking:
200s wall
BLOCKING (1)
Root Cause
src/v4/extdeps/languages/rust.dagRust range bound values were introduced as an enumerated substrate carrier without deciding whether the numeric value set is terminal, gated, or should dissolve into std integer/range structure → add the required carrier tag with a valid disposition, or dissolve the carrier now.
| value: RustIntegerRangeBoundValue | ||
| } | ||
|
|
||
| type RustIntegerRangeBoundValue |
There was a problem hiding this comment.
BLOCKING: RustIntegerRangeBoundValue is a new substrate coproduct with 19 variants but no required 🟢/🟡/🔴 dissolution classification, violating modeling-discipline Practice 4 / INVARIANTS P5.
|
Superseded by consolidated PR #3635 (variant-parameterization sweep + integer range facts) |
Worker attestation
node://adhoc-76f6bf85-988, not a GitHub issue.Summary
Discharges the #3500 deferred
primitive_fact_axis_rangegap for the Rust v4 language model.RustIntegerPrimitiveFactsnow carries per-primitiverange_min/range_maxsource facts throughRustIntegerRangeBound { identity: Symbol, value: RustIntegerRangeBoundValue }, so each bound has a stable bound identity and a structural Rust Reference value.The integer primitive bundle binds
primitive_fact_axis_rangethrough a dedicated range node. That node forwards each bound viarust_integer_range_bound_node, and forwards the bound value viarust_integer_range_bound_value_node; the value fact is therefore part of the canonicalNodeprojection instead of being dropped at the ModelCore binding boundary.The
primitive-fact-axis-range-unboundscaffold marker is removed frommodel_core.dagbecauserust.dagnow provides the named source carrier and axis binding. The earlier codex blocking reviews on opaque symbols / dropped bound values are addressed by replacing bare Symbol/String sidecars with a structural range-bound value coproduct and by changing the test ratchet to assert value projection.P5 receipt
Expanded hand-written Rust coverage is confined to the existing
src/v3/compiler/tests/integration/v4_compiler_emit_translate_smoke_test.rsv4 parse/smoke harness. The receipt authority is this PR body plus the deletedprimitive-fact-axis-range-unboundscaffold marker insrc/v4/std/model_core.dag: the new tests are the narrow ratchet for that scaffold deletion and assert the modeled.dagcarrier/projection shape, not new compiler behavior undersrc/v3/production source.Test plan
cargo fmt --check— passcargo test -p v3-compiler v4_rust_integer --test integration— pass, 4/4cargo test -p v3-compiler v4_std_model_core --test integration— pass, 7/7ctrl-build -- bash -lc 'cargo build -p v2-compiler --release && bash scripts/v4-bootstrap-viability.sh'— fails on rebasedorigin/mainbefore this PR's Rust range surface:java.dag/lean.dagimport missingfree_monoid_foldfromv4.std.algebra; this PR does not touch those files.cargo test -p v3-compiler— fails in remote runnertest_runner::execute_command_timebound_testsbecausegunbc_execute_command_bootstraphelper is missing; focused in-scope tests above pass.