Skip to content

feat(wren): serve agent skills and reference docs from the CLI - #2329

Merged
goldmedal merged 16 commits into
mainfrom
feat/refactor-cli
Jun 4, 2026
Merged

feat(wren): serve agent skills and reference docs from the CLI#2329
goldmedal merged 16 commits into
mainfrom
feat/refactor-cli

Conversation

@PaulChen79

@PaulChen79 PaulChen79 commented May 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Move the source of truth for agent workflow guides, reference docs, and prompt templates into the wren CLI. Replace the previous "five fat skills installed into the agent's skill directory" model with a single ~50-line discovery stub plus three new content-delivery commands: wren skills get/list, wren docs get/list, wren ask --guided/--direct.

Why: The old model installed ~/.claude/skills/wren-* markdown directly. That bundle drifted from the installed CLI (the markdown referenced commands and flags that didn't always match the user's pip install-ed version), and the agent loaded all five skills at session start whether it needed them or not. The new model ships the content inside the wrenai wheel itself, so the version an agent reads always matches the installed CLI, and the agent only pays for what it fetches.

Design discussion: .tmp/prds/2026-05-17-agent-cli-additive-refactor-zh.md (and English mirror). Modeled after vercel-labs/agent-browser's single-stub pattern.

What's new

CLI surfaces

# Workflow guides (5 bundled skills)
wren skills list
wren skills get onboarding              # entry point: setup + first query
wren skills get usage                   # day-to-day querying
wren skills get generate-mdl            # generate MDL from a DB schema
wren skills get dlt-connector           # SaaS via dlt → DuckDB → Wren project
wren skills get enrich-context          # business context (units, enums, cubes)
wren skills get <name> --full           # inline the skill's references/
wren skills get dlt-connector --script introspect_dlt   # fetch bundled script

# Reference docs (15 mirrored from docs/core/)
wren docs list
wren docs get <reference>               # connect, mdl, cubes, installation, quickstart, ...

# Prompt shaping (no execution — just renders to stdout)
wren ask "<question>" --guided          # strict task flow for weaker LLMs
wren ask "<question>" --direct          # minimal wrapping for stronger LLMs

wren ask has no default mode — the user must pick --guided or --direct (silently changing a default would alter agent behavior across an upgrade).

Discovery stub

skills/wren/SKILL.md is now a ~50-line stub that teaches the agent to fetch everything else from the CLI. npx skills add Canner/WrenAI installs it.

The five previously-shipped fat skill directories (skills/wren-onboarding, wren-usage, wren-generate-mdl, wren-enrich-context, wren-dlt-connector) have been deleted as part of this PR. They were briefly retained as 10-line redirect stubs during the migration; once the discovery stub + CLI delivery were validated, the redirects were dropped (commit 820b1f07). tests/unit/test_skill_stubs.py::test_deprecated_dirs_removed guards against regenerating them.

Reference docs mirror

docs/core/ is the source of truth; core/wren/src/wren/docs_content/refs/ is a synced mirror packaged into the wheel. scripts/sync_docs_content.py keeps them in lockstep: --check flags drift (including orphan files), sync rewrites the mirror. A new docs-sync-check CI job runs --check on every PR, and docs/core/** is part of the workflow's trigger paths so source-doc edits actually fire the check.

Commits

# Commit Scope
1 5008f0b1 feat(wren): add wren skills content delivery (get/list) with bundled usage skill Tracer bullet: end-to-end content delivery infra + first lifted skill (usage)
2 30266608 feat(wren): complete wren skills — 4 more skills + --full + --script Lift the remaining 4 skills, add --full/--script flags
3 e7c479b3 feat(wren): add wren docs get/list reference delivery + docs sync wren docs get/list + 15 reference mirrors + sync_docs_content.py
4 ad8ce56a feat(wren): add wren ask <prompt> --guided|--direct prompt shaping wren ask with two template modes; no default
5 5782a334 feat(skills): single discovery stub + deprecate the five fat skills New skills/wren/SKILL.md stub + temporary redirect stubs + marketplace.json/install.sh rewritten
6 9ba96f6f test(wren): guard that every wren <cmd> in served content is real CI guard — validates ~440 wren <cmd> invocations across served content against the real Typer command tree
7 2ab0842d docs: align README + docs/core with new CLI-served skills model README quickstart + docs/core/reference/{cli,skills}.md brought in sync
8 854965d1 fix(ci): drop obsolete versions.json check and lint lifted dlt script Remove .github/workflows/skills-check.yml (skill version-pinning is gone); ruff-fix the lifted introspect_dlt.py
9 1a3cd6ad fix(wren): served-content guard now catches unknown subcommands Close a false-negative in the guard (CodeRabbit major finding); fix one prose typo it caught
10 820b1f07 chore(skills): remove deprecated redirect-stub directories Close the redirect window; delete the 5 deprecated dirs; add test_deprecated_dirs_removed regression guard
11 66ca64c3 fix(ci): wire new tests into CI; harden served-content guard; declare .py artifacts goldmedal review follow-ups: pytestmark + discovery-stub scan + docs-sync CI gate + explicit .py artifact + version-stable cmd.commands walk

Each commit was independently code-reviewed before landing. Findings from review (regex over-trim, missing orphan detection, PyPI rename regression, multi-line backslash-continuation gap, unknown-subcommand false-negative) were folded back into the same commit via --amend while the branch was unpushed; review surfaced after the PR opened landed as new follow-up commits.

File layout

core/wren/src/wren/
├── skills_cli.py              # `wren skills` Typer subapp
├── skills_delivery.py         # importlib.resources helpers (get_skill, get_script, ...)
├── skills_content/            # bundled skills, packaged via hatch artifacts glob
│   ├── onboarding/SKILL.md
│   ├── usage/{SKILL.md, references/{memory,wren-sql}.md}
│   ├── generate-mdl/SKILL.md
│   ├── enrich-context/{SKILL.md, references/{gap_catalog,cube_proposals}.md}
│   └── dlt-connector/{SKILL.md, references/dlt_sources.md, scripts/introspect_dlt.py}
├── docs_cli.py                # `wren docs` extracted from cli.py + extended with get/list
├── docs_delivery.py
├── docs_content/refs/         # 15 mirrored reference docs
├── ask.py                     # render(template, prompt) — pure string substitution
├── ask_cli.py                 # `wren ask` Typer command (mutually-exclusive flags)
└── ask_templates/{guided,direct}.md.tmpl

scripts/sync_docs_content.py   # bidirectional sync + drift detector (CI gate)
skills/wren/SKILL.md           # discovery stub (sole entry point)

pyproject.toml extends the wheel artifacts glob to include src/wren/skills_content/**/*.md, src/wren/skills_content/**/*.py, src/wren/docs_content/refs/*.md, and src/wren/ask_templates/*.tmpl. Verified via unzip -l dist/wrenai-*.whl that every served file ships.

Migration

Before After
npx skills add Canner/WrenAI --skill '*' npx skills add Canner/WrenAI (single discovery stub)
/wren-onboarding slash command Agent runs wren skills get onboarding
~/.claude/skills/wren-usage/SKILL.md (installed locally) wren skills get usage (fetched from wheel)
wren-engine (PyPI) wrenai (renamed in parent commit; all install commands updated)
skills/wren-{onboarding,usage,…}/ directories in this repo Deleted — agents fetch via wren skills get

Verification

  • uv run pytest tests/unit/ -m unit (the exact CI invocation) — 325 passed, 38 skipped. The new 44 tests are now collected (previously deselected by -m unit).
  • tests/unit/test_cube_cli.py carries 5 pre-existing failures locally — traced to a missing cube_query_to_sql symbol in the locally installed wren-core-py==0.1.0 PyPI wheel; CI builds the wheel from source so they pass there.
  • New test suites: tests/unit/test_skills_cli.py, test_docs_cli_get.py, test_ask_cli.py, test_skill_stubs.py, test_served_content_guard.py (the CI guard).
  • The served-content guard introspects the real Typer command tree (via stable cmd.commands API) and walks every wren <cmd> snippet in skills_content/, docs_content/, ask_templates/, and the discovery stub at skills/wren/SKILL.md — any forward reference to a non-existent command, flag, or subcommand fails the build.
  • Verified wren skills get usage works both in editable install (uv sync) and from a freshly built wheel (uv pip install dist/wrenai-*.whl).
  • uv build produces a wheel that bundles every .md / .py / .tmpl referenced above, via explicit artifacts globs.
  • python core/wren/scripts/sync_docs_content.py --checkdocs_content in sync (15 references)., now wired into CI as the docs-sync-check job.

Test plan

  • CI: full pytest matrix (linux + macos)
  • CI: test_served_content_guard.py passes on the published wheel
  • CI: docs-sync-check job runs (and stays green) on PRs that touch docs/core/**
  • Downstream: pip install wrenai in a fresh venv, then wren skills list && wren skills get usage — should print the workflow guide
  • Downstream: npx skills add Canner/WrenAI installs skills/wren/SKILL.md (the new stub) into Claude Code's skill directory, and the agent successfully fetches wren skills get onboarding on prompt

🤖 Generated with Claude Code

PaulChen79 and others added 7 commits May 28, 2026 09:45
…d usage skill

Ship agent skill guides inside the wheel and serve them via `wren skills get
<name>` / `wren skills list`, so any shell-capable agent gets content that is
always version-aligned with the installed wren-engine — no agent-runtime skill
cache, no versions.json drift hack.

This tracer-bullet slice wires the delivery pipeline end-to-end with the `usage`
skill lifted into package data (the version-drift fetch section and the
frontmatter version field are trimmed). Existing commands are unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Lift the remaining four skill guides (onboarding, generate-mdl, dlt-connector,
enrich-context) into package data so `wren skills get <name>` covers all five.
Apply the same trims as the usage slice: drop the version field and the
versions.json fetch hack, repoint cross-skill references to `wren skills get
<name>`, and repoint bundled external doc links to `wren docs get <reference>`,
while keeping all substantive flow content.

Add progressive disclosure: `wren skills get <name> --full` inlines the skill's
references/*.md; `wren skills get <name> --script <s>` returns a bundled script
(e.g. dlt-connector's introspect_dlt). references and the script ship in the
wheel; evals are intentionally excluded.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Extract the `docs` sub-app into docs_cli.py (connection-info behavior
unchanged) and add `wren docs get <reference>` / `wren docs list`. References
are a curated mirror of docs/core/ shipped in the wheel under
docs_content/refs/, kept in sync by scripts/sync_docs_content.py (with a
--check gate for CI). docs/core/ stays the source of truth.

Also reconcile the generate-mdl skill's forward reference to a real reference
name (`wren docs get cubes`); a test enforces that every `wren docs get <ref>`
mentioned in bundled skills resolves to a known reference.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`wren ask` wraps a user's natural-language question in one of two bundled
prompt templates and prints the rendered result to stdout. It does not run
any query — it produces a prompt for an agent to consume.

Modes are mutually exclusive and one must be chosen explicitly (no default):
--guided prepends a strict TASK TYPE A/B task flow for weaker LLMs;
--direct does minimal wrapping for stronger LLMs that decide on their own.
No default exists on purpose: silently changing a default would alter agent
behavior across an upgrade.

Templates ship as package data and use a single `<USER_PROMPT>` placeholder
substituted at render time.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add skills/wren/SKILL.md — a single ~50-line discovery stub modelled on
vercel-labs/agent-browser. Its frontmatter description concentrates the
triggers from all five fat skills, and its body points an AI client at the
four CLI surfaces (wren skills get/list, wren docs get/list, wren ask).

The five previously-shipped fat skills (wren-onboarding, wren-usage,
wren-generate-mdl, wren-dlt-connector, wren-enrich-context) become ~17-line
redirect stubs that preserve their original frontmatter description (for
trigger fidelity) and tell the agent to run `wren skills get <name>` instead.
Their stale references/ / scripts/ / evals/ subdirs are removed; the lifted
content has been in package data since the earlier slices. Kept for one
release as part of the deprecation window.

Drop versions.json + check-versions.sh — content travels with the wheel, so
version drift is impossible by construction. install.sh, index.json,
marketplace.json, SKILLS.md, README.md, and AUTHORING.md all rewritten to
reflect the new model (and to direct new-skill authors at
core/wren/src/wren/skills_content/, not the skills/ tree).

A guard test ensures redirect stubs stay minimal, never mention unrelated
CLI surfaces (`wren ask`, `wren docs get`) that would rot independently,
and never re-grow their stale subdirs.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Introspect the Typer/Click command tree at test time and walk every
markdown / template under wren/skills_content/, wren/docs_content/refs/
and wren/ask_templates/, extracting each `wren <invocation>` snippet.
Each invocation's command path is verified against the real registered
tree; --flags mentioned are verified against that command's actual flag
set. Prose like "the wren engine" or "in the wren project layout" is
filtered out (first token must be a known top-level command).

`wren memory` is allow-listed (it's conditionally registered behind the
`wren-engine[memory]` extras; its flags are skipped under the same
condition until the extras are installed in CI).

Closes the forward-reference failure mode the earlier slices introduced
(skill content pointing at `wren docs get` / `--full` before they
existed). By the time the branch is ready to ship, every command and
flag mentioned in served content must actually exist in the CLI.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
README quickstart and "two beats" snippets switched to the v0.8 flow
(install wrenai → install discovery stub → ask agent → wren skills get).
docs/core/reference/cli.md extended with `wren skills`, `wren docs
get/list`, and `wren ask --guided/--direct`. docs/core/reference/skills.md
rewritten to describe the single-discovery-stub model, the
`wren skills get` delivery, and the in-wheel bundle layout.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added documentation Improvements or additions to documentation dependencies Pull requests that update a dependency file python Pull requests that update Python code core skills labels May 28, 2026
@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

This PR replaces the prior on-disk skill bundle with a CLI-served discovery-stub model, adds prompt templating for wren ask, bundles reference docs/skills into the wheel, provides a sync/check script, wires new wren docs and wren skills subcommands, and adds tests validating served content and CLI references.

Changes

CLI-Served Skill & Docs Distribution System

Layer / File(s) Summary
Ask prompt system
core/wren/src/wren/ask.py, core/wren/src/wren/ask_cli.py, core/wren/src/wren/ask_templates/*.md.tmpl
wren ask with required single-mode --guided/--direct, template-based render(mode, prompt), and placeholder substitution.
Reference docs delivery
core/wren/src/wren/docs_delivery.py, core/wren/src/wren/docs_cli.py, core/wren/scripts/sync_docs_content.py
Curated REFERENCE_SOURCES mapping, list_references()/get_reference(), wren docs CLI (connection-info, list, get), and sync/check script to mirror docs into wheel package data.
Skills delivery system
core/wren/src/wren/skills_delivery.py, core/wren/src/wren/skills_cli.py, core/wren/src/wren/skills_content/*/SKILL.md
Wheel-packaged skill content serving: list_skills(), get_skill(full), get_script(), and wren skills CLI with --full/--script. Includes skill guides for onboarding, usage, generate-mdl, enrich-context, dlt-connector.
CLI wiring
core/wren/src/wren/cli.py
Mounts docs_app, registers ask and skills_app, centralizing CLI subapps.
Reference documentation
core/wren/src/wren/docs_content/refs/*.md
Adds many reference docs (architecture, MDL, context, memory, quickstart, connect, dbt-integration, manage-project, operational, refine, cubes, etc.).

Skill Distribution Model and Wheel Configuration

Layer / File(s) Summary
Discovery stub & distribution metadata
skills/wren/SKILL.md, skills/index.json, skills/install.sh, skills/.claude-plugin/marketplace.json
Introduces wren discovery stub, updates index.json to discovery + deprecated redirect stubs, simplifies install script to install only stub, updates marketplace metadata, removes versions.json and check-versions.sh.
Wheel build configuration
core/wren/pyproject.toml
Includes skill/docs Markdown and ask templates in wheel artifacts globs for packaging.

Testing and Quality Gates

Layer / File(s) Summary
Ask and docs tests
core/wren/tests/unit/test_ask_cli.py, core/wren/tests/unit/test_docs_cli_get.py
Unit tests for wren ask rendering/flags and wren docs list/get behavior, content matching, and CI sync gate.
Skills tests
core/wren/tests/unit/test_skills_cli.py
Tests for wren skills list/get, --full inlining, script retrieval, and negative paths.
Served-content guard & stub tests
core/wren/tests/unit/test_served_content_guard.py, core/wren/tests/unit/test_skill_stubs.py
Scans served content for wren ... invocations, validates command-paths and long flags against the live CLI, and asserts distribution stub expectations.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • goldmedal
  • douenergy
  • chilijung

"🐰
I taught the CLI to fetch and sing,
Docs bundled tight, prompts trimmed with spring.
One stub to rule the agent's gentle quest,
Hop in, install, and let the skills do the rest."

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/refactor-cli

@PaulChen79 PaulChen79 self-assigned this May 28, 2026
Two CI failures surfaced once PR #2329 fired:

1. `.github/workflows/skills-check.yml` ran `bash skills/check-versions.sh`,
   but ticket 4 deleted both the script and `skills/versions.json`. The new
   delivery model doesn't pin skill versions — content is served from the
   installed `wrenai` wheel — so the version-parity check is no longer
   meaningful. Equivalent integrity is now enforced by the served-content
   guard added in ticket 6 (`tests/unit/test_served_content_guard.py`).

2. `core/wren/src/wren/skills_content/dlt-connector/scripts/introspect_dlt.py`
   was lifted into `src/` in ticket 1b. Ruff scans `src/`, so the original
   script's three F541 (f-string without placeholders) hits and one
   long-line block now fail `ruff check` / `ruff format --check`. Auto-fix
   only — semantics unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added the ci label May 28, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (1)
core/wren/src/wren/docs_content/refs/quickstart.md (1)

180-187: ⚡ Quick win

Add language specifiers to fenced code blocks.

Several code blocks are missing language identifiers, which can affect syntax highlighting and rendering. Consider adding appropriate language specifiers:

  • Lines 180-187, 298-317: directory tree structures → add text or plaintext
  • Lines 220-223, 249-259: user prompts/natural language queries → add text if highlighting is desired, or leave plain if the unmarked style is intentional
📝 Proposed fix for language specifiers
-```
+```text
 ~/jaffle-wren/
 ├── wren_project.yml        # project metadata
 ...

For user prompts at lines 220, 249, 253, 257:

-```
+```text
 Use the wren-generate-mdl skill to explore the jaffle_shop database
 ...

Also applies to: 220-223, 249-259, 298-317

🤖 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 `@core/wren/src/wren/docs_content/refs/quickstart.md` around lines 180 - 187,
Add language specifiers to the fenced code blocks that currently lack them: for
the directory-tree block that begins with "~/jaffle-wren/" (the tree listing
wren_project.yml, models/, views/, relationships.yml, instructions.md) and for
the natural-language prompt blocks such as the ones that start with "Use the
wren-generate-mdl skill to explore the jaffle_shop database" and other user
prompt examples; update each opening fence from ``` to ```text (or ```plaintext)
so these blocks are explicitly marked as plain text for proper syntax
highlighting and rendering.
🤖 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 `@core/wren/src/wren/docs_content/refs/manage-project.md`:
- Line 173: In manage-project.md fix the broken link by replacing the current
'./osi.md' target with the correct OSI guide location: update the markdown link
in the table row containing "OSI `semantic_model.yaml`" so it points to the
actual OSI guide file in the repository (replace the './osi.md' href with the
repo's OSI guide path) ensuring the link resolves from manage-project.md; locate
the link by searching for the literal './osi.md' in the file.

In `@core/wren/src/wren/docs_content/refs/mdl.md`:
- Line 237: The table row for `properties` in mdl.md is missing the Type column,
breaking the 4-column table; update the row for the `properties` entry to
include the missing Type cell (e.g., add a Type like `object` or the correct
type) so the row has four pipe-separated cells matching the header and restoring
proper table alignment for the `properties` row.

In `@core/wren/src/wren/skills_delivery.py`:
- Around line 72-79: get_script currently picks the first match from
scripts_dir.iterdir() and is non-deterministic when multiple files share the
same stem; modify get_script(name: str, script: str) to collect all matching
files whose stem equals script, and then: if none found raise
ScriptNotFoundError, if more than one found raise a new ScriptAmbiguousError
(include candidate filenames in the message) so callers get a deterministic
error instead of unpredictable behavior; additionally update _script_stems() to
return a sorted list of unique stems (deduplicate with a set and sort) so
listings are stable and don't show duplicates.

---

Nitpick comments:
In `@core/wren/src/wren/docs_content/refs/quickstart.md`:
- Around line 180-187: Add language specifiers to the fenced code blocks that
currently lack them: for the directory-tree block that begins with
"~/jaffle-wren/" (the tree listing wren_project.yml, models/, views/,
relationships.yml, instructions.md) and for the natural-language prompt blocks
such as the ones that start with "Use the wren-generate-mdl skill to explore the
jaffle_shop database" and other user prompt examples; update each opening fence
from ``` to ```text (or ```plaintext) so these blocks are explicitly marked as
plain text for proper syntax highlighting and rendering.
🪄 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: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: d461247b-94c9-4365-b9a8-44607dfabbe6

📥 Commits

Reviewing files that changed from the base of the PR and between d43582c and 854965d.

📒 Files selected for processing (61)
  • .github/workflows/skills-check.yml
  • README.md
  • core/wren/pyproject.toml
  • core/wren/scripts/sync_docs_content.py
  • core/wren/src/wren/ask.py
  • core/wren/src/wren/ask_cli.py
  • core/wren/src/wren/ask_templates/direct.md.tmpl
  • core/wren/src/wren/ask_templates/guided.md.tmpl
  • core/wren/src/wren/cli.py
  • core/wren/src/wren/docs_cli.py
  • core/wren/src/wren/docs_content/refs/architecture.md
  • core/wren/src/wren/docs_content/refs/connect.md
  • core/wren/src/wren/docs_content/refs/correctness.md
  • core/wren/src/wren/docs_content/refs/cubes.md
  • core/wren/src/wren/docs_content/refs/dbt-integration.md
  • core/wren/src/wren/docs_content/refs/installation.md
  • core/wren/src/wren/docs_content/refs/manage-project.md
  • core/wren/src/wren/docs_content/refs/mdl.md
  • core/wren/src/wren/docs_content/refs/memory-system.md
  • core/wren/src/wren/docs_content/refs/model.md
  • core/wren/src/wren/docs_content/refs/operational.md
  • core/wren/src/wren/docs_content/refs/quickstart.md
  • core/wren/src/wren/docs_content/refs/refine.md
  • core/wren/src/wren/docs_content/refs/what-is-context.md
  • core/wren/src/wren/docs_content/refs/what-is-mdl.md
  • core/wren/src/wren/docs_delivery.py
  • core/wren/src/wren/skills_cli.py
  • core/wren/src/wren/skills_content/dlt-connector/SKILL.md
  • core/wren/src/wren/skills_content/dlt-connector/references/dlt_sources.md
  • core/wren/src/wren/skills_content/dlt-connector/scripts/introspect_dlt.py
  • core/wren/src/wren/skills_content/enrich-context/SKILL.md
  • core/wren/src/wren/skills_content/enrich-context/references/cube_proposals.md
  • core/wren/src/wren/skills_content/enrich-context/references/gap_catalog.md
  • core/wren/src/wren/skills_content/generate-mdl/SKILL.md
  • core/wren/src/wren/skills_content/onboarding/SKILL.md
  • core/wren/src/wren/skills_content/usage/SKILL.md
  • core/wren/src/wren/skills_content/usage/references/memory.md
  • core/wren/src/wren/skills_content/usage/references/wren-sql.md
  • core/wren/src/wren/skills_delivery.py
  • core/wren/tests/unit/test_ask_cli.py
  • core/wren/tests/unit/test_docs_cli_get.py
  • core/wren/tests/unit/test_served_content_guard.py
  • core/wren/tests/unit/test_skill_stubs.py
  • core/wren/tests/unit/test_skills_cli.py
  • docs/core/reference/cli.md
  • docs/core/reference/skills.md
  • skills/.claude-plugin/marketplace.json
  • skills/AUTHORING.md
  • skills/README.md
  • skills/SKILLS.md
  • skills/check-versions.sh
  • skills/index.json
  • skills/install.sh
  • skills/versions.json
  • skills/wren-dlt-connector/SKILL.md
  • skills/wren-dlt-connector/evals/evals.json
  • skills/wren-enrich-context/SKILL.md
  • skills/wren-generate-mdl/SKILL.md
  • skills/wren-onboarding/SKILL.md
  • skills/wren-usage/SKILL.md
  • skills/wren/SKILL.md
💤 Files with no reviewable changes (4)
  • skills/wren-dlt-connector/evals/evals.json
  • .github/workflows/skills-check.yml
  • skills/versions.json
  • skills/check-versions.sh

Comment thread core/wren/src/wren/docs_content/refs/manage-project.md Outdated
Comment thread core/wren/src/wren/docs_content/refs/mdl.md Outdated
Comment thread core/wren/src/wren/skills_delivery.py
Comment thread core/wren/tests/unit/test_served_content_guard.py
PaulChen79 and others added 3 commits May 28, 2026 14:14
CodeRabbit review on PR #2329 flagged a major gap in the served-content
guard added by ticket 6: `_findings()` only validated leftover `--flag`
tokens against the resolved command's allowed flag set. Plain-word
leftover tokens — i.e. unknown subcommands sitting on a group node —
silently passed.

Concretely, content saying `wren docs typo` resolved to cmd_path="docs",
leftover=["typo"], and the loop only inspected `--`-prefixed tokens, so
the typo escaped. The guard's stated intent is "every `wren <cmd>`
mentioned in served content must resolve to a real CLI command", so this
was a real false negative.

Fix: in `_findings()`, immediately after `_resolve()`, if cmd_path is a
group (has at least one child registered in COMMANDS) and the first
leftover token is a plain word, flag it as an unknown subcommand. This
runs before `_SKIP_FLAG_VALIDATION_FOR_GROUPS` so typos under `memory`
are also caught. Leaf commands taking positional args
(`wren skills get usage`) are unaffected because they have no children.

Adding the check immediately surfaced a real prose-vs-invocation
ambiguity in `generate-mdl/SKILL.md`: "A wren profile configured
(`wren profile add`) …" was parsed as the invocation `wren profile
configured`. Reworded to "A connection profile (set up via `wren profile
add`)" — clearer for human readers and unambiguous to the guard.

Also made `relative_to(_REPO)` resilient so the new regression test can
inject content from a `tmp_path` outside the repo.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The five `skills/wren-{onboarding,usage,generate-mdl,dlt-connector,
enrich-context}/` redirect stubs were kept for one release after the
new model landed (ticket 4) so anyone who had previously run
`npx skills add Canner/WrenAI --skill '*'` and force-refreshed via the
old install.sh path would still resolve to *something*. The redirect
window is now closed:

- The discovery stub at `skills/wren/` is the sole entry point.
- `marketplace.json` already advertised only `wren`; `index.json` listed
  the five as DEPRECATED redirects.
- No content in the repo links to `skills/wren-<name>/` anymore (the
  served-content guard validates that nothing under skills_content /
  docs_content / ask_templates references them).

Changes:
- Delete `skills/wren-{onboarding,usage,generate-mdl,dlt-connector,
  enrich-context}/` (5 directories, 5 SKILL.md files).
- Simplify `skills/install.sh` to install a single skill (`wren`); drop
  the multi-skill expansion / dependency-resolution logic that only
  existed to handle the deprecated dirs.
- Drop the five DEPRECATED entries from `skills/index.json`.
- Drop "Deprecation window" sections from `skills/SKILLS.md` and
  `skills/README.md`; update the install.sh usage line.
- Replace the parametrized redirect-stub tests in `test_skill_stubs.py`
  with a `test_deprecated_dirs_removed` guard so a future regeneration
  of those dirs fails loudly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@goldmedal

Copy link
Copy Markdown
Collaborator

Review findings

Functionally solid and low-risk (no Rust / ibis-server / PyO3 / MDL-schema surface touched). Findings below in severity order.

🔴 High — the new tests (incl. the served-content guard) don't run in CI

CI runs uv run pytest tests/unit/ -v -m unit (wren-ci.yml). The unit marker is opt-in via module-level pytestmark = pytest.mark.unit (e.g. tests/unit/test_config.py); conftest.py only registers the marker, there's no path-based auto-marking. None of the 5 new test modules declare it:

  • test_served_content_guard.py, test_skills_cli.py, test_docs_cli_get.py, test_ask_cli.py, test_skill_stubs.py

Verified against the built wheel: pytest <those 5 files> -m unit44 deselected. They pass only when run without the filter — which masks the gap. So the served-content guard and all new CLI tests are silently skipped in CI.
Fix: add pytestmark = pytest.mark.unit to each of the 5 modules.

🟡 Medium — drift hole: the discovery stub is shipped to users but not guarded

npx skills add installs skills/wren/SKILL.md to users' local skill dirs (~/.claude/skills/wren/). The stub hard-codes top-level CLI command references (wren profile switch, wren context build, wren memory recall, …) that can drift from the installed CLI after an upgrade. But the served-content guard (test_served_content_guard.py) scans only skills_content/, docs_content/, and ask_templates/not skills/wren/SKILL.md. So the one file that actually lands on users' machines has zero CI validation; its commands are all valid today only by luck.
Fix: add skills/wren/SKILL.md to the guard's scan roots (and ideally the command references in install.sh / marketplace.json).

🟡 Medium — docs drift --check gate isn't wired into CI

scripts/sync_docs_content.py advertises a --check CI gate, but no workflow invokes it. Worse, wren-ci.yml's path filter excludes docs/core/**, so editing a source doc triggers neither the sync check nor wren-ci, and the mirror can drift silently. (All 15 mirrors are byte-identical to source today.)
Fix: add a sync_docs_content.py --check step to wren-ci and/or add docs/core/** to the trigger paths.

🟡 Medium — PR description is stale on redirect stubs

The body says the five fat skill dirs are kept as redirect stubs; the final state deletes all of them, and test_skill_stubs.py::test_deprecated_dirs_removed enforces that they stay gone. Code/tests are self-consistent — please update the PR narrative (and the Migration table / test-plan bullet).

🟢 Low — artifacts glob doesn't declare the bundled .py script

pyproject.toml artifacts list *.md / docs_content/**/*.md / ask_templates/*.tmpl but not src/wren/skills_content/**/*.py. I built the wheel and confirmed skills_content/dlt-connector/scripts/introspect_dlt.py does ship — but only via hatchling's implicit .py inclusion, not the declared glob.
Fix: add "src/wren/skills_content/**/*.py" so intent is explicit and robust to tooling changes.

