added ambiguity node after schema explorer#26
Open
yuvalkh wants to merge 4 commits into
Open
Conversation
📝 WalkthroughWalkthroughThe agent adds a dedicated ambiguity-detection and HITL clarification path before SQL generation, with retry limits and unanswerable outcomes. API and frontend contracts expose this status, while schema exploration and diagnostic failure messages are updated accordingly. ChangesAmbiguity workflow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant MCPServer
participant SchemaExplorer
participant DetectAmbiguity
participant AmbiguityResolution
participant QueryBuilder
User->>MCPServer: Submit request
MCPServer->>SchemaExplorer: Execute schema exploration
SchemaExplorer->>DetectAmbiguity: schema_plan and schema context
DetectAmbiguity->>DetectAmbiguity: Classify ambiguity
DetectAmbiguity->>QueryBuilder: Clear result
DetectAmbiguity->>AmbiguityResolution: Ambiguous result and questions
User->>MCPServer: Provide clarification
MCPServer->>AmbiguityResolution: Resume with feedback
AmbiguityResolution->>SchemaExplorer: Retry with updated user_query
DetectAmbiguity-->>MCPServer: Unanswerable result
MCPServer-->>User: is_unanswerable response
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@agent/src/agent/nodes/detect_ambiguity.py`:
- Around line 155-165: Update the parse-failure fallback in detect_ambiguity to
use a generic user-facing clarification in the reason field, preventing raw
exception details from reaching downstream clarifying-question output. Keep the
captured exception available only through logging or internal ambiguity_result
data, while preserving the existing ambiguous classification.
In `@agent/src/agent/nodes/schema_explorer.py`:
- Around line 525-529: Update the exception fallback in the schema explorer
parsing flow to construct SchemaExplorerOutput with an empty string (or the
model’s valid default) for schema_plan instead of None. Preserve the existing
plan = data.schema_plan or "" recovery behavior so parsing failures return
normally without triggering another validation error.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 26a079bf-e99e-4260-b056-0974e018258d
📒 Files selected for processing (14)
agent/scripts/upload_all_prompts.pyagent/src/agent/config.pyagent/src/agent/graph.pyagent/src/agent/mcp_server.pyagent/src/agent/nodes/detect_ambiguity.pyagent/src/agent/nodes/refiner.pyagent/src/agent/nodes/satisfaction_check.pyagent/src/agent/nodes/schema_explorer.pyagent/src/agent/state.pyagent/src/agent/utils/flag_bridge.pyagent/src/agent/utils/schema_enrichment.pybackend/app/routers/agent.pyfrontend/src/api/agent.tsfrontend/src/pages/AgentTestingPage.tsx
💤 Files with no reviewable changes (1)
- agent/src/agent/utils/flag_bridge.py
elirazpevz
reviewed
Jul 12, 2026
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
agent/src/agent/nodes/detect_ambiguity.py (1)
263-274: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winPass the fallback clarification via
feedbackso it reachesschema_explorer.When the user provides no feedback, the code constructs
user_feedbackwith the previous question but then explicitly omits it fromnew_queryand sets"feedback": None. This discards the fallback context entirely, leavingschema_explorerwith the exact same inputs as the previous run. This will result in a loop of identical outputs until max retries are hit.Pass the fallback context via the
feedbackstate field soschema_explorercan use it. The downstreamsql_static_validations_nodewill safely clear it before it reaches therejection_router.🐛 Proposed fix
new_query = state.get("user_query", "") if user_feedback and not user_feedback.startswith("[Previous"): new_query += f"\n[User Clarification: {user_feedback}]" return { # Update the main query so all subsequent nodes see the unified intent. "user_query": new_query, # Clear ambiguity decision so detect_ambiguity re-evaluates cleanly on retry. "ambiguity_type": None, "ambiguity_result": None, "clarifying_questions": None, - # Clear feedback so it doesn't accidentally trigger the rejection_router later. - "feedback": None, + # Pass the previous question as feedback if no user clarification was provided; + # sql_static_validations_node will safely clear it later. + "feedback": user_feedback if user_feedback.startswith("[Previous") else None, "ambiguity_retry_count": retry_count, "execution_path": ["ambiguity_resolution"], }🤖 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 `@agent/src/agent/nodes/detect_ambiguity.py` around lines 263 - 274, Update the retry return state in the ambiguity-handling flow to preserve the constructed fallback user_feedback through the "feedback" field instead of setting it to None. Keep the existing new_query behavior for explicit feedback, and ensure schema_explorer receives the fallback clarification while downstream validation clears it before rejection routing.
🤖 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 `@agent/src/agent/nodes/detect_ambiguity.py`:
- Around line 157-166: Update the exception fallback result in the ambiguity
detection flow to include only the current AmbiguityResult fields:
ambiguity_type, reason, and clarifying_questions. Remove intent_deconstruction,
active_schema_search, ambiguity_check, logical_sql_plan, and
schema_alignment_check from the parsed fallback.
---
Outside diff comments:
In `@agent/src/agent/nodes/detect_ambiguity.py`:
- Around line 263-274: Update the retry return state in the ambiguity-handling
flow to preserve the constructed fallback user_feedback through the "feedback"
field instead of setting it to None. Keep the existing new_query behavior for
explicit feedback, and ensure schema_explorer receives the fallback
clarification while downstream validation clears it before rejection routing.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 43eabab8-5f21-4f33-8c74-0657b5e4be95
📒 Files selected for processing (2)
agent/src/agent/nodes/detect_ambiguity.pyagent/src/agent/nodes/schema_explorer.py
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
agent/src/agent/config.py (1)
46-46: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winKeep ambiguity detection disabled until its Langfuse prompt is guaranteed to exist.
ENABLE_AMBIGUITY_DETECTdefaults toTrue, so upgrades withouttext2sql/detect_ambiguitywill route intodetect_ambiguityand raise at request time. Either leave the default off until rollout is complete or provision/validate the prompt at startup.🤖 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 `@agent/src/agent/config.py` at line 46, Update the ambiguity-detection configuration around ENABLE_AMBIGUITY_DETECT so it defaults to disabled until the Langfuse prompt identified by LANGFUSE_PROMPT_DETECT_AMBIGUITY is guaranteed to exist; alternatively, add startup validation/provisioning that verifies this prompt before enabling the feature.
🤖 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 `@agent/src/agent/config.py`:
- Line 73: Update the MAX_AMBIGUITY_RETRIES configuration field to use
validation with a default of 2 and a minimum value of 0, rejecting negative
environment-configured values while preserving zero as valid. Leave the existing
ambiguity retry handling unchanged.
---
Outside diff comments:
In `@agent/src/agent/config.py`:
- Line 46: Update the ambiguity-detection configuration around
ENABLE_AMBIGUITY_DETECT so it defaults to disabled until the Langfuse prompt
identified by LANGFUSE_PROMPT_DETECT_AMBIGUITY is guaranteed to exist;
alternatively, add startup validation/provisioning that verifies this prompt
before enabling the feature.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9b709a29-07df-45ab-923c-ef166ba17b9c
📒 Files selected for processing (3)
agent/src/agent/config.pyagent/src/agent/graph.pyagent/src/agent/nodes/detect_ambiguity.py
💤 Files with no reviewable changes (1)
- agent/src/agent/nodes/detect_ambiguity.py
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
2 participants
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.
Summary by CodeRabbit
New Features
Bug Fixes