feat(datafabric): SQL prompt changes from the recent bugbash - #1096
Open
milind-jain-uipath wants to merge 3 commits into
Open
milind-jain-uipath wants to merge 3 commits into
milind-jain-uipath wants to merge 3 commits into
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…match The entity-query subgraph is non-interactive and the ENTITY SELECTION step told the model to always pick an entity, so with two similar entities in context it silently assumed one and ran SQL. Add a rule to the v1 planning prompt: when 2+ entities are equally plausible and the question doesn't disambiguate, return a clarifying question naming the candidates instead of calling execute_sql. The subgraph's router already terminates cleanly on a no-tool reply, so the clarifying question bubbles up to the outer agent. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Regression per PR review: assert the rendered v1 prompt contains the
disambiguation rule + its no-tool ("instead of calling execute_sql") directive,
so a future template/rendering edit can't silently drop or join it. Covers both
entity-set and ontology paths (both render through the default v1 strategy).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
milind-jain-uipath
force-pushed
the
feat/datafabric-bugbash-prompt-changes
branch
from
September 16, 2026 19:34
80c9b03 to
da6f592
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved prompt-generation contradictions and an unreliable COUNT rule block safe approval.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Updates Data Fabric SQL prompts and constraints from the recent bugbash, with regression coverage and a version bump to 0.18.4.
Changes:
- Adds entity disambiguation, counting, limits, and recovery guidance.
- Tightens projection and aggregation constraints.
- Updates tests, package metadata, and the lockfile.
File summaries
| File | Summary and review findings |
|---|---|
uv.lock |
Synchronizes the locked package version. |
tests/agent/tools/test_datafabric_prompt_builder.py |
Adds prompt regression coverage. |
src/uipath_langchain/agent/tools/datafabric_tool/prompts/v1.py |
Updates SQL guidance. Critical (2 votes): the COUNT(Id) rule conflicts with generated query patterns. Moderate (3 votes): the primary-key fallback is not observable in the entity prompt. |
src/uipath_langchain/agent/tools/datafabric_tool/datafabric_prompts.py |
Updates SQL examples and constraints. Moderate (3 votes): the column-count rule conflicts with the generated Show all pattern. Moderate (3 votes): aggregate restrictions conflict with existing aggregation guidance and templates. |
pyproject.toml |
Bumps the package version to 0.18.4. |
Review details
- Files reviewed: 4/5 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| If no aggregation word appears, do NOT aggregate. | ||
| {aggregation_hints} | ||
| If query needs COUNT(*) - remember that COUNT(*) is not supported. Handle COUNT(*) as follows: | ||
| - If the entity schema contains the literal Id field (an exact, case-sensitive field-name match) - COUNT(*) can be replaced with COUNT(Id) since the Id field is UNIQUEIDENTIFIER. |
| 11. **Limit unbounded row queries** - Queries without WHERE that could return many rows must include a LIMIT clause (e.g., LIMIT 100). Scalar aggregate queries do not require LIMIT | ||
| 12. **Choice-set fields use display labels** - Choice-set fields list their allowed values in the Description column of the schema table. Use the display label as a string in WHERE clauses (e.g. ``WHERE Priority = 'Critical'``). Multi-select (array) choice-set fields cannot be filtered via SQL — only SELECT them.""" | ||
| 12. **Choice-set fields use display labels** - Choice-set fields list their allowed values in the Description column of the schema table. Use the display label as a string in WHERE clauses (e.g. ``WHERE Priority = 'Critical'``). Multi-select (array) choice-set fields cannot be filtered via SQL — only SELECT them. | ||
| 13. **More than 4 columns needs a filter** - Selecting more than 4 columns is only allowed when the query has a WHERE clause |
| 12. **Choice-set fields use display labels** - Choice-set fields list their allowed values in the Description column of the schema table. Use the display label as a string in WHERE clauses (e.g. ``WHERE Priority = 'Critical'``). Multi-select (array) choice-set fields cannot be filtered via SQL — only SELECT them.""" | ||
| 12. **Choice-set fields use display labels** - Choice-set fields list their allowed values in the Description column of the schema table. Use the display label as a string in WHERE clauses (e.g. ``WHERE Priority = 'Critical'``). Multi-select (array) choice-set fields cannot be filtered via SQL — only SELECT them. | ||
| 13. **More than 4 columns needs a filter** - Selecting more than 4 columns is only allowed when the query has a WHERE clause | ||
| 14. **Aggregate arguments are a single column** - COUNT/SUM/AVG/MIN/MAX take one plain column, never a CASE or a calculation; keep to 5 aggregate functions and 5 GROUP BY columns per query""" |
| {aggregation_hints} | ||
| If query needs COUNT(*) - remember that COUNT(*) is not supported. Handle COUNT(*) as follows: | ||
| - If the entity schema contains the literal Id field (an exact, case-sensitive field-name match) - COUNT(*) can be replaced with COUNT(Id) since the Id field is UNIQUEIDENTIFIER. | ||
| - If there is no Id field, but the schema marks a field as primary key, then that field can be used instead of COUNT(*). |
|
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
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.
What
Prompt changes to the v1 SQL prompt and SQL_CONSTRAINTS, based on findings from the recent bugbash.
Tests
pytest tests/agent/tools/test_datafabric_prompt_builder.py test_datafabric_tool.py test_datafabric_query_tool.py test_datafabric_choiceset.py — 45 passed.
🤖 Generated with Claude Code