🟢 Low — guard's command-tree walk is version-fragile

test_served_content_guard.py's _walk uses cmd.list_commands(ctx=None). Under the lockfile pins (typer 0.24.1 / click 8.3.1) this works; I reproduced that under typer 0.26.3 / click 8.4.1 it returns [], collapsing COMMANDS to the root and silently neutering the guard. pyproject.toml pins only typer>=0.12 (no upper bound). test_command_tree_loaded would catch the regression loudly — but only if it runs (see High finding).
Fix: iterate cmd.commands directly, or set a typer upper bound.


Verified locally: wheel builds and bundles all 17 docs/skills/ask files + the script; delivery modules import and work from the installed wheel; all 15 docs mirrors match source; all 44 new tests pass under the lockfile-pinned typer/click; commits follow conventional format.

PaulChen79 and others added 3 commits May 29, 2026 13:39
… .py artifacts

goldmedal's PR #2329 review surfaced five real CI/guard gaps. All fixed
in one commit.

🔴 High — new tests were silently deselected in CI

CI runs `uv run pytest tests/unit/ -m unit`, and the `unit` marker is
opt-in via module-level `pytestmark = pytest.mark.unit`. None of the five
new test modules (`test_served_content_guard`, `test_skills_cli`,
`test_docs_cli_get`, `test_ask_cli`, `test_skill_stubs`) declared it, so
all 44 new tests were `deselected` by CI and only ran locally without
the filter. Adding `pytestmark = pytest.mark.unit` to each module brings
the served-content guard plus every new CLI test into CI: `-m unit` now
collects 325 tests instead of 281.

