Repository navigation
Conversation
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
0b2c48a0· Trigger:schedule - Thinking:
280s wall
BLOCKING (3)
Root Cause
src/v2/00_core.dagNode updates are still hand-maintained record rebuilds → add a canonical identity-preserving Node reconstruction/update API or update every reconstruction in this PR with an explicit preserve/drop reason.src/v2/04_env.dagName lookup and binding-id lookup are modeled as separate payload-owning registries → make one declaration table authoritative and have alternate indexes store only keys/references.src/v2/04_env.dagBindingId freshness is local while environment merge assumes global uniqueness → namespace IDs by module or merge registries with duplicate-ID diagnostics.
| type Node { | ||
| name: String | ||
| ident: Int? | ||
| binding_id: BindingId? |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| type TypeDeclBinding { | ||
| binding_id: BindingId | ||
| name: String | ||
| resolved: Node |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
|
||
| fn merge_envs(envs: List<TypeEnv>) -> TypeEnv { | ||
| let merged_bindings = fold(envs, init: empty_map(), f: (acc, env) => map_merge(acc, env.bindings)) | ||
| let merged_decl_registry = fold(envs, init: empty_map(), f: (acc, env) => map_merge(acc, env.decl_registry)) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
66609328· Trigger:schedule - Thinking:
368s wall
BLOCKING (2)
Root Cause
src/v2/00_core.dagdeclaration identity is modeled as an unconstrained scalar instead of a nominal/freshness-checked carrier → make BindingId the boundary type everywhere and route construction through the allocator/diagnostic API.src/v2/04_infer.dagPD-3 compatibility exceptions are encoded as source-path heuristics instead of a typed, bounded deferral → remove the span skip or constrain it behind a documented trigger that still preserves fail-closed diagnostics.
| // Node -- unified recursive structure | ||
| // ========================================================================= | ||
|
|
||
| type BindingId = Int |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| let actual = peel_nominal_alias_identity(n: actual_raw, env: type_env, module_name: module_name) | ||
| if direct_call_arg_type_mismatch( | ||
| let arg_span = arg_value(n: ta).span | ||
| if direct_call_arg_span_skips(span: arg_span) { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Verified the current-head codex blockers against #4615 at 6660932. They are valid and I routed fixes to the worker. The diff still models declaration identity as raw Int (, , allocator returns Int), so BindingId is not yet the nominal/freshness-checked boundary type required by #4587. The diff also adds , a broad source-path heuristic bypass for PD-3 diagnostics; that must be removed or replaced with a typed, bounded deferral that preserves fail-closed diagnostics. #4615 remains draft until these are fixed, the PR body carries receipts, and parent signs off. — sent from bright-owl-653 |
|
Correcting shell-quoting damage in my prior comment. Verified the current-head codex blockers against #4615 at 6660932; they are valid and routed to the worker. The diff still models declaration identity as raw Int: type BindingId = Int; Node.binding_id: Int?; TypeDeclBinding.binding_id: Int; allocator returns Int. BindingId needs to be the nominal/freshness-checked boundary type required by #4587. The diff also adds direct_call_arg_span_skips, a broad source-path heuristic bypass for PD-3 diagnostics; that must be removed or replaced with a typed, bounded deferral that preserves fail-closed diagnostics. #4615 remains draft until these are fixed, the PR body carries receipts, and parent signs off. — sent from bright-owl-653 |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
e6ba8930· Trigger:schedule - Thinking:
392s wall
BLOCKING (1)
Root Cause
src/v2/04_env.dagTypeDeclBinding has no module-level declaration authority, only local intern key/name → add a canonical declaration key/span/module and only coalesce duplicate BindingId entries when that authority matches.
| Some { value: binding } => | ||
| match map_get(acc.decl_registry, binding_id) { | ||
| Some { value: existing } => | ||
| if existing.binding_key == binding.binding_key && existing.name == binding.name { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Verified the e6ba893 codex blocker against current #4615 head d2969e5. It is still valid: TypeDeclBinding still carries only binding_id, binding_key, name, and provenance, and merge_decl_registry_state coalesces duplicate BindingId entries when binding_key/name match. binding_key is still a spelling/intern key, not module-level declaration authority, so same-spelling declarations from different modules can be false-coalesced. This needs a canonical declaration authority key (module identity plus declaration span/source location, or equivalent closed declaration identity), and duplicate BindingIds should coalesce only when that authority matches exactly; otherwise fail closed with diagnostics. #4615 remains draft. — sent from bright-owl-653 |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
c1ab9014· Trigger:schedule - Thinking:
418s wall
BLOCKING (1)
Root Cause
src/v2/00_core.dagNode.binding_id uses None for both genuinely unbranded kernel nodes and missing/dropped declaration identity → split that state or make one-sided BindingId comparisons fail closed after lookup.
| Some { value: left_binding_id } => | ||
| match right.binding_id { | ||
| Some { value: right_binding_id } => left_binding_id == right_binding_id | ||
| None => authored_name_at(source_indices: source_indices, node: left) == authored_name_at(source_indices: source_indices, node: right) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Summary
Implements the #4587 brand-identity path for v2 type declarations:
BindingId,TypeDeclBinding, allocator state,Node.binding_id, andTypeEnv.decl_registryare the declaration identity channel. Declaration ids originate atbuild_type_envregistration, are preserved through structural resolution withwith_preserved_binding_id, and PD-3 compatibility compares declaration identity throughbinding_idinstead of spelling/ident_span.Phase-1 scope is intentionally single-module/user-module dogfood. Same-spelling cross-module behavior is guarded by an explicit witness; full phase-2 cross-module same-spelling claims remain out of scope.
Contract / Proof Notes
alloc_binding_idis the sole DAG mint path;register_type_decl_bindingstamps from declaration authority during type-env registration.pub struct BindingId(i64)with private inner value andpub(crate) fn mint;Node,TypeDeclBinding,decl_registry, duplicate tracking, and comparison paths carryBindingId, not rawInt/i64.lookup_type_forconsultsdecl_registry[binding_id].binding_keyand returns that name-keyed binding only after verifying it carries the requestedBindingId; otherwise it falls back tocarrier_bindings[binding_id]. Stamped nodes do not resolve by authored spelling.register_type_decl_bindingmints, stamps, and inserts the matchingdecl_registryrow in one accumulator update beforepre_local_envis merged; imported hidden carriers are collected from already-built parentTypeEnvs whose registries are merged intoimport_env;build_type_env_unresolveddoes not stamp localbinding_ids, so there is no stamped-local/empty-registry partial rollout.bindingsremains the visible/name-keyed namespace,decl_registryis the BindingId-to-declaration-authority table, andcarrier_bindingsis the hidden structural dependency carrier for non-visible imported dependencies. They coexist during phase 1 because visibility, declaration authority, and hidden transitive structure are distinct facts; stamped lookup checksdecl_registryfirst and validates the resolved binding carries the requestedBindingId, then usescarrier_bindingsonly for hidden dependencies, and never falls back to authored spelling for stamped nodes.merge_envsmerges declaration authority throughmerge_decl_registry_state, which coalesces only identicalTypeDeclAuthorityand records duplicate BindingIds as diagnostics.binding_keyremains lookup metadata only; stamping is not fed by spelling/name/intern key.TypeDeclBindingcarriesTypeDeclAuthority { module_name, decl_name, decl_span }; duplicateBindingIdmerge coalesces only when authority matches exactly, otherwise fails closed via duplicate diagnostics.with_authored_identity/ ident-span grafting is gone; structural rebuilds usewith_preserved_binding_id/node_with_preserved_binding_id.direct_call_arg_span_skipsis removed;module_skips_direct_call_arg_checkis hardfalse, with no path-prefix skip forsrc/v2,src/v3,src/v4,dsl/std,dsl/v4, ordsl/extdeps.Refinedheads carry nobinding_id; kernel container shells compare structurally and recurse into branded elements.node_type_compatibleleaf comparison fails closed when exactly one non-kernel side carriesbinding_id; same binding accepts only after cardinality gates pass.node_type_compatible/ PD-3.exact_structural_equality_zip_fold/Value::eqwere not edited, so the provenance A-pivot equality work is not entangled.Current-Head Execution Receipts
Current head:
d0d8819d539688ab7eddf7d0444d39431a4cd4f4.ctrl-build -- timeout 900s cargo run -p v2-compiler --bin regen_stage0->regen_stage0: wrote 68 generated stage0 files., zero hard diagnostics.ctrl-build -- timeout 600s cargo test -p v2-compiler-tests 'pd3_' -- --nocapture-> 18 passed, 0 failed. Includes the ported adversarial suite plusadv_same_named_decls_in_different_modules_must_stay_distinct.ctrl-build -- timeout 600s cargo test -p v2-compiler-tests infer_semantics -- --nocapture-> 44 passed, 0 failed.ctrl-build -- timeout 300s cargo test -p v2-compiler-tests anthropic_tool_result_content_rejects_legacy_string_slot -- --nocapture-> 1 passed, 0 failed.ctrl-build -- timeout 1200s cargo test -p v2-compiler-tests pipeline-> 336 passed, 24 failed, 41 ignored. Current main same command -> 335 passed, 24 failed, 41 ignored. Failure identity comparison: branch-only failures empty; main-only failures empty.ctrl-build -- timeout 1200s cargo test -p v2-compiler --lib-> 17 passed, 5 failed, 5 ignored. Current main same command -> 17 passed, 5 failed, 5 ignored. Failure identity comparison: branch-only failures empty; main-only failures empty.ctrl-build -- timeout 1800s cargo test --workspace --all-targetsboth fail onlyworkflow_no_path_regex_policy_ci_ymlbecause the test invokesgit ls-filesfrom a non-repo parent (/home/briansrls/.worktreeson branch,/tmpon main). New workspace failures empty by identity.Sole-Mint Proof
rg -n "BindingId \\{ value:" src/v2 -g '*.dag'-> onlysrc/v2/04_env.dag:58, insidealloc_binding_id.rg -n "BindingId::mint\\(" src/v2/stage0/src -g '*.rs' | rg -v 'v2_compiler_emit_rust.rs'-> onlysrc/v2/stage0/src/v2_compiler_infer_env.rs:73, the generated allocator.rg -n "\\bBindingId\\(" src/v2/stage0/src -g '*.rs' | rg -v 'v2_compiler_emit_rust.rs'-> onlypub struct BindingId(i64)andBindingId(value)inside the privatemintimplementation inv2_std_core.rs.From<i64>, raw constructor,as_binding_id, or raw diagnostic extractor.node_has_binding_idgrep hits are read-only comparison helpers, not construction.Earlier Focused Receipts Preserved
ctrl-build -- timeout 900s cargo test -p v2-compiler-tests pipeline::chained_type_alias_field_access -- --exact --nocapture-> 1 passed.ctrl-build -- timeout 900s cargo test -p v2-compiler-tests pipeline::indirect_type_alias_cycles_do_not_recurse_forever -- --exact --nocapture-> 1 passed.ctrl-build -- timeout 900s cargo test -p v2-compiler-tests diagnostics -- --nocapture-> 12 passed, 0 failed, 1 ignored.Readiness
Parent sign-off posted. Waiting on 2 dashboard approvals + CI green for operator merge. Do not merge from this worker session.