Repository navigation
Compositional parser: spec-driven dispatch (Stream 0) - #226
Conversation
…(Stream 0) Replace 30+ ShKw* keyword variants with single ShKeyword; item dispatch, operator precedence, and literal keywords now read from SyntaxSpec data in dsl/extdeps/languages/dag/syntax.dag. Adding a new item type with a standard body kind requires editing 1 file (syntax.dag), zero parser changes. Phase 0: TokenShape consolidation (ShKw* → ShKeyword, 70+ predicates deleted) Phase 1: SyntaxSpec types + data-driven parse_item via ItemForm/BodyKind Phase 2: Operator precedence from dag_operators spec table Phase 3: Literal keyword dispatch from dag_keyword_literals map 148/148 v2-compiler-tests pass. Net -264 lines. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 80c54e166a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Stream 0 (compositional parser) marked DONE with PR #226 details - R3 parser grade updated D→B+ with shipped/remaining breakdown - Current State section updated: 148 tests, spec-driven dispatch, stage0 regen blocker noted - review.dag critical path demoted to backlog (not architectural priority) - fixture/test blocks unblocked by compositional parser - Phase 5 milestones updated with Stream 0 completion Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Review feedback (PR #226): eat(), peek_is_expected_at(), parse_expr_list_until_acc() still called deleted shape_matches_expected; peek_is_node_decl still called deleted is_name_keyword_shape. Migrated all 4 to token_matches_expected / is_name_keyword. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
briansrls
left a comment
There was a problem hiding this comment.
Review (5 violations, 5 modeling suggestions)
Invariant violations (5)
Modeling (good)
src/v2/00_core.dagCollapsing keyword tokens to ShKeyword is a more faithful model of keywords as one lexical class than a closed enum of per-keyword variants.src/v2/01_tokenize.dagThe tokenizer now cleanly separates lexical recognition from parser semantics by emitting one ShKeyword shape for all reserved words.src/v2/02_parse.dagExpectKeyword and body-kind dispatch move parser control flow toward composition over syntax data instead of hardcoded keyword variants.src/v2/languages.dagSyntaxSpec, ItemForm, and OperatorSpec establish a reusable parser-facing vocabulary parallel to LanguageSpec, which is the right compositional direction.dsl/extdeps/languages/dag/syntax.dagEncoding .dag item forms and precedence as data makes the frontend more faithful to the language contract and lowers the edit surface for new syntax.
Modeling (improve)
src/v2/00_core.dagKeyword identity still escapes as raw Token.text string checks downstream, so the token model is not yet composed from a single syntax authority.src/v2/01_tokenize.dagThe keywords data should be derived from the syntax spec rather than restated here, because the same fact now lives in multiple declarations.src/v2/02_parse.dagThe parser still carries local keyword tables and keyword-string ladders, so the new syntax model is only partially compositional.src/v2/languages.dagItemForm’s boolean feature matrix is only partly authoritative and would compose more faithfully as smaller clause algebras in shared std.languages rather than a compiler-local shim.dsl/extdeps/languages/dag/syntax.dagThe spec still splits literal keywords outside SyntaxSpec and encodes |> and . with fake Add binops, so it is not yet a fully faithful single authority for frontend syntax.
The refactor is moving in the right direction, but the new syntax boundary is still partial and non-authoritative because keyword facts remain duplicated and several spec facts are unused or fabricated.
| let r = expect(tokens: tokens, state: state, expected: ExpectKeyword { text: form.keyword }) | ||
| if has_err(err: r.err) { return ItemResult { item: dummy, state: r.state, err: r.err } } | ||
| // Dispatch on body kind to the appropriate parser (after keyword) | ||
| match form.body_kind { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| OperatorSpec | ||
| } | ||
|
|
||
| import extdeps.languages.dag.syntax { dag_item_forms, dag_operators, dag_keyword_literals } |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| OperatorSpec { symbol: "*", left_bp: 15, right_bp: 16, binop: Mul }, | ||
| OperatorSpec { symbol: "/", left_bp: 15, right_bp: 16, binop: Div }, | ||
| OperatorSpec { symbol: "%", left_bp: 15, right_bp: 16, binop: Mod }, | ||
| OperatorSpec { symbol: "|>", left_bp: 17, right_bp: 18, binop: Add }, |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| @@ -39,20 +31,20 @@ import v2.std.core { | |||
| // Data declaration, not match arms. Adding a keyword = one entry here. | |||
|
|
|||
| data keywords: Map<String, TokenShape> = { | |||
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| } | ||
| // Excludes literal keywords (true, false, none) and acquire/release. | ||
| // With ShKeyword consolidation, checks shape + text membership. | ||
| data name_keyword_texts: List<String> = [ |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Deficit A — single keyword authority:
- Tokenizer derives keywords from dag_all_keywords (syntax.dag)
instead of maintaining local keyword map
- Parser imports dag_syntax_spec as authority, not individual fields
- Deleted name_keyword_texts enumeration; is_name_keyword derives
structurally (any keyword not in literal_keywords, not lifecycle)
- SyntaxSpec gains all_keywords and keyword_literals fields
Deficit B — operator binop fabrication:
- OperatorSpec.binop is now BinOpKind? (optional)
- Dot and pipe-arrow have binop: none (precedence-only entries)
Deficit C — unconsumed ItemForm facts:
- Added ItemPrefixResult type and parse_item_prefix function
- parse_item_by_form uses generic prefix for TypeBody/ExprBody/BlockBody
- has_type_params, has_params, has_return_type, has_uses all load-bearing
- 3 body-only parsers: parse_type/fn/block_body_from_prefix
148/148 v2-compiler-tests pass.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…anded + decidability direction) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Stream 0 eliminated all ShKw match arms from parse_item. The ratchet now asserts 0 — the target state. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
briansrls
left a comment
There was a problem hiding this comment.
Review (4 violations, 3 good, 3 improve)
Invariant violations (4)
Modeling (good)
src/v2/00_core.dagShKeyword keeps lexical classification at the token layer while moving language-specific keyword identity out to syntax data, which is a cleaner separation of concernssrc/v2/languages.dagSyntaxSpec and OperatorSpec sit at the right generic layer as parser schema and make item and operator behavior emerge from shared data rather than hardcoded parser branchesdsl/extdeps/languages/dag/syntax.dagdag_syntax_spec sits at the right concrete layer as the .dag instance of SyntaxSpec, and its operator and literal tables faithfully encode real surface syntax
Modeling (improve)
src/v2/00_core.dagTokenShape now erases keyword role entirely, so later parser logic still has to recover item and control meaning from strings instead of composing from explicit keyword-role structuresrc/v2/languages.dagItemForm is a flag matrix of booleans rather than a compositional sum of prefix and body forms, so impossible states remain representable and consumers still need ad hoc conditionalsdsl/extdeps/languages/dag/syntax.dagdag_all_keywords duplicates facts already present in dag_item_forms and dag_keyword_literals instead of deriving them compositionally, which weakens single-authority fidelity
The refactor is directionally strong and materially more compositional, but it still lands with visible invariant violations in hot-path lookup, boundary enforcement, fabricated operator fallback, and residual string-driven dispatch
| // No local keyword enumeration — adding a keyword = one entry in syntax.dag. | ||
|
|
||
| fn is_keyword_text(text: String) -> Bool { | ||
| dag_all_keywords |> any(k => k == text) |
There was a problem hiding this comment.
Invariant violation: is_keyword_text linearly rescans dag_all_keywords for every identifier token, violating the Performance Invariant and Facts Flow Forward preference for indexed O(1) lookup in hot paths
| if has_err(err: params_result.err) { return ItemPrefixResult { name: name, type_params: tp_result.params, params: [], inferred: none, uses: [], state: params_result.state, err: params_result.err } } | ||
|
|
||
| // 4. Return type (if form.has_return_type) | ||
| let ret_result = if form.has_return_type { |
There was a problem hiding this comment.
Invariant violation: parse_item_prefix ignores form.return_required and always parses return annotations optionally, so the new ItemForm fact is speculative metadata rather than an enforced boundary contract, violating Explicit boundary contracts
| parse_op_body_entries(tokens: tokens, state: adv.state, inputs: inputs, outputs: outputs, modifier_props: list_push(modifier_props, prop), transport: transport, exit_props: exit_props, response_props: response_props, mock_props: mock_props) | ||
| Some { value: ShKeyword } => { | ||
| let kw_text = tok.value.text | ||
| if kw_text == "input" { |
There was a problem hiding this comment.
Invariant violation: parse_op_body_entries switches on raw keyword text after keywords were moved into open SyntaxSpec data, violating No case enumeration for open sets
| if matching |> count > 0 { | ||
| match first(matching).value.binop { | ||
| Some { value: bk } => bk | ||
| None => Add |
There was a problem hiding this comment.
Invariant violation: find_operator_binop fabricates Add for missing or non-binary operator specs instead of failing, violating No fallbacks that fabricate
1. Tokenizer keyword scan O(n)→O(1): dag_keyword_set is Map<String,Bool> with hash lookup, replacing list scan on every identifier token. 2. return_required enforced: parse_item_prefix checks form.return_required and errors if no return annotation found for fn/extern declarations. 3. find_operator_binop returns BinOpKind? instead of fabricating Add. Callers produce parse error on None (unreachable for valid operators). Acknowledged: parse_op_body_entries keyword text switch (input/output/ idempotent/readonly/hermetic) is a domain sub-grammar within service operation bodies. Correct violation, but fix requires extending SyntaxSpec to describe sub-item grammars — beyond current Stream 0 scope. 150/150 v2-compiler-tests pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
briansrls
left a comment
There was a problem hiding this comment.
Review (3 violations, 3 good, 2 improve)
Invariant violations (3)
Modeling (good)
src/v2/00_core.dagReplacing the many ShKw variants with ShKeyword is a more faithful token model because token structure stays lexical while keyword identity lives in Token.text.src/v2/languages.dagSyntaxSpec, ItemForm, and OperatorSpec sit at the right shared compiler layer by defining parser vocabulary once and letting concrete frontends instantiate it as data.dsl/extdeps/languages/dag/syntax.dagdag_item_forms, dag_keyword_literals, and dag_operators move real .dag frontend facts out of parser code and into a language-specific spec instance, which is the right layering over the shared SyntaxSpec vocabulary.
Modeling (improve)
src/v2/languages.dagItemForm’s boolean matrix and OperatorSpec.binop? are less compositional than a small sum-typed grammar algebra because invalid combinations remain representable.dsl/extdeps/languages/dag/syntax.dagKeyword membership should be derived from the other syntax facts plus the few non-item structural keywords rather than stored as a parallel table.
The PR improves the parser’s compositional shape, but duplicate keyword authority and residual string-driven dispatch keep it short of the repo’s invariant bar.
| else if kw_text == "return" { parse_return(tokens: tokens, state: state) } | ||
| else if kw_text == "fn" { parse_fn_lambda(tokens: tokens, state: state) } | ||
| else { | ||
| // Keywords used as variable names (e.g., module, type, pattern). |
There was a problem hiding this comment.
Invariant violation: parse_primary still dispatches keyword-led expression forms through a raw string ladder (match/if/for/let/return/fn), which violates No case enumeration for open sets now that keyword identity is carried as open text rather than a closed enum.
| let prefix = parse_item_prefix(tokens: tokens, state: r.state, form: form) | ||
| if has_err(err: prefix.err) { return ItemResult { item: dummy, state: prefix.state, err: prefix.err } } | ||
| parse_block_body_from_prefix(tokens: tokens, prefix: prefix, start_span: start_span) | ||
| } | ||
|
|
There was a problem hiding this comment.
Invariant violation: parse_func_def reconstructs an inline ItemForm for func/pattern/interface instead of consuming SyntaxSpec as the authority, violating No duplicate representations and No parallel implementations.
| // The tokenizer and parser both derive from this list. | ||
|
|
||
| data dag_all_keywords: List<String> = [ | ||
| "module", "import", "type", "fn", "func", "service", "resource", |
There was a problem hiding this comment.
Invariant violation: dag_keyword_set duplicates keyword facts already present in dag_item_forms and dag_keyword_literals, so keyword changes require multiple edits and violate No duplicate representations.
Summary
ShKw*TokenShape variants into singleShKeyword; delete 70+ keyword predicates; all keyword dispatch now uses token textSyntaxSpec/ItemForm/BodyKindtypes;parse_itemdispatches viadag_item_formstable lookup — adding a new item type with standard body = 1 file edit (syntax.dag)infix_bpreads fromdag_operatorsspec table — adding a new operator = 1 entrytrue/false/none) reads fromdag_keyword_literalsmapNew file:
dsl/extdeps/languages/dag/syntax.dag— the .dag language is instance #1 of SyntaxSpec. Future language frontends provide their own spec.Net -170 lines. 148/148 v2-compiler-tests pass.
Test plan
cargo test -p v2-compiler-tests— 148 passed, 0 failedgrep -rc 'ShKw[A-Z]' src/v2/*.dag— 0 matches (all keyword variants eliminated)parse_item, 0 ininfix_bp🤖 Generated with Claude Code