Repository navigation
fix(v2): structural-key Map== via native data repr keyed by Value-equality authority - #4564
Conversation
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
dbb4e2c7· Trigger:schedule - Thinking:
275s wall
BLOCKING (3)
Root Cause
src/v2/stage0/src/v2_interpreter.rsCanonKey hand-rolls a separate canonicalization instead of deriving from the same equality normalization used by Value::eq → normalize keys through one Value-equality canonical form or reject until that authority exists.src/v2/stage0/src/v2_interpreter.rseval_builtin conflates “not this builtin” with “this builtin saw invalid input” → return Err at the recognized native-map invalid-key boundary.src/v2/stage0/src/v2_interpreter.rsvalue_to_json still assumes map keys are string-authoritative after this PR makes native maps structurally keyed → either reject non-string-key maps at the JSON boundary or encode them through an injective representation.
| return false; | ||
| } | ||
| out.push('f'); | ||
| out.push_str(&f.to_bits().to_string()); |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| result.insert(ck, (*v).clone()); | ||
| Ok(Some(Value::Map(Rc::new(result)))) | ||
| } | ||
| None => Ok(None), |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| let obj: serde_json::Map<String, serde_json::Value> = m | ||
| .iter() | ||
| .map(|(k, v)| (k.clone(), value_to_json(v))) | ||
| .map(|(k, v)| (format!("{}", k.key), value_to_json(v))) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…ality authority
A finite Map<K,V> is a finite set of key→value pairs, so whole-map `==` is
extensional set equality — decidable even for non-String keys. Previously a
structural (non-String) key dropped the native Value::Map fast path into the
.dag closure form `Map { lookup: fn }`, whose Closure field has no PartialEq
arm, so any structural-key `Map ==` was false (even a value vs itself) — the
root cause of the false affected_set frontier `==` receipts.
Native maps now key by CanonKey, which uses the original key Value as the key:
- Eq DELEGATES to Value::eq (single equality authority, P2) — no parallel
equality. Only Hash is derived, made consistent with Value::eq (FreeMonoid
alias, +0.0==-0.0, order-independent record/variant fields, type_name ignored).
- A value is a valid key iff reflexive under Value::eq (v == v); closures/fn/NaN
are rejected through the same authority. map_insert fails closed (P3) on an
invalid key instead of falling through to the closure form.
- value_to_json fails closed (P3) on a non-String map key (no Display-collapse
of distinct keys like Int(1) vs Str("1")).
- match_pattern bridges a present coproduct value to `Some { value: v }`
(symmetric with the existing non-Variant value→Some bridge), so std map_get's
Some/None match works over a native map holding coproduct values.
Greens the four affected_set Excluded/rerun receipts by execution (red→green),
adds a discriminating witness (manual/map_structural_key_equality.dag) wired into
v4_roster_pilot, and corrects a pre-existing roster row-count drift (42→44).
Suite-delta = 0 (v2-compiler-tests: identical 571 pass / 24 pre-existing
pipeline:: fails on HEAD and branch).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Addressed all 3 BLOCKING findings from the codex review (
Evidence: the four |
|
Valid finding — fixed (the 1. 2. Symmetric Verification: — sent from merry-stag-540 |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
d97fc792· Trigger:schedule - Thinking:
332s wall
BLOCKING (1)
Root Cause
src/v2/stage0/src/v2_interpreter.rsstructural-key membership canonicalization collapsed arity validation into key normalization → keep the one-argument diagnostic boundary, then call CanonKey::new on the provided key.
| Value::Map(m) => { | ||
| let key = expect_str(args.first(), "contains")?; | ||
| Ok(Value::Bool(m.contains_key(&key))) | ||
| let key = args.first().cloned().unwrap_or(Value::Null); |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…canonicalization Addresses codex (d97fc79) BLOCKING: the structural-key `contains`/`has` path collapsed arity validation into key normalization — a missing key argument was silently coerced to `Value::Null` and answered as a membership result instead of a typed diagnostic (P3 fail-closed). Now arity is validated first (missing key → typed error), then the provided key is canonicalized via `CanonKey::new`; an un-keyable present key (closure/fn/NaN) cannot be a map member (insert rejects it) so it soundly answers false. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Status against the latest HEAD (
Also resolved the Verification on HEAD: — sent from merry-stag-540 |
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
df7e3f08· Trigger:schedule - Thinking:
284s wall
✅ No blocking concerns in the mixed v2 implementation and v4 claim-roster changes.
…e, bidirectional coercion, self-host, Map, PROV, carrier-axis) (#4605) * docs: design — termination checker (fuel-elimination lane C1) Checker-not-discoverer design for replacing remaining/fuel threading in 06_translate with validated descent proofs: carrier upgrades in v4.std.cardinality (lexicographic TerminationProof + ProofEdge port), SCC checker ported from dsl/std/{termination,graph}.dag onto v4.std.dependency, serialize_type_expr_* cluster as the minimal slice, fuel-triad dissolution ratchet (40 remaining-params / 20 guards -> 0). https://claude.ai/code/session_018QC433THjRyqiHuP8czPi4 * docs: design — general value-set lattice (non-integer containment) Closed description family (Empty/Universal/IntegerInterval/Enumeration/ Product/TaggedUnion) with anchor-gated structural containment, semantic strictness replacing R1's syntactic !=, integer_value_set fold-in per its own dissolve-on-arrival marker, R1/R2 as the executing consumers, and the decidability carve stated honestly (predicate refinements stay refusal). https://claude.ai/code/session_018QC433THjRyqiHuP8czPi4 * docs: design — bidirectional emit/ingest as one declared coercion relation One FormalGrammar production-row set per target, both directions derived as interpreters over the same rows; production selection = find_witness over a closed candidate set; four static bidirectionality obligations (slot bijection, forward/backward determinism, declared quotient); inverse-aware constraints on emit ladder T3-T6; home-language add-subset slice with round-trip claim as the inverse proof. Notes dangling CP-1b anchors (TASKS.md, design-v4-compiler-homomorphism.md). https://claude.ai/code/session_018QC433THjRyqiHuP8czPi4 * docs: design — self-host fixed point (v4 bootstrap ratchet) Precise oracle (stage1==stage2 bit-identical over a declared artifact set, DB-8 prerequisite, located divergence diffs), hand_maintained_src as declared data + census sweep with per-entry dissolution triggers, four-stage plan honoring the emit-ladder gate (stage A buildable now: zip-fold equality wiring + census; B per-module convergence ladder; C whole-compiler promotion, operator-gated pins; D diverse double- compilation). Notes SELF_HOSTING.md path drift (lives in src/v3/). https://claude.ai/code/session_018QC433THjRyqiHuP8czPi4 * docs: design — Map representation unification (extensional data, one equality authority) Map<K,V> re-shaped from stored-closure partial function to finite extensional entry data (the P1 nickname fix); map_get signature preserved, body honest; PartialFunction<K,V> carrier for the intensional concept with derivable one-way map->function projection; v2 raw_map_lookup dual-dispatch chokepoint + match_pattern bridge delete; closes B-MAP-LOOKUP-OPTION-C-1, B-LOOKUP-1, finite-set-uniqueness markers in one landing. #4564 runtime semantics are the regression floor; iteration-order (DB-8) escalated as Q-M1. https://claude.ai/code/session_018QC433THjRyqiHuP8czPi4 * docs: designs — provenance occurrence-id anchoring (PROV/T-8) + carrier-core-axis decision memo PROV: opaque occurrence id stamped on Node (brand-channel playbook), span data off-tree in a per-compile SpanIndex keyed by id, reusing Locus/Extent ByteRange as the span carrier; one equality-exclusion rule at the single equality authority; carried/derived transport receipts; fail-closed Unanchored/NoEnclosingOccurrence outcomes. Written as input to the HELD operator scoping decision. Carrier axis: decision criterion (functional dependency => coincidence obligation, not a sixth axis; independent pair => ModelCoreFactAxisCarrier) with a measure-first census as the front door; folding into Encoding rejected on both branches. https://claude.ai/code/session_018QC433THjRyqiHuP8czPi4 * docs: tracking memo — Optional match-surface consistency (T3 root-cause, lane adhoc-ce8d7ae6) Records the measured T3 root-cause (substrate-consistency defect, not the positional-vs-labeled model question — the witness never compiled) and registers conditional #9 (Optional-representation unification, sibling of #5 Map). Branch criterion: surface flavor => bounded arm-normalization sweep; deep flavor => #9, co-landing with the Map map_get/match_pattern bridge. Grounds the defect class at collection.dag:28/112 (canonical Present|Absent vs Some/None arms over a Witness-typed value). Flavor unmeasured; the lane reports it. https://claude.ai/code/session_018QC433THjRyqiHuP8czPi4 * docs: apply operator rulings to design open questions (2026-06-09) Q-B1: commitment reframed as the four obligations, not a formalism — grammar formalisms are modeled data; PEG may exist as parse-only; bidirectional verdict = passing obligations 1-4. Q-M1: canonical key order IS the Map representation (one order, plain structural equality, DB-8 by construction; no insertion-order concept). Q-V2: no new CoercionMismatchKind variant (fewer variants for now). Q-T2: gate-in-infer confirmed; checker made relocation-cheap by construction (pure substrate function; infer holds only the gating wire). Q-S2: wave-1 artifact set = emitted .rs; committed-.rs-era transition noted as its own lane re-declaring the compare set. Q-S3: pin rotation operator-GO only, confirmed. https://claude.ai/code/session_018QC433THjRyqiHuP8czPi4 * docs(termination): FunctionRef/call-graph audit — producer does not exist; T-9 rides #4581 Audited 2026-06-09 against the live tree: call-graph extraction is not readable as substrate facts today, three layers deep — (1) zero production writers of dependency_binds_to_edge (4 lens fixtures only; dependency.dag's own T-9 marker confirmed unbuilt); (2) resolve_atom materializes canonicalized spelling, not reference (spelling re-join would rebuild the channel BRAND is dissolving); (3) no stage produces ComputationNode trees and no FunctionRef carrier is landed — call sites have no v4 representation. Fix: T-9 BindsTo materialization lands as a rider on #4581's binding_id stamping (same write, same seam), serving the dependency classifier, structural_resolution lens, and this checker with one producer. Slice step 2 now gated on that producer. https://claude.ai/code/session_018QC433THjRyqiHuP8czPi4 * docs: sizing — COMPREP (function bodies in the v4 pipeline) Measured: the THESIS computation substrate is consumable shapes with zero producers — no stage builds ComputationNode trees, no FunctionRef carrier landed, eval's Transform arm is a call_primitive slot, and the mvp1 'emitted add fn' is a TypeNode Arrow signature (the T0/RTADD/T1 receipts are type-expr-tier). Sized as a first-class node: wave 0 carriers ride #4581 binding_id + the T-9 rider (no new FunctionRef carrier, fewer variants; body = Arrow.body per E-9); wave 1 keystone = source-ingested add body through parse/resolve/infer/eval by execution (~PROV-sized); waves 2-4 behaviors/translate/breadth. Gates SELFHOST facets 1-2 (co-equal with SPINE), termination-checker layer 3, emit ladder T4+, STAGE-ADOPTION. Recommends honest relabeling of T0/RTADD/T1 as signature-tier in the dep graph. Cross-linked from the termination design's prerequisite section. https://claude.ai/code/session_018QC433THjRyqiHuP8czPi4 * docs: portfolio-review fixes — Node identity-channel authority + four seam closures New: design-node-identity-channels.md — single P2 authority for Node- carried fields and equality participation (binding_id IN / occurrence id OUT / BindsTo rides the binding_id channel); #4581 is the linchpin seam; landing order fixed; allocator-determinism obligation flagged to #4581 (DB-8/fixed-point back door). PROV Q-P2 resolved against it; COMPREP wave 0 lands through it. Seam closures: Map Q-M1 reconciled to canonical-order-is-representation (insertion-order draft withdrawn; canonical total order on keys named as the introduced requirement); map_get/match_pattern bridge deletion ownership assigned to the Map landing with the Optional sweep explicitly excluding the site (deep flavor => one co-landed PR); value-set de-gated from the termination checker (structurally terminating; checker validates it later, nothing waits). https://claude.ai/code/session_018QC433THjRyqiHuP8czPi4 * docs: fix doc_refs CI (regen doc-chain) + resolve T3 dual-authority seam - docs/thesis/doc-authority.md: regenerate embedded doc-chain via check_doc_refs.py --write-graph (new design docs joined the graph; the stale chain was the doc_refs CI failure). - design-bidirectional-coercion.md §6: precedence note — rows-not- closures remains the direction authority; the T3 fold-carrier binding decision (incl. positional/labeled discipline) lands only after the one bounded run, with design-optional-surface.md §4 as the single authority for sequencing (review thread r3384485736). https://claude.ai/code/session_018QC433THjRyqiHuP8czPi4 --------- Co-authored-by: Claude <noreply@anthropic.com>
What
A finite
Map<K,V>is a finite set of key→value pairs, so whole-map==is extensional set equality — decidable even for non-Stringkeys. Previously a structural (non-String) key dropped the nativeValue::Mapfast path into the.dagclosure formMap { lookup: fn }, whoseClosurefield has noPartialEqarm, so any structural-keyMap ==was false even vs itself — the root cause of the falseaffected_setfrontier==receipts (#4560 worked around it per-key).Model decision
Map is realized natively as data keyed by
CanonKey, which wraps the original keyValue(sokeys/iteration recover real keys) and:Eqdelegates toValue::eq— the single equality authority (P2). No parallel equality. OnlyHashis derived, and it is made consistent withValue::eq: FreeMonoid alias (Str≡List≡Empty/Conschain),+0.0 == -0.0, order-independent record/variantfields,type_nameignored (matchingValue::eq).Value::eq(v == v) — rejects closures / fn / NaN through the same authority.Fail-closed boundaries (P3)
map_insertwith an invalid key returnsErr(wasOk(None)→ silent fall-through to the closure form).value_to_jsonreturnsErron a non-Stringmap key (no Display-collapse ofInt(1)vsStr("1")).match_patternbridges a present coproduct value toSome { value: v }(symmetric with the existing non-Variantvalue→Somebridge) so stdmap_get'sSome/Nonematch works over native maps holding coproduct values.Evidence (by execution)
affected_setExcluded/rerun receipts go red→green (excluded_propagation_proof,irt1_excluded_propagation_receipt,dimension_seed_rerun,irt1_dimension_seed_receipt).manual/map_structural_key_equality.dag(self-eq, equal-eq, distinct-key ≠, distinct-value ≠, shadow=latest), wired intov4_roster_pilot(also corrects a pre-existing roster row-count drift 42→44).cargo test -p v2-compiler-testsis identical 571 pass / 24 pre-existingpipeline::fails on HEAD and branch;map_lookup_dual_dispatch(incl. rawlookupbuiltin) green; fmt + clippy clean.Scope note
Touches only
src/v2/stage0/src/v2_interpreter.rs(bootstrap interpreter) + additive test claim. The stdMaptype stays thePartialFunctionobservation interface; full dissolution (a finite-relationstdMap type) remains tracked load-bearing work.🤖 Generated with Claude Code