Skip to content

Enable strict record-field typecheck: close fail-open infer_record_lit hole (§5) - #5293

Merged
briansrls merged 8 commits into
mainfrom
strict-record-field-typecheck
Jun 20, 2026
Merged

briansrls merged 8 commits into
mainfrom
strict-record-field-typecheck

Conversation

@gunbai-bot

@gunbai-bot gunbai-bot Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

What

Closes the fail-open hole in the v1-seed typechecker's record-construction
path (infer_record_lit). Before this PR it silently accepted fields that
exist on no variant of the constructed type — a DESIGN.md §5 violation ("a
wrong answer is a loud error, never a warning"). That hole is exactly what
let the malformed BoolWitness { holds } rows (removed in #5272) typecheck
green.

Why it wasn't already strict — two resolver-precision gaps

The leniency was compensating for the resolver not being precise enough. Making
it strict required first making field-resolution confident, else real
constructions false-positive:

  1. Aliases not followed — Set<T> = PointwisePower<T>; Set { member } is
    correct but the resolver returned Set's children, not PointwisePower's.
  2. Variant-vs-collection name collisions — GrammarExpr.Optional (a
    variant) vs the Optional<T> collection; global lookup beat the
    expected-coproduct context.

Host change (src/v1/stage0/src/v1_compiler_infer.rs)

  • record_lit_construction_field_names — a confident field-name resolver:
    follows type X = Y aliases and searches the expected coproduct's arms before
    the global fallback. Returns None when not confident, so anonymous /
    unresolved records never false-positive.
  • field_in_any_variant_named — an ambiguity-aware secondary scan, run only
    on would-be-errors, suppressing the case where a field is valid on a
    same-named variant of another coproduct (e.g. TextNode) while keeping
    teeth on truly-undefined fields.
  • Emits CompilerDiagnostic::FieldNotFound for an unknown constructed field.

Real bugs this surfaced (silently tolerated before)

The strict check immediately found pre-existing constructions with the wrong
field name — fixed here using the existing constructor helpers:

  • infer_ground_add_mvp.dag: NodeLocus { node } → node_locus(node:) (field
    is anchor).
  • 3 lens/application tests: PortLocus { port } → port_locus(port:) (field
    is anchor).

Proof by execution (§5: executed-clean, not sampled-clean)

Full claim_batch floor green against current main (merged in):
447 discriminating witnesses pass, zero field-not-found false positives.
Discriminating probe BoolWitness { holds } goes red with
field 'holds' not found in type 'BoolWitness'. The sampled sweep had called
the corpus clean; only the executed full floor surfaced the real bugs above.

Scope / stacking

🤖 Generated with Claude Code

briansrls and others added 8 commits June 19, 2026 13:58
… predicates as test fn

The 13 `data claim_*: TestClaim = BoolWitnessClaim { witness: BoolWitness { holds: f() } }`
rows (timeseries_passive_map_fold_anchor_test ×7, timeseries_signal_passives_test ×6) are
triple-malformed dead spec that compiled only because the v1-seed record typechecker is
fail-open (`infer_record_lit` silently ignores undefined fields and never validates missing
required ones):
  - `holds` is not a field on `BoolWitness` (which has only `entry`, `function`);
  - they are typed `: TestClaim`, but `BoolWitnessClaim` is an arm of `UnifiedTestClaim`, not
    of `TestClaim` (whose arms are Compiles/Diagnostic/Equals/StructuralEquals/RoundTrip);
  - wrong inner shape.
The live floor discovers only `data unified_claim_*` (never `data claim_*`), so these never ran
— pure dead specification, and a §3 fork of the canonical `{ entry, function }` form ratified
by consolidation #4553.

Asymmetry handled per §5 (do not silently drop coverage):
  - anchor file: its 7 predicates were already live `test fn` (#5257) — pure deletion.
  - signal file: only 1 of its 6 predicates was a live `test fn`; the other 5
    (spice_capacitor/inductor_projects_*, passive_topology_preserves_component_name,
    named_affine_voltage_preserves_ground_reference, absolute_potential_from_named_preserves_reference)
    were plain `fn` wired ONLY through the dead `{holds}` rows — re-homed as `test fn` to
    RESTORE their intended floor coverage rather than orphan the assertions.

Drop the now-unused `BoolWitness`/`BoolWitnessClaim` imports from both files.

Green by execution: `claim_batch --source-root src/v2` over all 13 witnesses → 13 PASS,
including the 5 newly-live signal predicates. Their assertions discriminate on Present/Absent
(not tautological), so floor coverage is strictly increased, not faked.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…t hole

The v1-seed typechecker (infer_record_lit) was fail-OPEN on record
construction — it silently ignored fields that exist on no variant of the
constructed type (DESIGN.md §5: "a wrong answer is a loud error, never a
warning"). That hole is what let the malformed `BoolWitness { holds }` rows
typecheck green (see #5272). Closing it.

Host change (src/v1/stage0/src/v1_compiler_infer.rs):
- record_lit_construction_field_names: a *confident* field-name resolver that
  follows `type X = Y` aliases and searches the expected coproduct's arms
  before the global fallback (the two resolver-precision gaps the leniency was
  compensating for: `Set<T> = PointwisePower<T>` alias; variant-vs-collection
  name collisions like `GrammarExpr.Optional`). Returns None when not confident
  so anonymous/unresolved records never false-positive.
- field_in_any_variant_named: an ambiguity-aware secondary scan, run only on
  would-be-errors, that suppresses false positives where a field is valid on a
  same-named variant of *another* coproduct (e.g. TextNode) while keeping teeth
  on truly-undefined fields.
- emit CompilerDiagnostic::FieldNotFound for an unknown constructed field.

Real bugs this surfaced (silently tolerated before; field name was wrong):
- infer_ground_add_mvp.dag: `NodeLocus { node }` → `node_locus(node:)`
  (the field is `anchor`).
- 3 lens/application tests: `PortLocus { port }` → `port_locus(port:)`
  (the field is `anchor`).

Proven by execution: full claim_batch floor green (439 witnesses) with the
check live; probe `BoolWitness { holds }` goes red with
"field 'holds' not found in type 'BoolWitness'".

Missing-required-field enforcement deferred (optional-cardinality +
templating risk, unmeasured).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ile (§3/§7)

Per the §3/§7 review (cursor): the strict infer_record_lit check lived only in the
generated seed .rs (a host hand-edit), not the .dag authority. Port the helpers and the
infer_record_lit guard into src/v1/04_infer.dag so the seed is regenerated FROM the model:
  - record_lit_construction_field_names: confident field-name resolver (alias-follow +
    expected-coproduct-arm union), SKIPS generic/parametric types (newtype flattening makes
    their resolved field names non-source-faithful — the algebra-tower false positives).
  - field_in_any_variant_named: same-named-coproduct-arm ambiguity guard, run only on a
    would-be error (keeps TextNode-style constructions silent; full teeth elsewhere).
  - infer_record_lit emits FieldNotFound for a field on no field of a confidently-resolved
    non-generic record; none-result paths stay silent (no false positives).

§3 de-confliction: two 'type TextFile' collided in the bootstrap's combined src/v1+dsl
closure — v1 {path, content} (emit output) vs dsl/std {path, document} (markup). The
name-keyed type env clobbered one, so the strict check could not resolve 'content'. Rename
the dsl one to DocumentFile (it holds a Document; parallel to its sibling RenderedTextFile),
the smaller + semantically-correct fix. v1 TextFile {path, content} is now the sole binding.

The regenerated .rs follows (regen against current main's seed, post-merge).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ypecheck

# Conflicts:
#	src/v2/std/timeseries_signal_passives_test.dag
…false positives)

The struct_fields/expected-based resolver false-positived on main's newer types where
resolved field names diverge from source under newtype flattening / name collision:
  - `Watt = Measure<Power,One,Nat>` etc. — concrete aliases of the single-field generic
    `Measure<Q,S,M> { count: M }`; the wrapper dissolves so `count` isn't in the resolved set.
  - `Vendor { legal_name }` — a §3 name collision: `extdeps Vendor<Domain> { legal_name }`
    record vs the std `Vendor = UnknownVendor | ...` enum; lookup grabs the enum.

Replace it with a confident-only resolver: fire ONLY when the constructed name resolves to a
plain, fully-grounded record (params == 0, connective == Conj, children > 0), reading source
field names directly from its own children. Generics, concrete aliases of single-field
generics, and coproduct/enum (incl. the losing side of a collision) all return none -> silent.
Keeps full teeth on plain records, where every real bug lives (BoolWitness{holds},
NodeLocus{node}, PortLocus{port}). Drops the now-unused record_lit_expected_coproduct_arm.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The committed seed is the compiler's own emit of the strict 04_infer.dag authority (derived,
not hand-edited) — closes the §3/§7 review: the strict infer_record_lit check now lives in the
.dag model and the .rs is regenerated from it. Single-file diff (+136/-1, no new module deps):
only 04_infer.dag changed, so only its emitted module changes.

Proven by execution on the merged tree:
  - floor (claim_executor over src/v2+dsl): 0 field-not-found false positives
  - self-compile (src/v1+dsl via regen path): 0 field-not-found
  - cargo fmt --all --check + cargo clippy --all-targets -D warnings: clean
  - discriminating probe: `BoolWitness { holds: true }` -> loud
    'field holds not found in type BoolWitness'

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@gunbai-bot

gunbai-bot Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor Author

Addressed in 9737a87 + fb3a8d4 (rebased onto latest main; timeseries_signal_passives_test.dag conflict resolved by taking main's relocation + porting the holds-removal into the file main relocated those rows to, spice_passive_projection_test.dag).

§3/§7 port (the requested change). The strict infer_record_lit check now lives in the .dag authority src/v1/04_infer.dag; the committed src/v1/stage0/src/v1_compiler_infer.rs is the compiler's own emit of that source (derived, not hand-edited). Single-file seed diff (+136/-1, no new module deps) — only 04_infer.dag changed, so only its emitted module changes.

Tightened to confident-only (§5, no false positives). The first port false-positived on main's newer types where resolved field names diverge from source under newtype flattening / a §3 name collision:

  • Watt = Measure<Power,One,Nat> etc. — concrete aliases of the single-field generic Measure<Q,S,M> { count: M }; the wrapper dissolves so count isn't in the resolved set.
  • Vendor { legal_name } — extdeps Vendor<Domain> { legal_name } record vs the std Vendor = UnknownVendor | … enum; the name-keyed env grabs the enum.

record_lit_construction_field_names now fires only when the constructed name resolves to a plain, fully-grounded record (params == 0 && connective == Conj && children > 0), reading source field names from its own children. Generics, concrete aliases of single-field generics, and coproduct/enum (incl. the losing side of a §3 collision) all return none → silent. Full teeth retained on plain records, where every real bug lives.

§3 de-confliction. Two type TextFile collided in the bootstrap's combined src/v1+dsl closure (v1 {path, content} vs dsl/std {path, document}); renamed the dsl one → DocumentFile (it holds a Document, parallel to its sibling RenderedTextFile).

Proven by execution on the merged tree:

  • floor (claim_executor over src/v2+dsl): 523 witnesses pass, 0 field-not-found false positives
  • self-compile (src/v1+dsl via the regen path): 0 field-not-found
  • cargo fmt --all --check + cargo clippy --all-targets -- -D warnings: clean
  • discriminating probe: BoolWitness { holds: true } → error: field 'holds' not found in type 'BoolWitness'

— sent from silent-bee-439

@briansrls
briansrls merged commit d7d2151 into main Jun 20, 2026
1 check passed
@briansrls
briansrls deleted the strict-record-field-typecheck branch June 20, 2026 02:41
briansrls added a commit that referenced this pull request Jun 20, 2026
Manager trim: revert accidental #5293 infer rollback, DocumentFile→TextFile
rename, and lens/spice collateral. PR-1 scope is src/v2/std/algebra.dag +
freemonoid_gap_free_fns_test.dag only (locked plan).

Co-authored-by: Cursor <cursoragent@cursor.com>
briansrls added a commit that referenced this pull request Jun 20, 2026
Land additive v2.std.algebra gap fns (map, flat_map, reverse, skip/take/slice,
sort_by, first/last, fold) plus 10 green execution witnesses. Fix last via left-fold
(bootstrap-safe); defer enumerate — fold_list into FreeMonoid<IndexedElement<T>>
trips variant-not-found when co-loaded with gap fns. Merges main (infer #5293).

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant