docs: expand CLAUDE.md with full build, architecture, and skills reference#1441
Merged
douenergy merged 3 commits intoCanner:mainfrom Mar 11, 2026
Merged
docs: expand CLAUDE.md with full build, architecture, and skills reference#1441douenergy merged 3 commits intoCanner:mainfrom
douenergy merged 3 commits intoCanner:mainfrom
Conversation
…rence Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
github-actions
bot
added
the
documentation
Contributor
📝 WalkthroughWalkthroughDocumentation expansion of Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/CLAUDE.md:
- Around line 11-18: Update the heading text that currently reads "Four main
modules:" to accurately reflect the list length (change it to "Five main
modules:") in the .claude/CLAUDE.md section that lists wren-core,
wren-core-base, wren-core-py, ibis-server, and mcp-server so the heading and the
bullet list agree.
- Around line 80-85: The prerequisites under the "Prerequisites for local
strategy (one-time setup):" section currently list a macOS-specific command
("brew install zig") as if universal; update this section to either scope the
macOS command (e.g., prefix "macOS:" before "brew install zig") or add
equivalent Linux package instructions (e.g., "Linux:" with appropriate
package-manager commands for Zig such as apt/yum/pacman or a link to Zig install
instructions) and keep the rustup target lines as a cross-platform note (or
label them "macOS / Linux"). Ensure the edit references the same heading
"Prerequisites for local strategy (one-time setup):" so readers on non-macOS
platforms see the correct install steps.
Comment on lines
+11
to
+18
| Four main modules: | ||
|
|
||
| - **wren-core/** — Rust semantic engine (Cargo workspace: `core/`, `sqllogictest/`, `benchmarks/`, `wren-example/`). Handles MDL analysis, query planning, logical plan optimization, and SQL generation via DataFusion. | ||
| - **wren-core-base/** — Shared Rust crate with manifest types (`Model`, `Column`, `Metric`, `Relationship`, `View`). Has optional `python-binding` feature for PyO3 compatibility. | ||
| - **wren-core-py/** — PyO3 bindings exposing wren-core to Python. Built with Maturin. | ||
| - **ibis-server/** — FastAPI web server (Python 3.11). Provides REST API for query execution, validation, and metadata. Uses Ibis framework for data source connectivity. | ||
| - **mcp-server/** — MCP server exposing Wren Engine to AI agents (Claude, Cline, Cursor). | ||
|
|
Contributor
There was a problem hiding this comment.
Fix the module count in this section.
Line 11 says “Four main modules,” but the list contains five entries (wren-core, wren-core-base, wren-core-py, ibis-server, mcp-server). Please make the heading and bullets agree.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/CLAUDE.md around lines 11 - 18, Update the heading text that
currently reads "Four main modules:" to accurately reflect the list length
(change it to "Five main modules:") in the .claude/CLAUDE.md section that lists
wren-core, wren-core-base, wren-core-py, ibis-server, and mcp-server so the
heading and the bullet list agree.
Comment on lines
+80
to
+85
| **Prerequisites for local strategy (one-time setup):** | ||
| ```bash | ||
| brew install zig | ||
| rustup target add aarch64-unknown-linux-gnu # Apple Silicon | ||
| rustup target add x86_64-unknown-linux-gnu # Intel Mac | ||
| ``` |
Contributor
There was a problem hiding this comment.
Scope these prerequisites to macOS or add a Linux variant.
brew install zig is macOS-specific, but the heading reads like universal setup for the local build strategy. That will mislead non-macOS contributors unless this is labeled explicitly or paired with Linux instructions.
📝 Suggested wording
-**Prerequisites for local strategy (one-time setup):**
+**Prerequisites for local strategy on macOS (one-time setup):**+If Linux is also supported for the local strategy, add the equivalent package-manager command there as well.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/CLAUDE.md around lines 80 - 85, The prerequisites under the
"Prerequisites for local strategy (one-time setup):" section currently list a
macOS-specific command ("brew install zig") as if universal; update this section
to either scope the macOS command (e.g., prefix "macOS:" before "brew install
zig") or add equivalent Linux package instructions (e.g., "Linux:" with
appropriate package-manager commands for Zig such as apt/yum/pacman or a link to
Zig install instructions) and keep the rustup target lines as a cross-platform
note (or label them "macOS / Linux"). Ensure the edit references the same
heading "Prerequisites for local strategy (one-time setup):" so readers on
non-macOS platforms see the correct install steps.
douenergy
approved these changes
Mar 11, 2026
2 tasks
nhaluc1005
pushed a commit
to nhaluc1005/text2sql-practice
that referenced
this pull request
Apr 3, 2026
…rence (Canner#1441) Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
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.
Summary
.claude/CLAUDE.mdwith comprehensive build & development commands for all modules (wren-core, wren-core-py, ibis-server, mcp-server, Docker)Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit