fix(connector): coerce non-string SQL in strip_trailing_semicolon

[Bartok9]

Summary

• Coerce non-string sql arguments in strip_trailing_semicolon (None → "", other values via str(...)).
• Prevents TypeError inside the trailing-semicolon regex when callers pass accidental non-strings.

Motivation

Every connector funnels user/limit/EXOLAIN composition through this helper. A None or int SQL argument currently raises TypeError: expected string or bytes-like object deep in re.sub, masking the real caller bug with a generic regex crash.

License

Apache-2.0 path: core/wren/src/wren/connector/base.py (+ unit test).

Verification

cd core/wren && .venv/bin/python -m pytest tests/unit/test_strip_trailing_semicolon_coerce.py -q → 3 passed

Test plan

• unit tests for string / None / int
• CI green

Summary by CodeRabbit

• Bug Fixes

  • Improved SQL statement handling when input is empty, null, or not provided as text.
  • Non-text values are now safely converted to text before trailing semicolons and whitespace are removed.

• Tests

  • Added coverage for standard strings, null values, and non-text inputs.

[coderabbitai]

Walkthrough

The shared strip_trailing_semicolon helper now handles None and non-string inputs before trimming trailing semicolons. New unit tests cover string, null, and numeric inputs.

Changes

Semicolon Input Normalization

Layer / File(s)	Summary
Normalize helper inputs and verify behavior core/wren/src/wren/connector/base.py, core/wren/tests/unit/test_strip_trailing_semicolon_coerce.py	The helper returns an empty string for None, coerces other non-string values, preserves trailing-semicolon stripping, and adds focused tests for these cases.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• Canner/WrenAI#2517: Updates the same helper and tests equivalent input-coercion behavior.
• Canner/WrenAI#2480: Introduces the shared helper later hardened by this change.
• Canner/WrenAI#2477: Applies the helper in DatabricksConnector.query() with regression coverage.

Suggested reviewers: goldmedal

Poem

I’m a rabbit who trims each tail,
Semicolons vanish without fail.
None hops to an empty line,
Numbers turn to strings just fine.
Tests bloom softly in the trail.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: coercing non-string SQL inputs in strip_trailing_semicolon.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

core/wren/src/wren/connector/base.py (1)

19-27: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Update the type hint to match the new behavior.

The function is now intentionally designed to accept None and non-string values. However, the function signature on line 11 still types sql as str (def strip_trailing_semicolon(sql: str) -> str:). This will cause static type checkers to report errors when non-string arguments are passed to this function.

Consider updating the type hint to Any (e.g., sql: Any) to accurately reflect this defensive behavior.

# In core/wren/src/wren/connector/base.py, around line 11:
from typing import Any

def strip_trailing_semicolon(sql: Any) -> str:

🤖 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/connector/base.py` around lines 19 - 27, Update the
`strip_trailing_semicolon` parameter annotation to accept the `None` and
non-string values already handled by its implementation, importing `Any` from
`typing` if needed, while keeping the return type as `str`.

🤖 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.

Nitpick comments:
In `@core/wren/src/wren/connector/base.py`:
- Around line 19-27: Update the `strip_trailing_semicolon` parameter annotation
to accept the `None` and non-string values already handled by its
implementation, importing `Any` from `typing` if needed, while keeping the
return type as `str`.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 0cafb939-d329-405d-9f5f-0343e307f5a4

📥 Commits

Reviewing files that changed from the base of the PR and between 3dac00a and c96c2d6.

📒 Files selected for processing (2)

• core/wren/src/wren/connector/base.py
• core/wren/tests/unit/test_strip_trailing_semicolon_coerce.py