Build the RockQL v0.1 compiler foundation - #2
Conversation
|
You've hit your review limit for the week, but don't worry you'll get some more next week! Contact us at hello@zenable.io if you want this rate limit to go away |
📝 WalkthroughWalkthroughAdds the RockQL Rust workspace and compiler MVP, including AST, parser, formatter, SQL generation, CLI, examples, documentation, and GitHub Actions workflows for validation, builds, releases, and security checks. ChangesRockQL compiler MVP
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant rockql
participant rockql-parser
participant rockql-sql
User->>rockql: compile input
rockql->>rockql-parser: parse(source)
rockql-parser-->>rockql: Query
rockql->>rockql-sql: compile(Query, Dialect)
rockql-sql-->>rockql: SQL
rockql-->>User: print SQL
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
|
You've hit your review limit for the week, but don't worry you'll get some more next week! Contact us at hello@zenable.io if you want this rate limit to go away |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b9a2c16046
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
compiler/rockql-formatter/src/lib.rs (1)
1-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFormatter logic looks correct; add round-trip tests.
The variant handling correctly matches the parser's brace/expression contracts (e.g.
select {...}, barefilter/derive/take). However, this new crate ships with no test module, unlikerockql-sqlwhich added arockql-parserdev-dependency and unit tests. A formatter's most important property—idempotent round-tripping (format(parse(format(q))) == format(q))—is exactly the kind of thing that should be pinned down with tests now, before more transform kinds are added.🤖 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 `@compiler/rockql-formatter/src/lib.rs` around lines 1 - 59, Add a unit-test module for the public format function covering representative queries with the supported Transform variants. Parse the formatted output and assert the round-trip property format(parse(format(query))) == format(query), including brace-based select/sort and bare filter/derive/take forms; add the necessary parser crate as a development dependency following the rockql-sql test setup.
🤖 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 @.github/workflows/release.yml:
- Around line 8-9: Update the workflow-level permissions to set contents to read
by default, then add a contents: write permission specifically within the
publish job. Keep the build jobs on read-only access while preserving publish’s
ability to write release contents.
- Around line 29-32: Update the macOS x86_64 matrix entry in the release
workflow to use the active macos-15-intel runner instead of macos-13, while
preserving its target, artifact, and binary values.
In @.github/workflows/security.yml:
- Around line 21-23: Remove continue-on-error: true from the “Review dependency
changes” step using actions/dependency-review-action@v4 so dependency review
failures block the pull request. Keep the action and step configuration
otherwise unchanged.
In `@compiler/rockql-cli/src/main.rs`:
- Around line 69-96: Update the "-" branch in parse so stdin is assigned through
the same duplicate-input validation as the path branch. Preserve "-" as the
stdin input when it is the first positional input, but return "only one input
file may be supplied" when input was already set.
In `@compiler/rockql-parser/src/lib.rs`:
- Around line 330-365: Update split_segments to treat a top-level `|` as a
pipeline delimiter only when it is not paired with an adjacent `|`, preserving
quoted-string and escape handling. Ensure expressions containing the `||`
operator remain within one segment, and add a regression test covering `filter
first_name || last_name = 'Ada'`.
In `@compiler/rockql-sql/src/lib.rs`:
- Around line 96-104: Update the filter-combination logic in the SQL
construction block to wrap every individual filter expression in parentheses
before appending it, while preserving the existing newline and AND joining
behavior. Ensure multiple filters retain their intended grouping when
expressions contain OR or NOT.
- Around line 50-82: Update the transform processing around the Derive, Select,
and Filter branches to handle derived aliases referenced by later selects or
filters. Either reject or warn at compile time for this unsupported pipeline
shape, or preserve derived columns and alias scope by generating the required
subquery/CTE; do not emit a bare alias reference in the projection or WHERE
clause.
---
Nitpick comments:
In `@compiler/rockql-formatter/src/lib.rs`:
- Around line 1-59: Add a unit-test module for the public format function
covering representative queries with the supported Transform variants. Parse the
formatted output and assert the round-trip property format(parse(format(query)))
== format(query), including brace-based select/sort and bare filter/derive/take
forms; add the necessary parser crate as a development dependency following the
rockql-sql test setup.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 18119384-6a4e-4b24-8dd6-d76506eea542
📒 Files selected for processing (28)
.github/workflows/android-build.yml.github/workflows/ci.yml.github/workflows/docs.yml.github/workflows/release.yml.github/workflows/rust-tests.yml.github/workflows/security.yml.github/workflows/wasm-build.yml.github/workflows/web-build.yml.gitignoreCODE_OF_CONDUCT.mdCONTRIBUTING.mdCargo.tomlREADME.mdSECURITY.mdcompiler/rockql-ast/Cargo.tomlcompiler/rockql-ast/src/lib.rscompiler/rockql-cli/Cargo.tomlcompiler/rockql-cli/src/main.rscompiler/rockql-formatter/Cargo.tomlcompiler/rockql-formatter/src/lib.rscompiler/rockql-parser/Cargo.tomlcompiler/rockql-parser/src/lib.rscompiler/rockql-sql/Cargo.tomlcompiler/rockql-sql/src/lib.rsdeny.tomldocs/reference/mvp-language.mdexamples/employees.rockqlexamples/users.rockql
|
@copilot please fix the merge conflicts in this pull request. |
What changed
from,filter,select,derive,sort, andtake|separated pipelinescompile,check,format, andastCLI commandsWhy
The repository only contained a licence and one-line README. This establishes a reviewable, original RockQL compiler core before the playground, visual pipeline, Android application, bindings, and advanced transforms are added.
Developer impact
The core example can now be parsed and compiled through the CLI design:
The web, Android, and WebAssembly workflows detect whether their components exist and skip cleanly until those directories are implemented.
Validation
Local Rust execution was not available in the connected environment, so GitHub Actions is the authoritative compile/test validation for this pull request.
Summary by CodeRabbit
New Features
Documentation