🟡 Medium — the discovery stub wasn't covered by the guard

The served-content guard scanned only `skills_content/`, `docs_content/`
and `ask_templates/` — but the file that actually ships to users via
`npx skills add` is `skills/wren/SKILL.md`, which references top-level
commands (`wren profile switch`, `wren context build`, etc.) that can
drift after a CLI upgrade. Added `skills/wren/SKILL.md` as a fourth
scan root via a new `_iter_content_files()` helper; the regression test
also monkeypatches the new `_DISCOVERY_STUB` constant for isolation.

🟡 Medium — `sync_docs_content.py --check` wasn't wired into CI

`scripts/sync_docs_content.py` was designed as a CI drift gate but no
workflow invoked it, and `wren-ci.yml`'s path filter excluded
`docs/core/**` — so editing a source doc neither ran the check nor even
triggered wren-ci. Added `docs/core/**` to the trigger paths and a new
`docs-sync-check` job that runs `--check`. Verified locally:
`docs_content in sync (15 references).` exit=0.

🟢 Low — bundled `.py` script wasn't in the artifacts glob

`pyproject.toml` declared `.md` / `.tmpl` artifacts but not
`*.py`. `skills_content/dlt-connector/scripts/introspect_dlt.py` shipped
only via hatchling's implicit `.py` inclusion. Added
`"src/wren/skills_content/**/*.py"` so the intent is explicit and robust
to tooling changes. Confirmed via fresh `uv build` that the script
still ends up in the wheel.

