feat(wren): add type_mapping module and wren utils CLI + wren-generate-mdl skill#1513
Conversation
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ner#1509) Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds SQL type normalization utilities, CLI subcommands, documentation for a Wren MDL generation workflow, CLI registration, and unit/integration tests enabling single and batch type parsing across SQL dialects using sqlglot. Changes
Sequence DiagramsequenceDiagram
participant User
participant CLI as utils_cli<br/>(parse-type/parse-types)
participant TypeMap as type_mapping
participant sqlglot
participant Output
rect rgba(100, 200, 100, 0.5)
Note over User,Output: Single Type Normalization Flow
User->>CLI: wren utils parse-type "int8" --dialect postgres
CLI->>TypeMap: parse_type("int8", "postgres")
TypeMap->>sqlglot: parse & normalize type string
sqlglot-->>TypeMap: DataType.sql() result
TypeMap-->>CLI: Normalized type string
CLI->>Output: Print result
Output-->>User: "BIGINT"
end
rect rgba(150, 150, 200, 0.5)
Note over User,Output: Batch Type Normalization Flow
User->>CLI: wren utils parse-types --dialect postgres < data.json
CLI->>CLI: Read JSON array from stdin/file
CLI->>TypeMap: parse_types(columns, "postgres", type_field="raw_type")
loop For each column dict
TypeMap->>TypeMap: Clone dict & parse_type(raw_type)
TypeMap->>sqlglot: Normalize type
sqlglot-->>TypeMap: Normalized type
TypeMap->>TypeMap: Add "type" field to clone
end
TypeMap-->>CLI: List of enriched dicts
CLI->>Output: Print pretty JSON
Output-->>User: JSON with original + "type" fields
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
cli-skills/wren-generate-mdl/SKILL.md (2)
148-155: Add language identifier to fenced code block.This directory structure listing should have a language identifier. Using
textorplaintextis appropriate for non-code content like file trees.📝 Suggested fix
-``` +```text project/ ├── wren_project.yml ├── models/ ├── views/ ├── relationships.yml └── instructions.md ```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli-skills/wren-generate-mdl/SKILL.md` around lines 148 - 155, Update the fenced code block in SKILL.md that shows the project directory tree to include a language identifier (e.g., "text" or "plaintext") so the tree is rendered as plain text; locate the fenced block containing the directory listing (the triple-backtick block showing "project/ ├── wren_project.yml ...") and add the language token immediately after the opening backticks (e.g., ```text).
101-101: Minor grammar: hyphenate compound modifier.Per static analysis, "wren-core compatible" should be hyphenated when used as a compound adjective.
📝 Suggested fix
-**Goal:** Convert raw database types to wren-core compatible types. +**Goal:** Convert raw database types to wren-core-compatible types.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli-skills/wren-generate-mdl/SKILL.md` at line 101, Update the compound modifier in the Goal line by hyphenating "wren-core compatible": change the sentence "**Goal:** Convert raw database types to wren-core compatible types." so it reads "**Goal:** Convert raw database types to wren-core-compatible types." to correct the compound adjective usage (locate the Goal header / the sentence starting "Convert raw database types..." in SKILL.md).wren/tests/unit/test_type_mapping.py (1)
81-87: Consider capturing stderr for debugging test failures.The helper works correctly, but when tests fail, having stderr output would help diagnose issues.
💡 Optional: Add stderr assertion helper
def _run_wren(*args: str, stdin: str | None = None) -> subprocess.CompletedProcess: return subprocess.run( [sys.executable, "-m", "wren.cli", *args], input=stdin, capture_output=True, text=True, ) + + +def _assert_success(result: subprocess.CompletedProcess) -> None: + """Assert command succeeded, printing stderr on failure.""" + assert result.returncode == 0, f"Command failed with stderr: {result.stderr}"Then use
_assert_success(result)instead ofassert result.returncode == 0for better diagnostics.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wren/tests/unit/test_type_mapping.py` around lines 81 - 87, Modify the test helper _run_wren to capture stderr for easier debugging by passing stderr=subprocess.STDOUT to subprocess.run (so stderr is included in the output) and add a small assertion helper function named _assert_success(result) that asserts result.returncode == 0 but on failure raises/asserts with the combined output (result.stdout) so test failures show stderr; update tests to use _assert_success(result) instead of bare returncode checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@wren/src/wren/utils_cli.py`:
- Around line 48-51: Wrap the JSON loading logic that uses
Path(input_file).read_text + json.loads and the json.load(sys.stdin) branch in
try/except blocks to catch json.JSONDecodeError, FileNotFoundError and
PermissionError (and a generic Exception fallback); when caught, print a clear,
user-friendly message to stderr (including the filename when available) and exit
with a non-zero status instead of letting the raw exception propagate. Ensure
you reference the existing symbols input_file, Path(input_file).read_text,
json.loads and json.load(sys.stdin) so the changes only modify error handling
around those calls.
---
Nitpick comments:
In `@cli-skills/wren-generate-mdl/SKILL.md`:
- Around line 148-155: Update the fenced code block in SKILL.md that shows the
project directory tree to include a language identifier (e.g., "text" or
"plaintext") so the tree is rendered as plain text; locate the fenced block
containing the directory listing (the triple-backtick block showing "project/
├── wren_project.yml ...") and add the language token immediately after the
opening backticks (e.g., ```text).
- Line 101: Update the compound modifier in the Goal line by hyphenating
"wren-core compatible": change the sentence "**Goal:** Convert raw database
types to wren-core compatible types." so it reads "**Goal:** Convert raw
database types to wren-core-compatible types." to correct the compound adjective
usage (locate the Goal header / the sentence starting "Convert raw database
types..." in SKILL.md).
In `@wren/tests/unit/test_type_mapping.py`:
- Around line 81-87: Modify the test helper _run_wren to capture stderr for
easier debugging by passing stderr=subprocess.STDOUT to subprocess.run (so
stderr is included in the output) and add a small assertion helper function
named _assert_success(result) that asserts result.returncode == 0 but on failure
raises/asserts with the combined output (result.stdout) so test failures show
stderr; update tests to use _assert_success(result) instead of bare returncode
checks.
🪄 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: 45669067-b880-4c38-9fd6-1891ca27426d
📒 Files selected for processing (5)
cli-skills/wren-generate-mdl/SKILL.mdwren/src/wren/cli.pywren/src/wren/type_mapping.pywren/src/wren/utils_cli.pywren/tests/unit/test_type_mapping.py
Add sqlglot-based SQL type normalization available both as a Python import and via the `wren utils parse-type` / `wren utils parse-types` CLI commands. Also adds the `wren-generate-mdl` agent skill that guides through schema discovery, type normalization, and MDL project scaffolding. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add error handling in parse-types CLI for invalid JSON and missing files - Add _assert_success() helper to CLI tests for clearer failure diagnostics - Use _assert_success() consistently across all CLI integration tests - Fix SKILL.md: hyphenate "wren-core-compatible", add text lang to code fence Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
d46c631 to
665fb8d
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
wren/src/wren/utils_cli.py (1)
48-59: Error handling implemented as requested, with one gap.The JSON error handling addresses the previous review feedback. However,
path.read_text()can also raisePermissionErrorif the file exists but isn't readable—this would surface as an unhandled exception.🛡️ Optional: handle permission errors
try: if input_file: path = Path(input_file) if not path.exists(): typer.echo(f"Error: file not found: {input_file}", err=True) raise typer.Exit(1) - data = json.loads(path.read_text()) + try: + data = json.loads(path.read_text()) + except PermissionError: + typer.echo(f"Error: permission denied: {input_file}", err=True) + raise typer.Exit(1) else: data = json.load(sys.stdin) except json.JSONDecodeError as e:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wren/src/wren/utils_cli.py` around lines 48 - 59, The try/except around JSON loading should also handle file read permission errors: when input_file is provided and you call Path(...).read_text() inside the block (see variables input_file and path and call to path.read_text()), catch PermissionError (or broader OSError if you prefer) alongside json.JSONDecodeError, echo a clear error via typer.echo (e.g., "Error: cannot read file: <path>: <error>") and then raise typer.Exit(1) so permission issues don’t crash the CLI.wren/tests/unit/test_type_mapping.py (1)
122-144: Good CLI integration coverage.The tests validate both single and batch CLI commands, including stdin JSON input and field preservation in output.
Consider adding tests for error cases in a follow-up (e.g., invalid JSON input, missing
--typeor--dialectarguments) to ensure the CLI provides user-friendly error messages.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wren/tests/unit/test_type_mapping.py` around lines 122 - 144, Add negative CLI tests to wren/tests/unit/test_type_mapping.py to cover error cases: create tests like test_cli_parse_types_invalid_json that calls _run_wren("utils","parse-types","--dialect","postgres", stdin="not json") and asserts non-zero exit and a helpful error message in stderr; test_cli_parse_types_missing_dialect that calls _run_wren("utils","parse-types", stdin=json.dumps([...])) and asserts the CLI fails with a clear "missing --dialect" style message; and optionally test_cli_parse_types_missing_type or other required-arg scenarios similarly, using _assert_success negation and checking stderr text to ensure user-friendly errors from the parse-types command.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@wren/src/wren/utils_cli.py`:
- Around line 48-59: The try/except around JSON loading should also handle file
read permission errors: when input_file is provided and you call
Path(...).read_text() inside the block (see variables input_file and path and
call to path.read_text()), catch PermissionError (or broader OSError if you
prefer) alongside json.JSONDecodeError, echo a clear error via typer.echo (e.g.,
"Error: cannot read file: <path>: <error>") and then raise typer.Exit(1) so
permission issues don’t crash the CLI.
In `@wren/tests/unit/test_type_mapping.py`:
- Around line 122-144: Add negative CLI tests to
wren/tests/unit/test_type_mapping.py to cover error cases: create tests like
test_cli_parse_types_invalid_json that calls
_run_wren("utils","parse-types","--dialect","postgres", stdin="not json") and
asserts non-zero exit and a helpful error message in stderr;
test_cli_parse_types_missing_dialect that calls _run_wren("utils","parse-types",
stdin=json.dumps([...])) and asserts the CLI fails with a clear "missing
--dialect" style message; and optionally test_cli_parse_types_missing_type or
other required-arg scenarios similarly, using _assert_success negation and
checking stderr text to ensure user-friendly errors from the parse-types
command.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 01b5f4c8-1c7b-4886-a4b9-294b99d5a89c
📒 Files selected for processing (5)
cli-skills/wren-generate-mdl/SKILL.mdwren/src/wren/cli.pywren/src/wren/type_mapping.pywren/src/wren/utils_cli.pywren/tests/unit/test_type_mapping.py
✅ Files skipped from review due to trivial changes (2)
- wren/src/wren/type_mapping.py
- cli-skills/wren-generate-mdl/SKILL.md
…e-mdl skill (#1513) Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
wren.type_mappingmodule withparse_type()andparse_types()for sqlglot-based SQL type normalization (no custom mapping table — sqlglot is source of truth, graceful fallback on unknown types)wren utils parse-typeandwren utils parse-typesCLI subcommands for single and batch type normalization from the command linecli-skills/wren-generate-mdl/SKILL.md— a 7-phase agent workflow skill for bootstrapping an MDL project from an existing database (schema discovery → type normalization → YAML scaffolding → validation → memory indexing)Test plan
uv run pytest tests/unit/test_type_mapping.py -v— 18 tests passing (9parse_typeparametrized cases, 4parse_typesbatch cases, 5 CLI integration tests)wren utils parse-type --type "int8" --dialect postgres→BIGINTwren utils parse-types --dialect postgreswith JSON on stdin → JSON with"type"fields addeduv run ruff format --check src/ && uv run ruff check src/— clean🤖 Generated with Claude Code
Summary by CodeRabbit