feat(grammar): add executable frontend grammar shadows - #19
Merged
Conversation
|
Warning Review limit reached
Next review available in: 24 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe PR adds a restricted EBNF compiler and parser runtime, integrates build-time Rust generation into ChangesExecutable grammar compiler
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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.
Actionable comments posted: 10
🧹 Nitpick comments (6)
crates/rspdl-ko/Cargo.toml (1)
11-11: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep
rspdl-grammar-compilertest-only.
crates/rspdl-ko/src/generatedis compiled only under#[cfg(test)]. Move the line 11 entry to[dev-dependencies]; keep line 16 under[build-dependencies]forbuild.rs.🤖 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 `@crates/rspdl-ko/Cargo.toml` at line 11, Move the rspdl-grammar-compiler dependency entry used by test-only generated code from [dependencies] to [dev-dependencies] in the rspdl-ko Cargo manifest. Preserve the separate build-dependencies entry required by build.rs.crates/rspdl-grammar-compiler/src/compiler.rs (3)
382-399: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
reaches_ruleenumerates paths instead of nodes.The function removes
currentfromvisitingon the way out, so a node is re-explored once per incoming path. On a dense rule graph the work grows exponentially, andvalidate_left_recursioncalls it once per rule. Keep avisitedset of nodes that cannot reach the target, or compute strongly connected components of the FIRST graph once.🤖 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 `@crates/rspdl-grammar-compiler/src/compiler.rs` around lines 382 - 399, Update reaches_rule to memoize nodes proven unable to reach target, using a persistent visited/failed set rather than removing nodes from visiting after recursion. Check this set before traversal, record current when no path reaches target, and preserve cycle protection so dense graphs are not re-explored per incoming path.
193-228: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftDiagnostics point at the rule name for every nested error.
Exprcarries no offset, sovalidate_references_and_matchers,validate_repetition, andvalidate_left_recursionall reportrule.offset. A grammar file with a long rule gives a span that does not identify the failing operand. Carry the token offset on each parsed expression node so grammar diagnostics keep precise source spans, as the project requires for parser output.🤖 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 `@crates/rspdl-grammar-compiler/src/compiler.rs` around lines 193 - 228, Propagate token offsets into every parsed Expr node during parsing, then update validate_references_and_matchers, validate_repetition, and validate_left_recursion to report each offending expression’s offset instead of the enclosing rule.offset. Preserve rule offsets for errors that apply to the whole rule, while ensuring nested reference, matcher, repetition, and recursion diagnostics identify the precise operand span.
765-898: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd compiler tests for the rejected boundaries.
The suite covers syntax, duplicate, undefined rule, unknown matcher, nullable repetition, left recursion, and missing public rule. Add cases for
emit_rustwith an invalid function name, for an unterminated string literal, for an unsupported escape, and for byte-offset stability of eachCompileError. These tests pin the diagnostic contract that the migration harness compares against.🤖 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 `@crates/rspdl-grammar-compiler/src/compiler.rs` around lines 765 - 898, Extend the compiler test module with cases covering emit_rust rejecting an invalid function name, compilation rejecting unterminated string literals and unsupported escapes, and stable byte offsets for every CompileError variant exercised by these boundaries. Reuse the existing GrammarCompiler and CompileError assertions, and verify the exact diagnostic positions expected by the migration harness.crates/rspdl-grammar-compiler/src/runtime.rs (2)
156-185: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
from_generated_partsis public but its invariants are unchecked, soparsecan panic.
from_generated_partsaccepts anypublic_rulesandrulesvectors. If a public rule name has no matching rule, Line 185 panics. TheRuleRefarm at Line 250 panics in the same way for an undefined reference. Today onlyGrammarCompiler::compilebuilds grammars, so the invariants hold, but the constructor is reachable from outside the crate through#[doc(hidden)] pub.Either validate names inside
from_generated_partsand return aResult, or returnParseError::UnknownEntryand aNoMatchfailure instead of panicking. This keeps the runtime panic-free for arbitrary input.🤖 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 `@crates/rspdl-grammar-compiler/src/runtime.rs` around lines 156 - 185, The public constructor from_generated_parts must not allow malformed grammar data to cause parse panics. Validate that every public rule and RuleRef resolves to an entry in rules, preferably by changing construction to return a Result and rejecting undefined names; otherwise update parse to return ParseError::UnknownEntry or NoMatch for missing entries instead of using unchecked expect/panic paths.
435-536: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtend the runtime tests to the risky paths.
The three tests cover captures, farthest expectation, and ambiguity. Add cases for a capture over an optional expression, for a repeat over a matcher that returns several alternatives, and for repeated
parsecalls on the same input to pin determinism.🤖 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 `@crates/rspdl-grammar-compiler/src/runtime.rs` around lines 435 - 536, The runtime test module currently lacks coverage for optional captures, multi-alternative matcher repetition, and repeated parsing determinism. Add focused tests alongside captures_literal_and_contextual_matches, reports_the_farthest_expectation, and rejects_ambiguous_complete_parses: verify a capture wrapping an optional expression, verify repetition correctly handles multiple matches returned by Words::match_contextual, and invoke parse repeatedly on identical grammar/input while asserting identical results.
🤖 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.
Inline comments:
In `@crates/rspdl-grammar-compiler/src/compiler.rs`:
- Around line 178-184: Update is_rust_identifier to reject Rust reserved
keywords, including strict and contextual keywords as applicable, so emit_rust
cannot generate invalid function declarations. Ensure keyword-based grammar
filenames produce the existing CompileError path rather than emitting a bare
keyword function name.
- Around line 646-656: The parse_primary implementation uses a let chain that
requires Rust 1.88 while the project declares rust-version 1.85. Either rewrite
the condition using Rust 1.85-compatible control flow, or update the declared
minimum Rust version to 1.88, keeping the capture parsing behavior unchanged.
In `@crates/rspdl-grammar-compiler/src/runtime.rs`:
- Around line 212-338: Update evaluate and evaluate_repeat to enforce
configurable limits on total retained/generated outcomes and recursive RuleRef
depth, returning the existing structured ParseError when either limit is
exceeded instead of continuing or overflowing the stack. Add memoization for
rule evaluation keyed by rule name and input position, while preserving
capture/state behavior and existing parsing results within configured limits.
- Around line 286-299: The Expr::Capture runtime path drops outcomes when
combine_values produces no terminal, causing nullable capture bodies to reject
otherwise valid input. In crates/rspdl-grammar-compiler/src/runtime.rs:286-299,
preserve the outcome by recording an empty Capture at the current position, or
return a structured error; if choosing rejection, add the corresponding nullable
Expr::Capture validation and dedicated CompileErrorKind in
crates/rspdl-grammar-compiler/src/compiler.rs:300-324, with no direct change
required in the other site beyond consistency with that choice.
- Around line 345-357: Update combine_values and the attached-comparison parsing
flow so literal_comparison receives only the value captured by
`@integer_before`(...), excluding grammar literals such as “보다 커야 한다” or “보다 작아야
한다”. Ensure LiteralAst::Integer stores just the integer text for both
greater_attached and less_attached forms, and add coverage for both cases.
In `@docs/adr/0003-executable-frontend-grammar-compiler.md`:
- Line 20: Update the last_updated metadata in the ADR to the actual
review/update date, 2026-08-12, then regenerate docs/index.md so its generated
metadata or listing reflects the corrected source date.
In `@docs/architecture.md`:
- Around line 137-141: Update the migration-gate documentation around the
PARSE-to-SHADOW equivalence relationship to require checks for AST shape,
rejection boundaries, UTF-8 source ranges, recovery metadata, and structured
diagnostic spans, in addition to capture and acceptance equivalence, before
production cutover.
- Around line 116-120: Update the architecture graph around the rspdl-ko node to
add a separate build-time dependency edge to rspdl-grammar-compiler, while
preserving the existing runtime dependency edge and its current labeling.
In `@docs/index.md`:
- Line 29: Correct the source ADR front matter so its last-updated metadata is
not future-dated, then regenerate the generated index containing the
last_updated field. Ensure the rebuilt docs/index.md reflects the corrected
source metadata.
In `@docs/problems/0003-frontend-grammar-implementation-drift.md`:
- Around line 39-44: Revise the “How” section to remove prescribed
implementation and migration choices, including executable grammar formats,
differential parser testing, owning-layer tests, and production-by-production
rollout. Keep only the causal mechanism and observable evidence explaining the
frontend grammar implementation drift, while leaving solution and future
migration details to ADR 0003.
---
Nitpick comments:
In `@crates/rspdl-grammar-compiler/src/compiler.rs`:
- Around line 382-399: Update reaches_rule to memoize nodes proven unable to
reach target, using a persistent visited/failed set rather than removing nodes
from visiting after recursion. Check this set before traversal, record current
when no path reaches target, and preserve cycle protection so dense graphs are
not re-explored per incoming path.
- Around line 193-228: Propagate token offsets into every parsed Expr node
during parsing, then update validate_references_and_matchers,
validate_repetition, and validate_left_recursion to report each offending
expression’s offset instead of the enclosing rule.offset. Preserve rule offsets
for errors that apply to the whole rule, while ensuring nested reference,
matcher, repetition, and recursion diagnostics identify the precise operand
span.
- Around line 765-898: Extend the compiler test module with cases covering
emit_rust rejecting an invalid function name, compilation rejecting unterminated
string literals and unsupported escapes, and stable byte offsets for every
CompileError variant exercised by these boundaries. Reuse the existing
GrammarCompiler and CompileError assertions, and verify the exact diagnostic
positions expected by the migration harness.
In `@crates/rspdl-grammar-compiler/src/runtime.rs`:
- Around line 156-185: The public constructor from_generated_parts must not
allow malformed grammar data to cause parse panics. Validate that every public
rule and RuleRef resolves to an entry in rules, preferably by changing
construction to return a Result and rejecting undefined names; otherwise update
parse to return ParseError::UnknownEntry or NoMatch for missing entries instead
of using unchecked expect/panic paths.
- Around line 435-536: The runtime test module currently lacks coverage for
optional captures, multi-alternative matcher repetition, and repeated parsing
determinism. Add focused tests alongside
captures_literal_and_contextual_matches, reports_the_farthest_expectation, and
rejects_ambiguous_complete_parses: verify a capture wrapping an optional
expression, verify repetition correctly handles multiple matches returned by
Words::match_contextual, and invoke parse repeatedly on identical grammar/input
while asserting identical results.
In `@crates/rspdl-ko/Cargo.toml`:
- Line 11: Move the rspdl-grammar-compiler dependency entry used by test-only
generated code from [dependencies] to [dev-dependencies] in the rspdl-ko Cargo
manifest. Preserve the separate build-dependencies entry required by build.rs.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 496de8fc-68c4-4fae-b83f-84e66654e0bb
⛔ Files ignored due to path filters (8)
Cargo.lockis excluded by!**/*.lockcrates/rspdl-ko/src/generated/adapter.rsis excluded by!**/generated/**crates/rspdl-ko/src/generated/constraint.rsis excluded by!**/generated/**crates/rspdl-ko/src/generated/declarations.rsis excluded by!**/generated/**crates/rspdl-ko/src/generated/mod.rsis excluded by!**/generated/**crates/rspdl-ko/src/generated/policy.rsis excluded by!**/generated/**crates/rspdl-ko/src/generated/provenance.rsis excluded by!**/generated/**crates/rspdl-ko/src/generated/relation.rsis excluded by!**/generated/**
📒 Files selected for processing (21)
CONTRIBUTING.mdCargo.tomlcrates/rspdl-grammar-compiler/Cargo.tomlcrates/rspdl-grammar-compiler/README.mdcrates/rspdl-grammar-compiler/src/compiler.rscrates/rspdl-grammar-compiler/src/lib.rscrates/rspdl-grammar-compiler/src/runtime.rscrates/rspdl-ko/Cargo.tomlcrates/rspdl-ko/build.rscrates/rspdl-ko/src/grammar/constraint.ebnfcrates/rspdl-ko/src/grammar/declarations.ebnfcrates/rspdl-ko/src/grammar/policy.ebnfcrates/rspdl-ko/src/grammar/provenance.ebnfcrates/rspdl-ko/src/grammar/relation.ebnfcrates/rspdl-ko/src/lib.rsdocs/adr/0003-executable-frontend-grammar-compiler.mddocs/architecture.mddocs/index.mddocs/problems/0003-frontend-grammar-implementation-drift.mddocs/rfcs/0001-controlled-korean-surface-grammar.mddocs/rfcs/0004-natural-korean-domain-grammar.md
Comment on lines
+345
to
+357
| fn combine_values(values: &[Capture]) -> Option<Capture> { | ||
| let first = values.first()?; | ||
| let last = values.last()?; | ||
| Some(Capture { | ||
| value: values | ||
| .iter() | ||
| .map(|value| value.value.as_str()) | ||
| .collect::<Vec<_>>() | ||
| .join(" "), | ||
| start: first.start, | ||
| end: last.end, | ||
| }) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect generated Korean shadow parsers for multi-terminal capture consumption.
set -euo pipefail
fd -e rs . crates/rspdl-ko | while IFS= read -r file; do
rg -n -C 4 'captures\(|\.capture\(' "$file" || true
doneRepository: rspdl/rspdl-core
Length of output: 3432
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- runtime.rs ---'
sed -n '300,380p' crates/rspdl-grammar-compiler/src/runtime.rs
printf '%s\n' '--- Korean generated parsers and tests ---'
fd -e rs . crates/rspdl-ko | sort
rg -n -C 8 'generated_(constraint|provenance|screen|relation)|Capture|capture|captures|oracle\(|handwritten|field_left|field_right|model|screen_name|relation_name' crates/rspdl-ko
printf '%s\n' '--- grammars and capture declarations ---'
rg -n -C 5 'capture|field_operator|field_left|field_right|source_model|target_model|relation_name|screen_name|model' --glob '*.rspdl' --glob '*.toml' --glob '*.rs' crates docsRepository: rspdl/rspdl-core
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- combine_values and Capture definitions ---'
rg -n -C 12 'fn combine_values|struct Capture|impl.*Capture|value:.*String' crates/rspdl-grammar-compiler/src crates/rspdl-grammar-compiler
printf '%s\n' '--- provenance shadow parser ---'
sed -n '1,180p' crates/rspdl-ko/src/generated/provenance.rs
printf '%s\n' '--- provenance tests ---'
sed -n '300,390p' crates/rspdl-ko/src/generated/provenance.rs
printf '%s\n' '--- Korean grammar files ---'
fd -i -e rspdl -e toml -e txt . crates/rspdl-ko crates/rspdl-grammar-compiler docs | sort
printf '%s\n' '--- capture declarations, limited to grammar-like files ---'
rg -n -C 4 'capture|field|model|screen_name|source_model|target_model' \
crates/rspdl-ko/build.rs crates/rspdl-ko/grammar crates/rspdl-ko/src \
--glob '*.rspdl' --glob '*.toml' --glob '*.txt' --glob 'build.rs' \
| head -n 1200Repository: rspdl/rspdl-core
Length of output: 24160
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- combine_values and Capture definitions ---'
rg -n -C 12 'fn combine_values|struct Capture|impl.*Capture|value:.*String' crates/rspdl-grammar-compiler/src crates/rspdl-grammar-compiler
printf '%s\n' '--- provenance shadow parser ---'
sed -n '1,180p' crates/rspdl-ko/src/generated/provenance.rs
printf '%s\n' '--- provenance tests ---'
sed -n '300,390p' crates/rspdl-ko/src/generated/provenance.rs
printf '%s\n' '--- Korean grammar files ---'
fd -i -e rspdl -e toml -e txt . crates/rspdl-ko crates/rspdl-grammar-compiler docs | sort
printf '%s\n' '--- capture declarations, limited to grammar-like files ---'
rg -n -C 4 'capture|field|model|screen_name|source_model|target_model' \
crates/rspdl-ko/build.rs crates/rspdl-ko/grammar crates/rspdl-ko/src \
--glob '*.rspdl' --glob '*.toml' --glob '*.txt' --glob 'build.rs' \
| head -n 1200Repository: rspdl/rspdl-core
Length of output: 24160
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Korean build grammar construction ---'
sed -n '1,260p' crates/rspdl-ko/build.rs
printf '%s\n' '--- generated grammar source references ---'
rg -n -C 5 'provenance_grammar|surface_name|screen_name|target_model|source_model|Capture|capture\(' crates/rspdl-ko crates/rspdl-grammar-compiler --glob '*.rs'
printf '%s\n' '--- adapters that construct terminal values ---'
rg -n -C 12 'fn match_marked_ref|fn match_literal|TerminalMatch::new|surface_name_prefixes|canonical_id|comma_reference' crates/rspdl-ko/src
printf '%s\n' '--- runtime capture evaluation ---'
sed -n '120,345p' crates/rspdl-grammar-compiler/src/runtime.rsRepository: rspdl/rspdl-core
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Korean build grammar construction ---'
sed -n '1,260p' crates/rspdl-ko/build.rs
printf '%s\n' '--- generated grammar source references ---'
rg -n -C 5 'provenance_grammar|surface_name|screen_name|target_model|source_model|Capture|capture\(' \
crates/rspdl-ko crates/rspdl-grammar-compiler --glob '*.rs'
printf '%s\n' '--- adapters that construct terminal values ---'
rg -n -C 12 'fn match_marked_ref|fn match_literal|TerminalMatch::new|surface_name_prefixes|canonical_id|comma_reference' \
crates/rspdl-ko/src
printf '%s\n' '--- runtime capture evaluation ---'
sed -n '120,345p' crates/rspdl-grammar-compiler/src/runtime.rsRepository: rspdl/rspdl-core
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- grammar files ---'
fd -e ebnf . crates/rspdl-ko/src/grammar -x sh -c 'echo "--- $1"; cat -n "$1"' sh {} \;
printf '%s\n' '--- handwritten surface_name and marked_ref ---'
sed -n '1250,1285p' crates/rspdl-ko/src/parser.rs
sed -n '1350,1420p' crates/rspdl-ko/src/parser.rs
printf '%s\n' '--- generated adapter implementations ---'
sed -n '1,260p' crates/rspdl-ko/src/generated/adapter.rsRepository: rspdl/rspdl-core
Length of output: 13662
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
for path in sorted(Path("crates/rspdl-ko/src/grammar").glob("*.ebnf")):
text = path.read_text()
print(f"--- {path} ---")
for match in re.finditer(r'(?m)^\s*([A-Za-z_][A-Za-z0-9_]*)\s*:\s*(.*)$', text):
name, expr = match.groups()
print(f"{name}: {expr.strip()}")
PY
printf '%s\n' '--- handwritten parser values for provenance productions ---'
rg -n -C 18 'fn parse_field_intent|fn parse_screen|fn parse_sum_derivation|fn parse_recalculation|intent_words|operation_word' crates/rspdl-ko/src/parser.rs crates/rspdl-ko/src/generated/provenance.rs
printf '%s\n' '--- differential assertions and span assertions ---'
sed -n '250,430p' crates/rspdl-ko/src/generated/provenance.rsRepository: rspdl/rspdl-core
Length of output: 32124
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
for path in sorted(Path("crates/rspdl-ko/src/grammar").glob("*.ebnf")):
text = path.read_text()
print(f"--- {path} ---")
for match in re.finditer(r'(?m)^\s*([A-Za-z_][A-Za-z0-9_]*)\s*:\s*(.*)$', text):
name, expr = match.groups()
print(f"{name}: {expr.strip()}")
PY
printf '%s\n' '--- handwritten parser values for provenance productions ---'
rg -n -C 18 'fn parse_field_intent|fn parse_screen|fn parse_sum_derivation|fn parse_recalculation|intent_words|operation_word' \
crates/rspdl-ko/src/parser.rs crates/rspdl-ko/src/generated/provenance.rs
printf '%s\n' '--- differential assertions and span assertions ---'
sed -n '250,430p' crates/rspdl-ko/src/generated/provenance.rsRepository: rspdl/rspdl-core
Length of output: 32124
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- constraint capture consumers and tests ---'
sed -n '70,145p' crates/rspdl-ko/src/generated/constraint.rs
sed -n '145,285p' crates/rspdl-ko/src/generated/constraint.rs
printf '%s\n' '--- relation capture consumers ---'
sed -n '1,180p' crates/rspdl-ko/src/generated/relation.rs
printf '%s\n' '--- integer matcher implementations ---'
rg -n -C 15 'integer_before|fn integer|integer_match|TerminalMatch::new' crates/rspdl-ko/src/generated/constraint.rs crates/rspdl-ko/src/generated
printf '%s\n' '--- handwritten and generated constraint differential assertions ---'
rg -n -C 10 'greater_attached|less_attached|parse_constraint|generated_constraint|Integer\(' crates/rspdl-ko/src/generated/constraint.rs crates/rspdl-ko/src/parser.rsRepository: rspdl/rspdl-core
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- constraint capture consumers and tests ---'
sed -n '70,145p' crates/rspdl-ko/src/generated/constraint.rs
sed -n '145,285p' crates/rspdl-ko/src/generated/constraint.rs
printf '%s\n' '--- relation capture consumers ---'
sed -n '1,180p' crates/rspdl-ko/src/generated/relation.rs
printf '%s\n' '--- integer matcher implementations ---'
rg -n -C 15 'integer_before|fn integer|integer_match|TerminalMatch::new' \
crates/rspdl-ko/src/generated/constraint.rs crates/rspdl-ko/src/generated
printf '%s\n' '--- handwritten and generated constraint differential assertions ---'
rg -n -C 10 'greater_attached|less_attached|parse_constraint|generated_constraint|Integer\(' \
crates/rspdl-ko/src/generated/constraint.rs crates/rspdl-ko/src/parser.rsRepository: rspdl/rspdl-core
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
# Read-only model of the runtime path for the attached integer alternatives.
# integer_before("보다") yields one terminal value; the following grammar
# literals yield two more terminal values; combine_values joins all three.
for branch, number in [("greater_attached", "10"), ("less_attached", "10")]:
combined = " ".join([number, "커야" if branch == "greater_attached" else "작아야", "한다"])
source = f"{number}보다 {'커야' if branch == 'greater_attached' else '작아야'} 한다"
print(branch)
print("capture.value:", combined)
print("source span text:", source)
print("value is numeric:", combined.isdecimal())
print("value occurs in source:", combined in source)
PY
printf '%s\n' '--- generated constraint call sites ---'
rg -n -C 6 'parse_constraint\(|GeneratedConstraint|generated::constraint' crates/rspdl-ko crates --glob '*.rs'
printf '%s\n' '--- handwritten attached-integer behavior and tests ---'
rg -n -C 10 '보다 커야|보다 작아야|커야 한다|작아야 한다|greater_attached|less_attached' \
crates/rspdl-ko/src/parser.rs crates/rspdl-ko/src/generated/constraint.rs crates/rspdl-ko/src --glob '*.rs'Repository: rspdl/rspdl-core
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
# Read-only model of the runtime path for the attached integer alternatives.
for branch, number in [("greater_attached", "10"), ("less_attached", "10")]:
ending = "커야" if branch == "greater_attached" else "작아야"
combined = " ".join([number, ending, "한다"])
source = f"{number}보다 {ending} 한다"
print(branch)
print("capture.value:", combined)
print("source span text:", source)
print("value is numeric:", combined.isdecimal())
print("value occurs in source:", combined in source)
PY
printf '%s\n' '--- generated constraint call sites ---'
rg -n -C 6 'parse_constraint\(|GeneratedConstraint|generated::constraint' \
crates/rspdl-ko crates --glob '*.rs'
printf '%s\n' '--- handwritten attached-integer behavior and tests ---'
rg -n -C 10 '보다 커야|보다 작아야|커야 한다|작아야 한다|greater_attached|less_attached' \
crates/rspdl-ko/src/parser.rs \
crates/rspdl-ko/src/generated/constraint.rs \
crates/rspdl-ko/src --glob '*.rs'Repository: rspdl/rspdl-core
Length of output: 50372
Preserve only the integer in attached comparison captures.
greater_attached and less_attached include grammar literals, so combine_values produces "0 커야 한다" for 0보다 커야 한다. literal_comparison then stores this value as LiteralAst::Integer instead of "0". Capture only @integer_before(...) or strip the following literals before constructing the integer literal. Add coverage for both forms.
🤖 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 `@crates/rspdl-grammar-compiler/src/runtime.rs` around lines 345 - 357, Update
combine_values and the attached-comparison parsing flow so literal_comparison
receives only the value captured by `@integer_before`(...), excluding grammar
literals such as “보다 커야 한다” or “보다 작아야 한다”. Ensure LiteralAst::Integer stores
just the integer text for both greater_attached and less_attached forms, and add
coverage for both cases.
| - natural-korean-domain-grammar | ||
| problem_refs: | ||
| - frontend-grammar-implementation-drift | ||
| last_updated: "2026-08-13" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use an actual update date.
The current review date is August 12, 2026, but last_updated is 2026-08-13. Set the source date to the actual update date, then regenerate docs/index.md.
[skip_comment]
⛔ Skipped due to learnings
Learnt from: CR
Repo: rspdl/rspdl-core PR: 0
File: docs/architecture.md:0-0
Timestamp: 2026-08-08T09:36:58.880Z
Learning: Applies to docs/conformance/**/*.json : Treat golden files as specification contracts; do not approve changes through snapshot refresh alone—review them together with the corresponding grammar or semantic-change RFC.
Learnt from: jwsong98
Repo: rspdl/rspdl-core PR: 13
File: docs/rfcs/0005-field-provenance-and-sum-derivation.md:6-6
Timestamp: 2026-08-02T16:26:14.871Z
Learning: This repository uses the `Asia/Seoul` timezone for document metadata. Documentation created on `2026-08-03` KST can correspond to a review timestamp on `2026-08-02` UTC; evaluate date metadata in the repository timezone.
Learnt from: CR
Repo: rspdl/rspdl-core PR: 0
File: docs/problems/0001-data-lifecycle-modeling-gap.md:0-0
Timestamp: 2026-08-08T09:37:26.623Z
Learning: Applies to docs/problems/**/*.{md,mdx} : Feature proposals and RFCs must reference this problem using its stable ID, `data-lifecycle-modeling-gap`, in `problem_refs`.
Learnt from: CR
Repo: rspdl/rspdl-core PR: 0
File: docs/problems/0001-data-lifecycle-modeling-gap.md:0-0
Timestamp: 2026-08-08T09:37:26.623Z
Learning: Applies to docs/problems/**/*.{md,mdx} : Derived data must document input availability, recalculation timing, and behavior after source deletion.
Learnt from: CR
Repo: rspdl/rspdl-core PR: 0
File: docs/guides/problem-driven-development.md:0-0
Timestamp: 2026-08-08T09:37:39.264Z
Learning: Applies to docs/guides/**/*.{md,mdx} : Maintain traceability from `Problem Topic` to `PRD/RFC/ADR/Spec` to code to tests or diagnostics; solution documents must reference the relevant Problem Topic IDs through `problem_refs`.
Learnt from: CR
Repo: rspdl/rspdl-core PR: 0
File: docs/guides/problem-driven-development.md:0-0
Timestamp: 2026-08-08T09:37:39.264Z
Learning: A completed change must be traceable to its Problem Topic, use consistent terminology and Rule IDs across documentation, code, and fixtures, return explainable failure evidence, explicitly document unimplemented scope and `unknown` behavior, and must not be approved solely because test snapshots were updated.
Learnt from: CR
Repo: rspdl/rspdl-core PR: 0
File: docs/guides/problem-driven-development.md:0-0
Timestamp: 2026-08-08T09:37:39.264Z
Learning: Implement changes by linking all applicable Problem Topic IDs through `problem_refs`, selecting the smallest end-to-end vertical slice, specifying semantic rules and structured diagnostics first, then adding nearby unit tests and public conformance fixtures.
🤖 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 `@docs/adr/0003-executable-frontend-grammar-compiler.md` at line 20, Update the
last_updated metadata in the ADR to the actual review/update date, 2026-08-12,
then regenerate docs/index.md so its generated metadata or listing reflects the
corrected source date.
| - typed-domains-and-logic-core | ||
| - frontend-semantic-analysis-contract | ||
| last_updated: "2026-08-12" | ||
| last_updated: "2026-08-13" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Regenerate the index after correcting the source metadata.
last_updated: "2026-08-13" is future-dated relative to August 12, 2026. Correct the ADR front matter first, then rebuild this generated index.
🤖 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 `@docs/index.md` at line 29, Correct the source ADR front matter so its
last-updated metadata is not future-dated, then regenerate the generated index
containing the last_updated field. Ensure the rebuilt docs/index.md reflects the
corrected source metadata.
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
Why
The normative EBNF and the handwritten parser had to be maintained separately. This made every frontend production expensive to add and allowed the specification and implementation to drift. The generated parsers establish executable grammar infrastructure while preserving the current parser as a behavioral oracle.
Refs #18
Behavior and compatibility
The handwritten Korean parser remains the production path. Generated parsers are test-only shadows, so this PR does not change the public grammar, Locale AST, structured diagnostics, formatter output, lowering, or Canonical IR.
Structured diagnostic/recovery parity, production cutover, and handwritten parser removal remain follow-up phases tracked in #18.
Validation
./scripts/check.shrspdl-kotests and 9 grammar compiler testsSummary by CodeRabbit
New Features
Documentation