parse: fix bare-body parallel — block's own parens wrap the parallel#25
Merged
Conversation
Follow-up to #24, which shipped parallel <*> support but with a bug: it required a REDUNDANT inner paren — accepting only ( ( a <*> b ) ), not the bare ( a <*> b ) that the original grammar and the LLM actually use. The bare form failed live in Slack with: script.Parse: 1:40: unexpected token "<*>" (expected ")") Root cause: the block body was a *parsedPipeline (sequential-only), and <*> was a property of an inner group only. So the block's own ( ... ) could not itself be a parallel. The original internal/agentscript grammar lets the block's own parens wrap a parallel (Statement = ( "(" Parallel ")" | Command )); this now matches. Grammar (pkg/script/parse.go): expr = pipeline ( <*> pipeline )* // 1 ⇒ sequential, 2+ ⇒ parallel pipeline = stage ( >=> stage )* stage = call | "(" expr ")" // group = nesting/precedence block = backend mode "(" expr ")" // body IS an expr → may be parallel The block body is now a parsedExpr; a parsedGroup wraps a parsedExpr. toAST: exprToAST emits ast.Parallel for multi-branch exprs, wrapped in a one-stage Pipeline so Block.Body stays an ast.Pipeline (the invariant Lower + the memory bridge rely on); groupToAST unwraps a parallel stage inside a group. The always-Pipeline body contract is preserved. Verified end to end through the new pipeline: memory static ( ssl_check "google.com" <*> dns_lookup "github.com" ) → parse → resolve → RunMemory → BOTH branches execute. (Exact Slack shape.) Plus 3-way, nested-then-pipe, and ( search >=> analyze <*> search >=> analyze ) >=> merge >=> ask "..." Tests: parse_parallel_test.go covers bare-body parallel (→ Parallel stage), 3-way, nested, sequential-is-not-parallel, complex-resolves. Replaced obsolete reject-parallel assertion. All prior parse tests pass. CI: vet, gofmt, staticcheck, go test -race ./pkg/..., build pass.
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
2 participants
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.
The bug (live in Slack, after #24)
#24 shipped
<*>support but required a redundant inner paren — it only accepted( ( a <*> b ) ), not the bare( a <*> b )the original grammar and the LLM actually use. The bare form failed with:That's the exact error from
@loom in memory: check the SSL for google.com and DNS for github.com at the same time.Root cause
The block body was a
*parsedPipeline(sequential-only);<*>lived only inside an inner group. So the block's own( ... )couldn't be a parallel. The originalinternal/agentscriptgrammar lets the block's own parens wrap a parallel — this now matches.Fix (
pkg/script/parse.go)Block body is now a
parsedExpr; a group wraps aparsedExpr.exprToASTemitsast.Parallelfor multi-branch exprs, wrapped in a one-stage Pipeline so Block.Body stays an ast.Pipeline (invariant preserved — no regression to existing parse tests).Verified end-to-end (the exact Slack shape)
Plus 3-way, nested-then-pipe, and
( search >=> analyze <*> search >=> analyze ) >=> merge >=> ask.Tests
parse_parallel_test.go: bare-body parallel → Parallel stage; 3-way; nested; sequential-is-not-parallel; complex-resolves. Replaced the obsolete reject-parallel assertion. All prior parse tests pass.go test -race ./pkg/...