🟢 Low — guard's command-tree walk was version-fragile

`_walk` used `cmd.list_commands(ctx=None)` which evolves with click and
can return `[]` on newer versions, silently collapsing COMMANDS to the
root and neutering the guard. Switched to `cmd.commands.items()` (the
stable underlying dict).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The new `docs-sync-check` CI job runs `python core/wren/scripts/sync_docs_content.py --check`
in a bare Python env (no wren install). The script did
`from wren.docs_delivery import REFERENCE_SOURCES`, which hit
`wren/__init__.py` → `wren.engine` → `import pyarrow` and crashed with
`ModuleNotFoundError: No module named 'pyarrow'`.

Load `docs_delivery.py` standalone via `importlib.util.spec_from_file_location`
so the package `__init__.py` (and therefore pyarrow) never runs. Register
the loaded module in `sys.modules` before `exec_module` so its
`@dataclass(frozen=True)` decorator can resolve `cls.__module__` — required
on Python 3.14+ (the local repro), harmless on 3.11–3.13 (CI).

Verified: works both in a bare system python and in the editable uv venv.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@goldmedal
goldmedal marked this pull request as draft May 29, 2026 06:14
@PaulChen79
PaulChen79 marked this pull request as ready for review June 1, 2026 05:51
PaulChen79 and others added 2 commits June 4, 2026 08:57
Reviewer feedback: don't ship a parallel copy of docs/core inside the
wheel behind a new `wren docs get`/`list` CLI. Keep `wren docs
connection-info` exactly as before, and point skill reference links back
to the docs/core web pages — the pre-refactor skills approach.

Remove the doc-delivery additions:
- delete docs_delivery.py, the docs_content/refs mirror, and
  scripts/sync_docs_content.py
- strip docs_cli.py back to the `connection-info` command only
- drop docs_content from the wheel artifacts and remove the
  docs-sync-check CI job (+ its docs/core path trigger)
- delete test_docs_cli_get.py; fix the served-content guard and stub
  tests to assert `docs connection-info` instead of `docs get/list`

Repoint skill references to web docs:
- onboarding / generate-mdl / usage SKILL.md now link to
  github.com/Canner/WrenAI/blob/main/docs/core/... instead of
  `wren docs get <ref>`
- update the discovery stub, ask template, README, and
  docs/core/reference/{cli,skills}.md to match

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@goldmedal goldmedal left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @PaulChen79 look great 👍

@goldmedal
goldmedal merged commit cbd10cd into main Jun 4, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci core dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation python Pull requests that update Python code skills

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants