docs(skills): add file-based connector docs, venv setup, and fix path/typo issues#1438
Conversation
…, and getting-started doc Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…o 1.1 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughDocumentation files updated with path clarifications and relative path changes; skill versions bumped for wren-connection-info (1.1 → 1.2) and wren-quickstart (1.0 → 1.1); expanded documentation on DuckDB file-based format and workspace path configurations. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 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>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
skills/wren-connection-info/SKILL.md (1)
120-137: Good documentation addition with a minor grammar fix.The file-based connector documentation is accurate and aligns with the implementation in
ibis-server/app/model/connector.py. The DuckDB section correctly specifies thaturlshould be a folder path, which matches how_list_duckdb_filesdiscovers.duckdbfiles in a directory.Minor grammar fix: "File based" should be hyphenated as "File-based" when used as a compound adjective.
,
📝 Suggested fix
-### File based (S3, Minio, GCS, local file) +### File-based (S3, Minio, GCS, local file)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/wren-connection-info/SKILL.md` around lines 120 - 137, Update the heading and any usages of the phrase "File based" in SKILL.md to the hyphenated form "File-based" (e.g., change the heading line "### File based (S3, Minio, GCS, local file)" to "### File-based (S3, Minio, GCS, local file)") to fix the compound-adjective grammar; no functional code changes required.docs/quickstart.md (1)
156-156: Workspace path inconsistency with other docs.This file still uses
~/wren-workspace(lines 73-74, 98, 132) whiledocs/getting_started_with_claude_code.mdandskills/wren-quickstart/SKILL.mdhave been updated to use${PWD}/wren-workspace. Users following different guides will see conflicting recommended paths.Consider aligning the workspace path convention across all documentation for consistency, or explicitly note both options with guidance on when to use each.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/quickstart.md` at line 156, The docs/quickstart.md contains inconsistent workspace path usage (using ~/wren-workspace) versus other docs that use ${PWD}/wren-workspace; update quickstart.md to use ${PWD}/wren-workspace everywhere it currently references ~/wren-workspace (including the Database folder example and any other occurrences), or alternatively add a single clarifying note near the top explaining both conventions and when to use each (example: use ${PWD}/wren-workspace for container/local reproducible paths and ~ for user-home shortcuts), ensuring references like "Database folder path" and any path examples match the chosen convention.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/getting_started_with_claude_code.md`:
- Around line 68-74: Update the inconsistent workspace path usage: replace the
hardcoded ~/wren-workspace in the docker run -v option with the same
${PWD}/wren-workspace used earlier (or introduce a single variable like
$WORKSPACE_PATH and use it in both Phase 1 and Phase 2). Locate the docker run
command that contains "-v ~/wren-workspace:/workspace \\" and change the
left-hand mount to reference ${PWD}/wren-workspace or $WORKSPACE_PATH so both
phases use the identical workspace path variable.
In `@skills/wren-quickstart/SKILL.md`:
- Line 57: Update the SKILL.md in the wren-mcp-setup skill to use the same
workspace path as wren-quickstart: replace the literal "~/wren-workspace"
occurrence with "${PWD}/wren-workspace" so Phase 3 (`@wren-mcp-setup`) does not
conflict with wren-quickstart; search for the exact token "~/wren-workspace" in
that SKILL.md and change it to "${PWD}/wren-workspace", preserving surrounding
commands (e.g., mkdir -p) and any examples.
---
Nitpick comments:
In `@docs/quickstart.md`:
- Line 156: The docs/quickstart.md contains inconsistent workspace path usage
(using ~/wren-workspace) versus other docs that use ${PWD}/wren-workspace;
update quickstart.md to use ${PWD}/wren-workspace everywhere it currently
references ~/wren-workspace (including the Database folder example and any other
occurrences), or alternatively add a single clarifying note near the top
explaining both conventions and when to use each (example: use
${PWD}/wren-workspace for container/local reproducible paths and ~ for user-home
shortcuts), ensuring references like "Database folder path" and any path
examples match the chosen convention.
In `@skills/wren-connection-info/SKILL.md`:
- Around line 120-137: Update the heading and any usages of the phrase "File
based" in SKILL.md to the hyphenated form "File-based" (e.g., change the heading
line "### File based (S3, Minio, GCS, local file)" to "### File-based (S3,
Minio, GCS, local file)") to fix the compound-adjective grammar; no functional
code changes required.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 88df2f93-6eaa-4184-8a8d-33ae7b8eeab6
📒 Files selected for processing (6)
docs/getting_started_with_claude_code.mddocs/quickstart.mdskills/index.jsonskills/versions.jsonskills/wren-connection-info/SKILL.mdskills/wren-quickstart/SKILL.md
| mkdir -p ${PWD}/wren-workspace | ||
| ``` | ||
|
|
||
| The completed workspace will look like: | ||
|
|
||
| ``` | ||
| /${PWD}/wren-workspace/ | ||
| ${PWD}/wren-workspace/ |
There was a problem hiding this comment.
Path fix is correct, but internal inconsistency remains.
The fix from /${PWD}/wren-workspace to ${PWD}/wren-workspace is correct — the leading slash would have created an invalid path.
However, there's an inconsistency within this file: Phase 1 (line 68) now uses ${PWD}/wren-workspace, but Phase 2's docker run command (line 139) still uses ~/wren-workspace:
-v ~/wren-workspace:/workspace \
Consider updating line 139 to match, or using a variable like $WORKSPACE_PATH that can be set to either location.
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 73-73: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/getting_started_with_claude_code.md` around lines 68 - 74, Update the
inconsistent workspace path usage: replace the hardcoded ~/wren-workspace in the
docker run -v option with the same ${PWD}/wren-workspace used earlier (or
introduce a single variable like $WORKSPACE_PATH and use it in both Phase 1 and
Phase 2). Locate the docker run command that contains "-v
~/wren-workspace:/workspace \\" and change the left-hand mount to reference
${PWD}/wren-workspace or $WORKSPACE_PATH so both phases use the identical
workspace path variable.
|
|
||
| ```bash | ||
| mkdir -p ~/wren-workspace | ||
| mkdir -p ${PWD}/wren-workspace |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for workspace path references across skill files
echo "=== Checking workspace path patterns in skills ==="
rg -n "wren-workspace" --type md skills/Repository: Canner/wren-engine
Length of output: 762
🏁 Script executed:
# Check Phase 3 in wren-quickstart and the getting_started docs reference
cat skills/wren-quickstart/SKILL.md | head -80Repository: Canner/wren-engine
Length of output: 3232
🏁 Script executed:
# Check the getting_started docs for path references
rg -n "wren-workspace" --type md docs/Repository: Canner/wren-engine
Length of output: 760
🏁 Script executed:
# Search for Phase 3 and `@wren-mcp-setup` in wren-quickstart
rg -n "Phase 3|@wren-mcp-setup" skills/wren-quickstart/SKILL.mdRepository: Canner/wren-engine
Length of output: 611
🏁 Script executed:
# Get full wren-quickstart file to verify Phase 3
wc -l skills/wren-quickstart/SKILL.mdRepository: Canner/wren-engine
Length of output: 97
Cross-skill path inconsistency requires follow-up fix.
wren-quickstart/SKILL.md uses ${PWD}/wren-workspace (line 57), but wren-mcp-setup/SKILL.md uses ~/wren-workspace (line 44). Since Phase 3 invokes @wren-mcp-setup, users may encounter conflicting default paths. Update wren-mcp-setup/SKILL.md to use ${PWD}/wren-workspace for consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/wren-quickstart/SKILL.md` at line 57, Update the SKILL.md in the
wren-mcp-setup skill to use the same workspace path as wren-quickstart: replace
the literal "~/wren-workspace" occurrence with "${PWD}/wren-workspace" so Phase
3 (`@wren-mcp-setup`) does not conflict with wren-quickstart; search for the exact
token "~/wren-workspace" in that SKILL.md and change it to
"${PWD}/wren-workspace", preserving surrounding commands (e.g., mkdir -p) and
any examples.
Summary
format: duckdb); bumped to v1.2/${PWD}/...to${PWD}/...; bumped to v1.1${PWD}path fixversions.jsonandindex.jsonto reflect new skill versionsTest plan
wren-connection-infoskill correctly describes file-based connector formats (S3, Minio, GCS, local, DuckDB)wren-quickstartvenv step renders correctly and workspace path resolves as expectedversions.jsonandindex.jsonare consistent with SKILL.md metadata versions🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
Documentation
Version Updates