feat(cli-skills): strengthen self-learning guidance in wren-usage skill#1514
Merged
goldmedal merged 4 commits intoCanner:feat/cli-0.2.0from Apr 4, 2026
Merged
Conversation
Four prompt-level improvements to help agents learn more effectively: - Default-store: flip from "store only if confirmed" to "store unless there's a reason not to" — captures implicit confirmations from follow-up questions and silent-accept sessions - Step 2.5: complexity assessment + decomposition strategy before writing SQL — guides agents to split multi-metric / time-series questions into simpler sub-queries - Step 3: dry-plan pre-verification for complex queries — lightweight generator/evaluator split without a second LLM - Workflow 2: structured two-layer error diagnosis (MDL vs DB) with per-error-pattern fix tables — especially useful for small models that need an explicit decision tree Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
github-actions
bot
added
the
documentation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds Step 2.5 for question complexity assessment and optional decomposition; splits execution into simple vs. complex with Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant CLI as Wren-CLI
participant Planner as Wren-dry-plan
participant DB as Database
participant Memory as Wren-Memory
User->>CLI: Ask NL question
CLI->>CLI: Assess complexity (Step 2.5)
alt Simple query
CLI->>DB: Execute SQL (--sql)
DB-->>CLI: Results
else Complex query
CLI->>Planner: Validate SQL (wren dry-plan)
Planner-->>CLI: Plan / errors
alt Plan OK
CLI->>DB: Execute SQL (--sql)
DB-->>CLI: Results
else Plan error
CLI->>User: Layered diagnostics & remediation steps
end
end
alt Successful & NL-described
CLI->>Memory: wren memory store --nl ... --sql ...
Memory-->>CLI: Stored
else Failure / exploratory / user opt-out
CLI-->>Memory: (skip storage)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 |
![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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cli-skills/wren-usage/SKILL.md (1)
225-225:⚠️ Potential issue | 🟡 MinorDecision-tree wording is stale with the new default-store policy.
Line 225 says “Store confirmed query,” which conflicts with Step 4’s store-by-default behavior.
Suggested wording update
-Store confirmed query → wren memory store --nl "..." --sql "..." +Store successful query (default) → wren memory store --nl "..." --sql "..."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli-skills/wren-usage/SKILL.md` at line 225, Update the decision-tree wording that currently reads "Store confirmed query" to reflect the new store-by-default policy: replace that label with a phrase like "Store confirmed query (stored by default — use --no-store to opt out)" and ensure any nearby reference to Step 4 clarifies that storing is the default behavior; update the CLI example line "→ wren memory store --nl \"...\" --sql \"...\"" or its caption if necessary to indicate the default-store semantics and mention the flag to opt out.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli-skills/wren-usage/SKILL.md`:
- Around line 106-110: The "**Skip storing only when:**" list omits the "no NL
question/raw SQL" condition; edit that section to add a bullet like "The user
asked a no‑NL question or provided raw SQL (do not store)" so it matches the
exclusion described in memory.md, ensuring the wording aligns with the existing
rule in memory.md to prevent conflicting agent behavior.
---
Outside diff comments:
In `@cli-skills/wren-usage/SKILL.md`:
- Line 225: Update the decision-tree wording that currently reads "Store
confirmed query" to reflect the new store-by-default policy: replace that label
with a phrase like "Store confirmed query (stored by default — use --no-store to
opt out)" and ensure any nearby reference to Step 4 clarifies that storing is
the default behavior; update the CLI example line "→ wren memory store --nl
\"...\" --sql \"...\"" or its caption if necessary to indicate the default-store
semantics and mention the flag to opt out.
🪄 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
Run ID: a6dc0701-fc88-4f96-8af3-df97628f9a36
📒 Files selected for processing (2)
cli-skills/wren-usage/SKILL.mdcli-skills/wren-usage/references/memory.md
…ry.md Add the missing "no NL question / raw SQL" exclusion to SKILL.md Step 4 so both files list the same five skip conditions and agents see no conflict. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli-skills/wren-usage/SKILL.md`:
- Around line 69-75: The README's "Step 3" description calls simple queries
"single table" but the example invoked by wren --sql ('SELECT c_name,
SUM(o_totalprice) FROM orders JOIN customer ON orders.o_custkey =
customer.c_custkey GROUP BY 1 ORDER BY 2 DESC LIMIT 5') uses a JOIN; reconcile
this by either updating the Step 3 text to allow "single-table or simple
MDL-defined joins" (referencing the Step 2.5 note about "Simple JOINs") or by
replacing the example with a true single-table query (e.g., SELECT with GROUP BY
on one table) so the wording and the example are consistent.
🪄 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
Run ID: 1c2b99eb-79ec-4402-9b28-cbba95cfbc15
📒 Files selected for processing (1)
cli-skills/wren-usage/SKILL.md
The description said "single table" but the example used a JOIN. Updated to "single table or simple MDL-defined JOINs" to match both the example and the Step 2.5 decomposition guidance. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli-skills/wren-usage/SKILL.md`:
- Around line 77-78: Update the wording that classifies JOINs as complex so it
no longer overlaps with the "simple MDL-defined JOINs" rule: replace the broad
"JOINs" phrase on the complex-queries guidance with "non-trivial joins (e.g.,
non‑MDL-defined joins, multi-join chains, or joins combined with
subqueries/CTEs)" and explicitly state that MDL-defined/simple joins are still
handled as simple queries to prevent ambiguity between the entries referenced as
"simple MDL-defined JOINs" and the complex-query guidance.
🪄 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
Run ID: 042c457b-a3f0-4c50-abe7-bcf90117f69d
📒 Files selected for processing (1)
cli-skills/wren-usage/SKILL.md
…h simple MDL JOINs "JOINs" was too broad — MDL-defined JOINs should still go through the simple/direct path. Tighten to "non-trivial JOINs not covered by MDL relationships" to keep simple vs complex paths non-overlapping. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
goldmedal
added a commit
that referenced
this pull request
Apr 4, 2026
…ll (#1514) 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
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
wren dry-planbefore executing; simple queries still execute directlyFiles changed
cli-skills/wren-usage/SKILL.mdcli-skills/wren-usage/references/memory.mdTest plan
memory.md"Storing queries" — same exceptions, no contradiction🤖 Generated with Claude Code
Summary by CodeRabbit