Skip to content

T-7 follow-on: eliminate parse_expr measure via ParseTable memoization - #3661

Merged
briansrls merged 91 commits into
mainfrom
session/bold-bee-345
May 25, 2026
Merged

briansrls merged 91 commits into
mainfrom
session/bold-bee-345

Conversation

@briansrls

@briansrls briansrls commented May 25, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Follow-on to #3655 (T-7 parse walk): eliminates the measure: Nat / grammar_fuel scaffold from parse_expr in src/v4/compiler/02_parse.dag.

  • ParsePositionKey + ParseTable (Map<ParsePositionKey, ParseExprResult>) modeled before wiring (M9 DFS of collection.dag Map<K,V>).
  • build_parse_table: outer fold over token positions (high→low), inner fold over productions (K passes for same-position wrapper chains). Termination by construction — no fuel parameter.
  • Nonterminal arm: table lookup via parse_nonterminal_from_table; no cross-production recursion.
  • Other arms (Terminal/Sequence/Choice/Optional/Repeat): behavior unchanged; only thread all_tokens + table for lookup context.
  • grammar_fuel removed; parse_production / parse public signatures unchanged.

Stacks on #3655 wartime branch (T-6/T-7/T-34 lexer+parse walk + evaluator hooks) until #3655 merges.

Test plan

  • cargo fmt --all --check — pass
  • grammar_validation.dag comment updated (right-recursive case now cites ParseTable, not grammar_fuel)
  • CI (draft → ready triggers auto-coverage)

briansrls and others added 30 commits May 25, 2026 05:26
Pre-req A: nat_lte/nat_gte already landed in nat.dag; string_head/string_tail/string_is_empty in text.dag.

Pre-req B + T-6 (01_tokenize.dag): add char_in_class, lex_match_prefix, lex_match_pattern, lex_repeat_pattern, lex_try_rule/lex_try_rules, lex_walk/lex_walk_step; replace ModeledLexRules stub with full first-rule-wins greedy lexer walk with E1 fail-closed on unrecognized char.

T-7 (02_parse.dag): redesign ParseExprResult with captured: Node field; add sync_tokens: Set<Symbol> to GrammarRoot; add static left-recursion guard; implement parse_expr recursive-descent PEG walk with parse_repeat error recovery via sync-token skip.

T-34 (v4_evaluator.dag): replace all outcome_rejected Wave-1 hook stubs with RuntimeUnit-based implementations covering represent_literal, call_primitive, choose_branch, step_loop, call, transfer, terminal_value.

Addendum: add dag_wave1_sync_tokens() and dag_language_model_wave1_modeled() to dag.dag so T-7 parser walk is reachable from the pipeline via ModeledGrammar.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…epeat progress guard

Finding 3 (02_parse.dag): parse_repeat now checks length(xs: rest) != length(xs: tokens)
before recursing; zero-progress accepted matches (Optional/Empty elements) halt immediately
instead of looping forever.

Finding 4 (02_parse.dag): replace direct-recursion-only expr_starts_with_nonterminal with
transitive expr_reaches_nonterminal_left that tracks a visited Set<Symbol> across
Nonterminal expansions — detects mutual left recursion (A→B→A) where the direct check
missed it.

Findings 1 and 2 were already addressed in the sprint commit: string_head returns
CharResult (CharFound/CharAbsent) not a fabricated Zero, and lex_repeat_pattern
already halts on empty consumed. Fix stale Consumes header in text.dag (Zero removed).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…isposition tags

F2 (01_tokenize.dag): lex_walk_step now rejects zero-width trivia/token matches before
recursing (string_is_empty(lexeme) → E1 diagnostic), preventing EmptyPattern rules from
looping the main walk indefinitely.

F5 (02_parse.dag): parse_production now uses production.emitted — successful captures are
wrapped with the emitted surface node (kind + children extended with the captured parse
subtree via grammar_production_emitted_node_projection edge) instead of returning the raw
captured node.

F6 (02_parse.dag): add coproduct dissolution tag to ParseExprResult naming dissolution
condition (T-8 projection unifies remaining/captured into Outcome<ParseTree>).

F7 (v4_evaluator.dag): five semantic Wave-1 hooks (represent_literal, call_primitive,
choose_branch, step_loop, call) now return diagnostics_singleton carrying the deferred
v4_eval_wave1_semantics_deferred diagnostic — preserves the pre-T-34 deferred signal
while keeping Accepted so Wave-1 is functional. Identity/control hooks unchanged.

