feat(pydantic): forward session properties to the engine - #2684
feat(pydantic): forward session properties to the engine#2684lucifer726 wants to merge 1 commit into
Conversation
WrenEngine.query/dry_plan/dry_run already accept `properties`, but the toolkit's direct Python API never passed them, so a model guarded by row-level access control could not be read from the SDK: planning fails when a required session property is missing and there was no way to supply one per call. The Pydantic AI tools keep their existing signatures on purpose. Their `ctx` is documented as ignored because the toolkit already captures all required state, and exposing a session property as a model-fillable tool argument would let the LLM choose the identity its own access control is keyed on. Closes Canner#2638 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. Walkthrough
ChangesSession property support
Estimated code review effort: 2 (Simple) | ~15 minutes Merge Risk: ⚪ Minimal · up to The PR forwards optional session properties through the toolkit APIs without introducing an actionable merge-blocking risk; it is merge-ready after normal checks and review. Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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 |
What changes
WrenToolkit.query(),dry_plan()anddry_run()now acceptpropertiesand forwardit to the engine, so a caller can supply MDL session properties per call.
WrenEngine.query/dry_plan/dry_runalready acceptproperties(core/wren/src/wren/engine.py)— the toolkit simply never passed it, so a model guarded by row-level access control could
not be read through the SDK at all: planning fails when a required session property is
missing, and there was no way to supply one.
Closes #2638.
Why the Pydantic AI tools are unchanged
wren_query/wren_dry_plankeep their current signatures on purpose. Theirctxisdocumented as ignored because "the toolkit already captures all required state"
(
_tools.py), and turning a session property into a model-fillable tool argument would letthe LLM choose the identity that RLAC is keyed on — the model would be deciding which rows
it is allowed to see. Binding session state for agent runs is a separate design question
(toolkit- or run-scoped), so this PR only opens the direct Python API that the issue asks
for.
How this was verified
Scope of the claim: these tests assert the parameter reaches
WrenEngine. I did not standup a live RLAC project against a real data source, so this PR does not claim an end-to-end
RLAC observation — the engine-side behaviour is the existing, already-tested path.
propertiesis forwarded byquery/dry_plan/dry_run.(
properties=Noneby default), since they pinned the exact kwargs._toolkit.pyreverted, 8 passed with it).
pytest tests/unit→ 103 passed.ruff check .andruff format --check .clean.Summary by CodeRabbit
New Features
Bug Fixes