feat(loomgen): integrate generated line lexer skeleton#706
Merged
Conversation
📝 WalkthroughWalkthroughLine-mode lexer generation now emits ChangesLine lexer integration
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant LoomgenCLI
participant LineLexerEmitter
participant SkeletonIntegrator
participant FixtureFiles
participant CI
LoomgenCLI->>LineLexerEmitter: preflight line modes and emit helpers
LineLexerEmitter->>SkeletonIntegrator: provide generated_lex_<mode> names
SkeletonIntegrator->>FixtureFiles: create or migrate lexer_skeleton.g.mbt
FixtureFiles->>CI: regenerate and compile regression artifacts
CI->>FixtureFiles: verify generated files and run native tests
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
loomgen/main.mbt (1)
1023-1081: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winSkeleton delegate is written before the file it delegates to — an I/O failure mid-run leaves a non-compiling package.
write_outputs(...)(line 1023) writes/integrateslexer_skeleton.g.mbt, whoselex_<mode>functions now callgenerated_lex_<mode>(see lines 493-499, 516-518). That helper function is only written afterward, at lines 1069-1081. If@fs.write_string_to_fileforline_lexer_jobfails afterwrite_outputsalready succeeded, the committed/working-treelexer_skeleton.g.mbtends up delegating to a function that was never written, breaking compilation — exactly the "half-written output" hazard the surrounding comments (e.g. line 897-899) explicitly try to avoid for other jobs.Contrast with
regenerate_line_lexer_regression_fixtureinregenerate_fixtures.mbt(lines 82-89), which correctly writes the dependency (line_lexer_path) before the dependent (skeleton_path). The CLI path here has the order reversed.Move the line-lexer write before the
write_outputs(...)call so a write failure never leaves a dangling skeleton reference.🔧 Proposed fix: write the line-mode lexer before write_outputs()
+ // Write the preflighted line-mode lexer (already built + validated above). `#561` + // Written before write_outputs() so a write failure here never leaves + // lexer_skeleton.g.mbt delegating to a helper that was never written. + match line_lexer_job { + Some((path, content, _)) => { + `@fs.write_string_to_file`(path, content) catch { + _ => { + println("Error writing " + path) + `@sys.exit`(1) + return + } + } + println(" " + path) + } + None => () + } + match write_outputs( te, @@ Ok(_) => () } // Write the preflighted step lexer (already built + validated above). `#521` match lexer_job { ... } - // Write the preflighted line-mode lexer (already built + validated above). `#561` - match line_lexer_job { - Some((path, content, _)) => { - `@fs.write_string_to_file`(path, content) catch { - _ => { - println("Error writing " + path) - `@sys.exit`(1) - return - } - } - println(" " + path) - } - None => () - } - // Write the preflighted GrammarIr (already built + validated above). `#522` match grammar_ir_job { ... }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@loomgen/main.mbt` around lines 1023 - 1081, Move the line_lexer_job write block before the write_outputs(...) call so the generated line-mode lexer exists before lexer_skeleton.g.mbt is written. Preserve the existing error handling, exit behavior, and output messages for line_lexer_job, and remove its later duplicate block after write_outputs.
🧹 Nitpick comments (2)
loomgen/regenerate_fixtures.mbt (2)
150-186: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeeply nested
matchpyramid hurts readability; consider flattening.Five levels of nested
matchjust to run five sequential fixture jobs and stop at the first error. A loop over an array of thunks would be flatter and easier to extend when a sixth fixture is added.♻️ Proposed refactor using a loop over closures
fn regenerate_fixtures() -> Result[Unit, String] { let jobs : Array[() -> Result[Unit, String]] = [ fn() { regenerate_grammar_ir_fixture( "loomgen/fixtures/error_node_fixture.mbt", "loomgen/fixtures/error_node_fixture.g.mbt", "error_node_grammar_ir", ) }, fn() { regenerate_grammar_ir_fixture( "loomgen/fixtures/rule_grammar_fixture.mbt", "loomgen/fixtures/rule_grammar_fixture.g.mbt", "mini_grammar_ir", ) }, fn() { regenerate_grammar_ir_fixture( "loomgen/fixtures/separated_list_fixture.mbt", "loomgen/fixtures/separated_list_fixture.g.mbt", "separated_list_grammar_ir", ) }, fn() { regenerate_line_pattern_fixture( "loomgen/fixtures/line_pattern_fixture.mbt", "loomgen/fixtures/line_pattern_fixture.g.mbt", ) }, fn() { regenerate_line_lexer_regression_fixture( "fixtures/line_lexer_regression/src/line_lexer_src.mbt", "fixtures/line_lexer_regression/line_lexer.g.mbt", "fixtures/line_lexer_regression/lexer_skeleton.g.mbt", ) }, ] for job in jobs { match job() { Err(msg) => return Err(msg) Ok(_) => () } } Ok(()) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@loomgen/regenerate_fixtures.mbt` around lines 150 - 186, Refactor regenerate_fixtures to replace the nested match chain with an ordered array of zero-argument jobs and a loop. Execute each fixture regeneration sequentially, immediately return the first Err message, and return Ok(()) only after all five jobs succeed; preserve the existing fixture arguments and execution order.
7-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded source filename in error messages instead of a parameter.
Unlike
regenerate_grammar_ir_fixtureandregenerate_line_pattern_fixture(which format errors with thesource_pathparameter they receive),build_line_lexer_regression_outputshardcodes"line_lexer_src.mbt"in its error strings (lines 21, 24) even though it has nosource_pathparameter to keep it in sync. If the fixture's source path ever changes, these messages go stale silently.♻️ Proposed fix: thread source_path through
fn build_line_lexer_regression_outputs( source : String, existing_skeleton : String, + source_path : String, ) -> Result[(String, String), String] { let annotations = match parse_annotations(source) { Ok(annotations) => annotations Err(msg) => return Err(msg) } guard annotations.token_enum is Some(token_enum) else { - return Err("line_lexer_src.mbt has no `#loom.token` enum") + return Err(source_path + " has no `#loom.token` enum") } guard annotations.term_enum is Some(term_enum) else { - return Err("line_lexer_src.mbt has no `#loom.term` enum") + return Err(source_path + " has no `#loom.term` enum") }(and update the call site in
regenerate_line_lexer_regression_fixtureto passsource_path)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@loomgen/regenerate_fixtures.mbt` around lines 7 - 44, Thread a source_path parameter through build_line_lexer_regression_outputs and update regenerate_line_lexer_regression_fixture to pass it. Use source_path when constructing the missing token-enum and term-enum error messages, matching the parameterized error handling in the other fixture builders.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@loomgen/main.mbt`:
- Around line 1023-1081: Move the line_lexer_job write block before the
write_outputs(...) call so the generated line-mode lexer exists before
lexer_skeleton.g.mbt is written. Preserve the existing error handling, exit
behavior, and output messages for line_lexer_job, and remove its later duplicate
block after write_outputs.
---
Nitpick comments:
In `@loomgen/regenerate_fixtures.mbt`:
- Around line 150-186: Refactor regenerate_fixtures to replace the nested match
chain with an ordered array of zero-argument jobs and a loop. Execute each
fixture regeneration sequentially, immediately return the first Err message, and
return Ok(()) only after all five jobs succeed; preserve the existing fixture
arguments and execution order.
- Around line 7-44: Thread a source_path parameter through
build_line_lexer_regression_outputs and update
regenerate_line_lexer_regression_fixture to pass it. Use source_path when
constructing the missing token-enum and term-enum error messages, matching the
parameterized error handling in the other fixture builders.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c96b8654-110d-44c9-ba32-c39fac745d0b
📒 Files selected for processing (17)
.github/workflows/ci.ymldocs/README.mddocs/archive/completed-phases/2026-07-13-line-lexer-skeleton-integration.mddocs/decisions/2026-07-13-line-lexer-skeleton-integration.mddocs/superpowers/specs/2026-07-13-line-lexer-skeleton-design.mdfixtures/line_lexer_regression/lexer_skeleton.g.mbtfixtures/line_lexer_regression/line_lexer.g.mbtfixtures/line_lexer_regression/line_lexer_support.mbtfixtures/line_lexer_regression/regenerate.shloomgen/README.mdloomgen/emit_lexer_skeleton.mbtloomgen/emit_lexer_wbtest.mbtloomgen/emit_line_lexer.mbtloomgen/fixtures/line_pattern_fixture.g.mbtloomgen/main.mbtloomgen/regenerate_fixtures.mbtloomgen/regression_wbtest.mbt
💤 Files with no reviewable changes (1)
- fixtures/line_lexer_regression/line_lexer_support.mbt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
generated_lex_<mode>and delegate throughlexer_skeleton.g.mbtoverride points.--force-lexeras the explicit overwrite path.--regenerate-fixturesthe canonical fixture generator; propagate generation failures to the CLI and enforce formatted generated-artifact drift checks in CI.Verification
moon fmt --checkmoon check --deny-warn --target nativemoon test --target native— 3436 passed, 0 failedmoon test loomgen --target native— 182 passed, 0 failedmoon test fixtures/line_lexer_regression --target native— 2 passed, 0 failedgit diff --exit-codeclean.--regenerate-fixturesexits 1 when its fixture source is unavailable.Closes #699