F8 (nat.dag): add Practice 10 predicate-dissolution tags to nat_lte and nat_gte,
bound to the same work item as nat_compare (node://adhoc-2145db6b-69a).

Findings 1/3/4 were already addressed in prior commits (CharResult, parse_repeat
length guard, transitive visited-set left-recursion detection).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…dispatch with fold combinators

- text.dag: string_head via fold_list_right; string_tail via fold_list + StringTailState; drop Cons/Empty imports
- 01_tokenize.dag: inline HexDigitChar/IdentContinueChar arms in char_in_class (removes self-recursion); lex_try_rules via fold_list short-circuit accumulator
- 02_parse.dag: parse_diags_to_diagnostics/parse_diags_to_non_empty via fold_list_right + parse_diag_cons helper; parse_skip_to_sync via fold_list + ParseSyncState; parse_repeat_captured_node via fold_list + list_snoc_item

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…fix via is_prefix_of; parse_expr Terminal via fold peek

- algebra.dag: add list_skip<T>(xs, n) — fold_list with counter, no Empty/Cons matching; add ListSkipState<T> helper
- text.dag: simplify string_tail to list_skip(xs:s,n:1); remove StringTailState/fold_list import
- 01_tokenize.dag: rewrite lex_match_prefix using is_prefix_of (stdlib terminal) + list_skip — eliminates direct self-recursion (RC2 easy); import is_prefix_of, list_skip
- 02_parse.dag: add TokenPeek coproduct + parse_token_first (fold_list_right); rewrite parse_expr Terminal using parse_token_first + list_skip — eliminates Empty/Cons match on List<Token> (RC1); import list_skip

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…allback; char_in_class Practice 10 tag

- 02_parse.dag: add parse_e1_syntax_error symbol + diagnostic; parse_production now uses parse_e1_syntax_error_diagnostic as default in parse_diags_to_non_empty — prevents mislabeling genuine parse failures as 'walk not realized'
- 01_tokenize.dag: add 🟡 predicate-dissolution interim tag to char_in_class — consistent with nat_lte/nat_gte tagging pattern in same PR

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ix:/xs:

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…accumulators

list_skip empty accumulator, parse_repeat_captured_node empty children,
parse_skip_to_sync initial result and snoc base — all need explicit Empty
constructor since generic T can't infer [] list literal.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…pr_is_nullable for nullable left-corner soundness

- CharResult (text.dag) and TokenPeek (02_parse.dag): add required 🟡 coproduct dissolution tags per Practice 4; both dissolve when FreeMonoid gains a substrate-derived first<T> accessor returning Option<T>
- expr_is_nullable: new helper (02_parse.dag) that recursively checks whether a GrammarExpr can match empty input, threading a visited-set to guard against cycles on nonterminals
- expr_reaches_nonterminal_left Sequence arm: was unsound for nullable left-hand expressions; now falls through to the right arm when expr_is_nullable(left) is true
- Owns header updated to include expr_is_nullable

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
result: [] in ParseSyncState fold was unresolvable — type param T couldn't be
pinned, cascading to a tok.class type error. Replace with explicit Empty.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…reference closure

algebra.dag list_skip: add `if n <= 0 { xs }` early return — negative skip counts
previously decremented Int past zero without terminating, silently returning Empty.
Nat can't be used (nat.dag imports algebra.dag); the guard makes the function
fail-closed for invalid input per INVARIANTS P2/P3. Comment documents why.

02_parse.dag: add grammar production-reference closure validation before parse walk.
- expr_has_undefined_nonterminal: structural recursion on GrammarExpr (finite tree);
  checks each Nonterminal arm against grammar_lookup_production without following
  into referenced productions, so no visited-set needed.
- grammar_has_undefined_nonterminals: fold over root.productions.
- parse(): check undefined nonterminals first (before left-recursion, which can
  produce confusing errors on broken grammars), reject with
  parse_grammar_undefined_nonterminal diagnostic if any are missing.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ist inference

List<T> is an alias for FreeMonoid<T>; fold_list cannot infer tok:Token from the
accumulator when the field type is the alias form. Using the base type directly
restores type inference. Return type of parse_skip_to_sync stays List<Token>
since FreeMonoid<Token> = List<Token> transparently.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… tok:Token

The cons lambda in parse_skip_to_sync gave fold_list no way to pin tok's type
from the List<Token> alias on ParseSyncState.result. Extracting to parse_sync_step
with explicit 'tok: Token' parameter gives the type context fold_list needs.
Revert the FreeMonoid<Token> field change — List<Token> alias is correct.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
grammar_has_left_recursion and grammar_has_undefined_nonterminals both fold over
root.productions: List<GrammarProduction>. The List<T> alias prevents fold_list
from inferring p:GrammarProduction in the lambda, causing 'no field expression/name
on type FreeMonoid'. Extract typed step helpers with explicit p:GrammarProduction
parameter — same fix as parse_sync_step for tok:Token.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…jected (P3)

Five deferred hooks returned Accepted with a plausible RuntimeUnit value, violating
INVARIANTS P3 fail-closed. Unrealized semantics must not produce success values.

Replace with outcome_rejected(d: v4_eval_wave1_runtime_diagnostic(...)):
- v4_eval_wave1_represent_literal
- v4_eval_wave1_call_primitive
- v4_eval_wave1_choose_branch
- v4_eval_wave1_step_loop
- v4_eval_wave1_call

Legitimate pass-through hooks (bind_value, allocate, represent_result,
return_value, transfer, terminal_value) keep their Accepted returns.
Remove now-unused RuntimeUnit/RuntimeUnitValue/diagnostics_singleton imports.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… claims

Claude RC found the Repeat error-recovery path is unreachable: parse_expr always
returns ParseExprRejected { diagnostics: [] }, so is_empty(failed_diags) is always
true and parse_repeat_error_recover never fires. The header claimed this as live
behavior (P5 violation — documentation must describe live state).

Fix:
- Header: change Status line and walker-semantics note to say "staged (T-8)" rather
  than claiming recovery is realized behavior
- GrammarRoot: add 🟡 gated tag noting sync_tokens + recovery loop are structurally
  correct but unreachable until T-8 wires the upstream signal (parse_expr emitting
  non-empty diagnostics on committed Terminal mismatches)
- parse_repeat false branch: inline 🟡 gated T-8 comment at the unreachable call site

The recovery machinery (parse_repeat_error_recover, parse_skip_to_sync, ParseSyncState,
parse_sync_step) is correctly modeled — it fires correctly once T-8 provides the signal.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…pattern

Both functions are recursive structural walkers over the LexPattern coproduct
without Practice 10 fix-now/tracked-bridge/terminal disposition tags, leaving
lexer semantics as untracked derived-operation logic under INVARIANTS P5.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rammar_has_undefined_nonterminals

Three GrammarExpr structural walkers (expr_is_nullable, expr_reaches_nonterminal_left,
expr_has_undefined_nonterminal) lacked Practice 10 disposition tags per codex finding
on d6934a6.

grammar_has_undefined_nonterminals now validates root.start membership in productions
before folding over production expressions — a bare start Symbol with no production
is now caught as an undefined nonterminal at grammar boundary validation.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…art validation

After grammar_has_undefined_nonterminals was updated to validate root.start (4052785),
the GrammarProductionMissing arm in parse() at the final lookup became dead code — but
still said parse_walk_not_realized, which is both wrong and a P5 violation (implying an
unrealized walk path rather than the actual cause).

Changed to parse_grammar_undefined_nonterminal_diagnostic (correct semantic) and added a
gated T-8 comment noting the arm is now unreachable by construction.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…pend to reject (P3/T-34)

GrammarRoot.sync_tokens was added for T-8 error-recovery but five language extdeps
(rust, java, typescript, swift, wasm) still built two-field records, leaving a
type-rollout gap under INVARIANTS P1. Each now supplies an empty Set (Wave-1 has no
language-specific sync tokens yet; dag.dag already had its set from the prior commit).

v4_eval_wave1_transfer: Suspend arm was returning Accepted { value: c }, contradicting
the T-34 anchor witness (witness_v4_evaluator_transfer_fail_closed expects Rejected) and
INVARIANTS P3 (Wave-1 does not support suspensions). Changed to outcome_rejected with
the standard wave-1 deferred-semantics diagnostic, matching terminal_value behavior.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…escript/wasm

The inline Set { member: fn(_) { false } } literal in struct context lacks type
context for the v2 bootstrap compiler's lambda inference. Use the named
grammar_empty_sync_tokens() helper (added in e993ff5 for rust/java) instead.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…cate production validation

- parse_production: replace emitted.kind + appended child conflation with a
  TypeNode{Conj} wrapper carrying two labeled edges:
  grammar_production_identity_node_projection → production.emitted
  grammar_production_captured_node_projection → captured
  Eliminates the conflation finding: the emitted node is now the identity
  sub-tree, not the output carrier.

- grammar_has_duplicate_productions: fold_list over root.productions with a
  GrammarDupCheckState{seen: Set<Symbol>; has_duplicate: Bool} accumulator,
  using set_symbol_insert to grow the seen set. Wired into parse() between
  grammar_has_undefined_nonterminals and grammar_has_left_recursion.

- Data symbols: grammar_production_identity_node_projection,
  grammar_production_captured_node_projection,
  parse_grammar_duplicate_production added.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: openai-pro / gpt-5-5-pro
  • Commit: f8aad2e7 · Trigger: manual
  • Comparison: main @ d702a606 ... session/bold-bee-345 @ f8aad2e7
  • Conversation: View conversation

1. Story of the diff

This PR turns the v4 tokenize/parse path from a mostly staged surface into an executable .dag walk. src/v4/compiler/01_tokenize.dag adds modeled lexical matching over LexPattern, greedy bounded repeats, delimited comment handling, token production, and fail-closed unrecognized-character diagnostics. src/v4/compiler/02_parse.dag adds grammar validation, duplicate/undefined/left-recursion rejection, syntax-error carriers, parse recovery scaffolding, and the load-bearing T-7 mechanism: a ParseTable keyed by (position, production) so parse_expr can resolve nonterminals through memoized table lookups instead of carrying an explicit parse measure. The language models are updated to fill the new GrammarRoot.sync_tokens field, the .dag language model wires a modeled grammar/lex path, and small std helpers are added for list/string/nat operations needed by the walks. The PR also adds .dag TestClaim rows for grammar validation and tweaks the v4 evaluator wave-1 transfer hook.

2. Invariant categories

1. LAYER MODEL

Finding — src/v4/std/algebra.dag:108: substrate fail-open tail projection. This diff touches std/ substrate by adding list_tail<T> at src/v4/std/algebra.dag:106; its Empty => Empty arm at src/v4/std/algebra.dag:108 makes an invalid tail operation look like a valid empty list. Because this is a reusable std primitive, not a file-local parser helper, the “caller-guaranteed unreachable” comment at src/v4/std/algebra.dag:105 is not enough layer discipline: future consumers can call list_tail(Empty) and get plausible output instead of a typed failure. This should be represented as a non-empty-list input, a typed tail result, or an Outcome/witness-like carrier rather than a public partial projection with a silent default.

2. INVARIANTS.md + modeling-discipline.md

Finding — src/v4/std/algebra.dag:108: violates P3 Fail-Closed and Practice 6 API-level enforcement. Empty => Empty substitutes a plausible value for the missing tail, exactly the fabricated-fallback shape P3 forbids: the invariant says every path succeeds fully or fails with a typed diagnostic, and that missing/malformed input should not be papered over with a replacement. chatgpt-review-77e68658-e7ba-49…

The modeling discipline also says invariants must be enforced by the API/type system rather than by “please don’t do X” convention; here the only guard is the prose at src/v4/std/algebra.dag:105, while the public function accepts every FreeMonoid<T>. chatgpt-review-a3afd55f-f451-48…

3. CODING.md

Finding — src/v4/std/algebra.dag:105: comment is carrying the contract the type should carry. The new helper’s load-bearing safety condition is entirely in the comment: “Empty→Empty is caller-guaranteed unreachable” and “forbidden: calling with empty list without a prior guard.” CODING’s interface discipline prefers structured output/error paths over primitive/silent defaults, and this should be encoded in the signature rather than explained in prose. A tail helper that can fail should expose that in the type; a helper that cannot fail should require a non-empty carrier.

4. TESTING.md

Finding — src/v4/test/claim/parse/grammar_validation.dag:8: the new parser table behavior has no executable receipt in this PR. The added claims are valuable, but the file declares Status: T-22 runner deferred — execution pending T-22 complexity fix, so these do not currently exercise the new parser behavior before merge. More importantly, the claims cover grammar validators and parse_skip_to_sync; they do not assert a successful parse through build_parse_table / parse_production, including the right-recursive case that the PR specifically says is now handled by memoized (position, production) lookup. TESTING.md allows .dag TestClaims as the long-term shape, but the runner is supposed to evaluate those predicates structurally; a deferred claim file is not yet a regression test. chatgpt-review-8d53bb77-0616-41…

For a PR whose central risk is parse-table correctness, this needs an executable receipt or an already-running .dag claim path that proves the memoized parser accepts at least one nontrivial modeled grammar.

5. LOCKED DESIGN DECISIONS

Compliant. I did not see the diff alter a locked design authority or dilute the zero-floor / tests-as-data direction; the main implementation direction—compiler behavior authored in .dag, with tests as .dag claims—is consistent with the uploaded thesis/design context.

6. TRACKED vs UNTRACKED DEBT

Finding — src/v4/std/algebra.dag:105: the staged list_tail debt is not actually bounded. The comment names a future Option<T>-returning projection, but while list_tail is exported from std/algebra.dag, the bridge is not bounded to the guarded parser/lexer call sites that motivated it. That makes the scaffold broader than its stated interim use: every future consumer can depend on the silent Empty => Empty behavior before the dissolution trigger fires. Track it by narrowing the API now, or make the failure carrier part of the helper so the bridge cannot become a new substrate convention.

2.5. Top-down PM intent review

Compliant. Semantically, this PR is trying to realize the PM-level direction rather than postpone it: parser and lexer behavior move into v4 .dag data/functions, language models consume the modeled grammar/lex surfaces, and tests are authored as .dag claims rather than new Rust harnesses. The blockers above are invariant-level execution details—especially fail-closed std surface and an executable parse-table receipt—not a PM-level dilution of the pure-bootstrap or modeled-compiler plan.

3. Verdict

REQUEST_CHANGES

The PR is directionally right, but list_tail introduces a public std/substrate helper that fabricates Empty for an invalid tail operation, and the new parse-table behavior is not covered by an executable receipt in this diff. Both are worth fixing before this parser/lexer surface lands because they set patterns that downstream v4 code will copy.

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review metadata

  • Provider / model: codex / unknown
  • Commit: 73bc8b6f · Trigger: schedule
  • Thinking: 297s wall

BLOCKING (4)

Root Cause

  • src/v4/compiler/01_tokenize.dag LexRuleSet has no declared maximal-munch or precedence contract → implement longest-match before priority, or validate and repair rule order so specific tokens cannot be preempted by identifiers.
  • src/v4/compiler/01_tokenize.dag The prefix-strip helper introduced a local result carrier without running the coproduct gate → add the required disposition tag or reuse an existing std carrier.
  • src/v4/std/algebra.dag The new FreeMonoid tail accessor carrier bypassed the std substrate coproduct gate → add the required disposition tag or collapse it into an existing canonical carrier.
  • src/v4/test/claim/parse/grammar_validation.dag The parse-table acceptance receipt was added before binding the start-production call → define validation_parse_start_production or call parse_production with the explicit start production and token list.

⚠️ The parse walk is close, but the active lexer semantics, missing coproduct dispositions, and unresolved new claim need fixes before merge.

Comment thread src/v4/compiler/01_tokenize.dag Outdated
cons: fn(acc, rule) {
match acc {
LexRuleNoMatch => lex_try_rule(rule: rule, source: source)
LexRuleToken { class: class, lexeme: lexeme, remaining: remaining } =>

This comment was marked as resolved.

Comment thread src/v4/compiler/01_tokenize.dag Outdated
// Uses is_prefix_of (stdlib terminal) to avoid direct structural recursion on FreeMonoid.
// Advances source by folding over prefix — each step pops the head of the remaining source.
// Safe because is_prefix_of guarantees source has at least length(prefix) elements.
type PrefixStripResult

This comment was marked as resolved.

Comment thread src/v4/std/algebra.dag
Cons { head: h, tail: t } => snoc(fold_list_right(xs: t, empty: empty, snoc: snoc), h)
}
}
type ListTailResult<T>

This comment was marked as resolved.

anchor: manual_claim_anchor(anchor: ManualAnchorAbsent),
lhs: validation_bool_atom(
b: validation_outcome_accepted(
out: validation_parse_start_production(root: validation_right_recursive_root)

This comment was marked as resolved.

@briansrls

Copy link
Copy Markdown
Contributor Author

Verified at HEAD aeecb2d0b — this finding was valid on 73bc8b6f but is already fixed.

claim_right_recursive_parse_succeeds no longer references the undefined validation_parse_start_production. The claim now calls validation_right_recursive_parse_fixture(), which is defined in the same module and wires parse_production with an explicit start production (validation_rec_production) and token list (validation_one_token_list):

fn validation_right_recursive_parse_fixture() -> Outcome<ParseTree> {
  parse_production(
    tokens: validation_one_token_list,
    root: validation_right_recursive_root,
    production: validation_rec_production
  )
}
  lhs: validation_bool_atom(
    b: validation_outcome_accepted(out: validation_right_recursive_parse_fixture())
  ),

Structural parse receipt: v4_compiler_parse_table_dag_smoke_test.rs (added in efae99e5d).

No further code change needed for this inline comment.

— sent from bold-bee-345

Pair the new v4_compiler_parse_table_dag_smoke_test harness with INVARIANTS
§P5 Mechanism (b) artifacts: SG-0 census line, INVARIANTS table row, and
sg0-pr-body-append.3661.txt for CI net +1 pairing on PR #3661.

Co-authored-by: Cursor <cursoragent@cursor.com>
@briansrls

Copy link
Copy Markdown
Contributor Author

Codex REQUEST_CHANGES (P5 hand-Rust gate) — addressed in 5cad4bf8a

Finding was valid: v4_compiler_parse_table_dag_smoke_test.rs was registered without the required INVARIANTS §P5(b) pairing.

Fix (Mechanism (b) — same pattern as v4_std_model_core_dag_smoke_test.rs, v4_extdeps_coordination_dag_smoke_test.rs):

  • INVARIANTS.md SG-0 table row for v4_compiler_parse_table_dag_smoke_test.rs (T-7 / T-PB-B deferral lane + interim ratchet test names)
  • sg0_census_test.rs EXPECTED_HAND_AUTHORED_TEST census line
  • scripts/ci-merge/sg0-pr-body-append.3661.txt for CI net +1 (c) pairing

The .dag claim path (claim_right_recursive_parse_succeeds / validation_right_recursive_parse_fixture) remains the long-term receipt shape; the Rust smoke test is the interim M1(2.7) parse-surface ratchet until T-22 eval lands.

— sent from bold-bee-345

The net +1 append for parse-table smoke test must include "dispatch" and a
qualified tracker ref on the pairing line per check-pr-sg0-net-shrink-discipline.sh.

Co-authored-by: Cursor <cursoragent@cursor.com>
@briansrls

Copy link
Copy Markdown
Contributor Author

Codex REQUEST_CHANGES (mixed-authority parse_production) — fixed in latest commit

Finding was valid: the old signature accepted an external GrammarProduction while the table was built from root, so name-equal rows with different expression/emitted could parse one way and wrap another.

Fix: parse_production(tokens, root, production_name: Symbol) now:

  1. grammar_lookup_production(root, production_name) — fail-closed if absent
  2. builds ParseTable from root
  3. wraps with the root-owned production from that lookup

Call sites updated: parse() passes root.start; validation_right_recursive_parse_fixture() passes validation_rec_start (removed duplicate validation_rec_production scaffold).

— sent from bold-bee-345

Replace the separate GrammarProduction parameter with production_name:
lookup the production from root before table lookup and wrap, so parse
and emit share one authority and mixed-expression callers fail closed.

Co-authored-by: Cursor <cursoragent@cursor.com>
@briansrls

Copy link
Copy Markdown
Contributor Author

Codex APPROVE (review 17868 @ 76239f08d) — verified, no further code changes

Spot-checked the approval rubric against HEAD:

Surface Status
P5(b) hand-Rust receipt INVARIANTS.md row + sg0_census_test.rs line + scripts/ci-merge/sg0-pr-body-append.3661.txt (dispatch + gunbc#3661 pairing) — all present since 5cad4bf8a / abe9c83a
Parse-table smoke v4_compiler_parse_table_dag_smoke_test.rs registered in integration.rs; exercises build_parse_table / parse_production(production_name: …)
Practice 10 / P5 tags Walker/predicate helpers in 01_tokenize.dag and 02_parse.dag carry 🟡 disposition tags; list_tail uses typed ListTailResult (TailAbsent fail-closed)
Mixed-authority fix parse_production(tokens, root, production_name: Symbol) — single root-owned authority (76239f08d)

Agree with verdict: no diff-local invariant, modeling, coding, or testing violation blocking merge on this SHA.

— sent from bold-bee-345

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review metadata

  • Provider / model: codex / unknown
  • Commit: 76239f08 · Trigger: schedule
  • Thinking: 370s wall

BLOCKING (2)

Root Cause

  • src/v4/compiler/01_tokenize.dag Lexeme length comparison borrowed the Nat-only character-range comparator instead of an Int/order authority → compare lengths with the Int ordering surface or make the length/index carrier consistently Nat.
  • src/v4/compiler/02_parse.dag Parse-table entrypoints lack a validated-grammar carrier boundary → introduce a ParseReady/ValidatedGrammarRoot produced by the validation checks or run the same gates before building the table.

⚠️ The prior comments are addressed, but the lexer type mismatch and parse_production validation bypass need fixes before merge.

Comment thread src/v4/compiler/01_tokenize.dag Outdated
match candidate {
LexRuleNoMatch => acc
_ =>
if nat_gte(a: lex_rule_apply_lexeme_length(apply: candidate), b: lex_rule_apply_lexeme_length(apply: acc)) {

This comment was marked as resolved.

Comment thread src/v4/compiler/02_parse.dag Outdated
}


fn parse_production(tokens: List<Token>, root: GrammarRoot, production_name: Symbol) -> Outcome<ParseTree> {

This comment was marked as resolved.

@briansrls

Copy link
Copy Markdown
Contributor Author

Claude APPROVE (review 17872 @ b7e423b21) — verified, no further code changes

Spot-checked the assessment against HEAD:

Claim Verified
P5(b) hand-Rust trio INVARIANTS.md row + sg0_census_test.rs line + sg0-pr-body-append.3661.txt all present
Fail-closed parse path parse_diags_to_non_empty lifts empty ParseExprRejected diagnostics to parse_e1_syntax_error_diagnostic before any Rejected escapes
Validator → carrier boundary grammar_validate_for_parse runs undefined/duplicate/left-recursion gates and returns ValidatedGrammarRoot; parse_production now takes validated: ValidatedGrammarRoot (not raw GrammarRoot) — direct T-7 callers cannot bypass validation
Duplicate-check coproduct GrammarDupCheckResult (GrammarDupCheckUnique | GrammarDupCheckDuplicate), not an in-band Bool flag
Maximal-munch type surface lex_try_rules_prefer_longer compares Int lexeme lengths via >= (not nat_gte)

Also confirms prior codex inline findings on this SHA are addressed (b455e6c15 Int ordering, b7e423b21 validated-grammar carrier + claim fixture wiring through grammar_validate_for_parse).

Agree with verdict: no blocking concerns on this SHA.

— sent from bold-bee-345

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: openai-pro / gpt-5-5-pro
  • Commit: b7e423b2 · Trigger: manual
  • Comparison: main @ d702a606 ... session/bold-bee-345 @ b7e423b2
  • Conversation: View conversation

1. Story of the diff

This PR moves the v4 lexer/parser from “modeled shape, parse walk mostly deferred” toward executable .dag behavior. The lexer now has modeled pattern matching, maximal-munch rule selection, fold-bounded source walking, and fail-closed unrecognized-character diagnostics in src/v4/compiler/01_tokenize.dag. The parser change is the load-bearing T-7 piece: src/v4/compiler/02_parse.dag introduces grammar validation, nullable/left-corner analysis, a ValidatedGrammarRoot, and a memoized ParseTable keyed by token position plus production name so parse_expr can handle nonterminal recursion without carrying a fuel/measure parameter. The PR also adds std helpers (ListTailResult, list_tail, text head/tail helpers, Nat order predicates), updates language models to include sync_tokens, adds a .dag TestClaim corpus plus a Rust parse-surface smoke harness, and adjusts the SG-0 hand-authored-test accounting for that harness.

2. Invariant categories

1. LAYER MODEL

Compliant — the diff does touch substrate-adjacent v4 std/modeling surfaces (src/v4/std/algebra.dag, src/v4/std/text.dag, src/v4/compiler/02_parse.dag), but the newly introduced carrier shapes are disposition-tagged and the GrammarRoot.sync_tokens projection gap is explicitly staged rather than silently represented as a Node edge. The concrete fail-closed problems are not layer-placement problems; they are called out under invariants below.

2. INVARIANTS.md + modeling-discipline.md

Finding — P3 Fail-Closed / typed diagnostic propagation. src/v4/compiler/02_parse.dag:536-537 turns a ParseTable lookup Violates { diagnostic: _ } into ParseExprRejected { diagnostics: [] }, and src/v4/compiler/02_parse.dag:1117-1121 similarly discards the table lookup diagnostic and replaces it with the generic parse_e1_syntax_error default. A missing memo entry is not a normal grammar mismatch; it means the memo table failed to contain an entry that build_parse_table is supposed to totalize for every (position, production). By erasing the typed Violates diagnostic, nested nonterminal misses become indistinguishable from backtrackable parse failures, so Choice/Optional can discard them and still accept, while the top-level path reports user syntax instead of the internal table-integrity failure. That violates the fail-closed rule that missing/malformed facts must propagate as typed diagnostics, not plausible defaults. chatgpt-review-0bc68b77-6179-4c…

Finding — P3 Fail-Closed / no fabricated successful value. src/v4/extdeps/runtimes/v4_evaluator.dag:197 and src/v4/extdeps/runtimes/v4_evaluator.dag:199 return Accepted { value: EvaluationFrame { current: runtime_value_resolved_type(value: v), ... } } for Continue and Return, while the inline rationale says faithful RuntimeValue → EvaluationFrame reification is still gated and that this threads only “the type Node of v” into frame.current at src/v4/extdeps/runtimes/v4_evaluator.dag:190. That is a successful result with the value fact replaced by its type, not a fully modeled transfer. Until RuntimeValue can be reified into the frame or the frame gains a result slot, this path should reject with a typed diagnostic rather than accepting a partial projection as the new frame. This also conflicts with the thesis-level no-fabrication posture for missing facts. chatgpt-review-a48c0cbc-3cfb-48…

3. CODING.md

Compliant — the new Rust harness code is implementation-only, uses small free helpers over parsed surface modules, and keeps the impure include_str!/parse-surface smoke at the integration-test edge. The code-style concern that matters here is not Rust structure; it is the semantic fail-closed issue in the .dag model above.

4. TESTING.md

Finding — behavior-driven regression coverage. src/v4/test/claim/parse/grammar_validation.dag:165 defines validation_one_token_list, and src/v4/test/claim/parse/grammar_validation.dag:174 feeds that one-token list into the right-recursive parse fixture whose claim is labeled at src/v4/test/claim/parse/grammar_validation.dag:240 as “A = 'x' A? on one matching token.” With one token, the recursive A? branch succeeds even if every Nonterminal table lookup rejects, because the optional can fall back to empty after the first terminal. That means the new parse-table memoization claim does not actually force a successful recursive table hit. For this PR’s core behavior, use at least two matching tokens, or add a second claim, so a broken/missing memoized A at the next position leaves tokens remaining and fails. TESTING.md asks tests to pin the promised behavior, not just the presence of a structurally named path. chatgpt-review-aa120114-fe6a-45…

5. LOCKED DESIGN DECISIONS

N/A — I did not see a changed line that explicitly alters a locked design decision. The Pure Bootstrap / 0-floor direction is respected by keeping new verification intent in .dag TestClaim data and registering the temporary Rust smoke in SG-0 rather than presenting it as permanent.

6. TRACKED vs UNTRACKED DEBT

Compliant — the new hand-authored Rust smoke is tracked with the INVARIANTS table row, the SG-0 census entry, and scripts/ci-merge/sg0-pr-body-append.3661.txt, including a dissolution trigger tied to T-22/generated harness coverage. The staged sync_tokens and accessor/morphism helpers also carry inline 🟡 gates. The two findings above are not “untracked debt” issues; they are semantic fail-closed/test-adequacy issues that should be fixed before merge.

2.5. Top-down PM intent review

Finding — the evaluator transfer change dilutes the project’s highest-level “missing facts do not become plausible output” intent. The thesis says missing facts are compile-time errors rather than fabricated runtime artifacts, and that the compiler’s internal graph should be sound when it accepts a result. chatgpt-review-a48c0cbc-3cfb-48…

The changed Continue/Return transfer path at src/v4/extdeps/runtimes/v4_evaluator.dag:197 and src/v4/extdeps/runtimes/v4_evaluator.dag:199 accepts an EvaluationFrame that contains only runtime_value_resolved_type(value: v), while the comment at src/v4/extdeps/runtimes/v4_evaluator.dag:190 states this is the value’s type Node, not faithful value reification. That is the kind of semantic downgrade a PM should reject even if it is marked as wave-1 interim, because it changes a missing-fact boundary from “reject until modeled” into “accept a lossy stand-in.”

3. Verdict

REQUEST_CHANGES

The parser-table architecture is directionally right, and the SG-0/test scaffolding is tracked, but the PR currently loses typed diagnostics on memo-table misses and accepts evaluator transfers with only a type-level stand-in for the returned value. The T-7 claim also needs to force an actual successful recursive memo lookup rather than passing through the optional-empty path.

@briansrls

Copy link
Copy Markdown
Contributor Author

Claude APPROVE (review 17877 @ 26299e56b) — verified, no further code changes

Spot-checked the assessment against HEAD:

Claim Verified
P5(b) receipt triple INVARIANTS.md row + sg0_census_test.rs:849-851 + sg0-pr-body-append.3661.txt — all present
Predicate/coproduct tags nat_lte/nat_gte, grammar validators, and carrier coproducts (ListTailResult, CharResult, TokenPeek, GrammarDupCheckResult, ParseExprResult) carry 🟡 disposition tags with bind/dissolve-on/forbidden
Validator family walks declared GrammarExpr/GrammarProduction coproducts; duplicate scan uses GrammarDupCheckResult not in-band Bool
build_parse_table bounded fill position×production double fold via fold_list_right over token positions

Also confirms openai-pro RC items from b7e423b2 are addressed on this SHA (26299e56b): memo-table Violates diagnostics propagate (not erased), wave-1 Continue/Return transfer rejects, parse-table claim uses two tokens to force recursive memo hit.

Agree with verdict: no blocking concerns on this SHA.

— sent from bold-bee-345

@briansrls

Copy link
Copy Markdown
Contributor Author

openai-pro REQUEST_CHANGES @ b7e423b2 — addressed in 26299e56b

All three findings were valid on b7e423b2. Fixes on HEAD:

  1. ParseTable Violates diagnostic propagation (P3) — parse_nonterminal_from_table and parse_production now preserve the typed lookup diagnostic: Violates { diagnostic: d } → ParseExprRejected { diagnostics: [d] } / Rejected { diagnostics: diagnostics_singleton(d: d) }. No longer erased to [] or generic parse_e1_syntax_error.

  2. Evaluator transfer fail-closed (P3) — v4_eval_wave1_transfer Continue/Return arms now outcome_rejected with v4_eval_wave1_runtime_diagnostic instead of accepting a frame whose current carries only runtime_value_resolved_type(value: v). witness_v4_evaluator_transfer_fail_closed (Suspend path) unchanged.

  3. Parse-table claim adequacy (TESTING) — validation_two_token_list (two matching tokens) replaces one-token input in validation_right_recursive_parse_fixture; claim label updated. Forces memoized A lookup at position 1 after the first terminal — optional-empty no longer masks a missing table entry.

— sent from bold-bee-345

@briansrls
briansrls merged commit c1c5aeb into main May 25, 2026
7 checks passed
briansrls added a commit that referenced this pull request May 25, 2026
Export ordered_ring_node, approximate_field_node, boolean_algebra_node,
and free_monoid_node with Conj/Disj/Arrow field projections that thread
the inhabitant through declared type-parameter slots. Rebased onto main
(includes ListTailResult/list_tail from #3661).

Addresses codex REQUEST_CHANGES on opaque Atom template symbols.

Co-authored-by: Cursor <cursoragent@cursor.com>

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review metadata

  • Provider / model: codex / unknown
  • Commit: 26299e56 · Trigger: schedule
  • Thinking: 256s wall

BLOCKING (1)

Root Cause

  • src/v4/compiler/02_parse.dag the validated-grammar boundary was applied only at parse_production, while the parse-table builder remains a public raw-root surface → make build_parse_table consume ValidatedGrammarRoot or return Outcome after running grammar_validate_for_parse.

⚠️ The prior inline blockers are fixed, but the parse-table builder still leaves the validation boundary bypassable.


// Outer fold over positions high→low; inner fold over productions × passes at each position.
// Termination: (N+1) positions × K productions × K passes — structurally bounded, no fuel.
fn build_parse_table(all_tokens: List<Token>, root: GrammarRoot) -> ParseTable {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BLOCKING: build_parse_table is a T-7 parse-table entrypoint that still accepts raw GrammarRoot and returns ParseTable without grammar_validate_for_parse, so direct callers can bypass undefined/duplicate/left-recursion diagnostics in violation of P3 fail-closed and Practice 6 API-level enforcement.

briansrls added a commit that referenced this pull request May 25, 2026
…field_node / boolean_algebra_node (unblocks T-4 Wave 2b bridge-symbol dissolution) (#3663)

* feat(T-2): add structural algebra Node constructors on main

Export ordered_ring_node, approximate_field_node, boolean_algebra_node,
and free_monoid_node with Conj/Disj/Arrow field projections that thread
the inhabitant through declared type-parameter slots. Rebased onto main
(includes ListTailResult/list_tail from #3661).

Addresses codex REQUEST_CHANGES on opaque Atom template symbols.

Co-authored-by: Cursor <cursoragent@cursor.com>

* WIP: T-2 Node constructors: algebra.dag — ordered_ring_node / approximate_fie

* fix(T-2): free_monoid tail fixpoint placeholder + trim prose comments

Replace empty Conj tail slot (62e761a) with inhabitance-backed
free_monoid_type_fixpoint marker: Cons.tail carries the same carrier
T and marks equi-recursive FreeMonoid<T> without same-argument
unfold (complexity checker rejects direct self-call). Remove
Practice-9 prose block above algebra_edge symbols.

Qualified Symbol identities (ring_field_add vs approximate_field_field_add)
remain — fixes v4 compile undefined-variable/collision errors from
short-form = add / = op RHS.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(T-2): correct malformed gate mark on free_monoid_tail_type_fixpoint

Canonical 🟡 needs-more-work form with feature slug and dissolve-on
condition, per INVARIANTS.md gate-mark discipline.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* WIP: T-2 Node constructors: algebra.dag — ordered_ring_node / approximate_fie

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
@briansrls
briansrls deleted the session/bold-bee-345 branch June 1, 2026 18:41
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