Add planning_questionnaire to the agent mode#2566
Conversation
|
@BugBot run |
Summary of ChangesHello @azizmejri1, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the agent's ability to interact with users by integrating a crucial clarification step into its core workflow. By making the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request successfully extends the planning_questionnaire tool to be available in the standard agent mode, not just in plan mode. This allows the agent to proactively ask clarifying questions before starting implementation, which should improve its performance on ambiguous tasks. The changes are well-executed across the board: the agent's stopping condition is updated, tool definitions are refactored for clarity, and the system prompt is modified to instruct the agent on this new workflow step. The implementation is clean and logical. I have no specific comments as the changes are solid.
🔍 Multi-Agent Code ReviewFound 5 issue(s) flagged by consensus of 3 independent reviewers. Summary
Issues to Address
🟢 Low Priority Issues (2 items)
See inline comments for details on MEDIUM issues. Generated by multi-agent consensus review (3 agents, 2+ agreement threshold) |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@BugBot run |
azizmejri1
force-pushed
the
questionnaire-agent-mode
branch
from
4104006 to
3a277ec
Compare
|
@BugBot run |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
🔍 Dyadbot Code Review SummaryFound 1 new issue(s) flagged by 3 independent reviewers. Summary
Issues to Address
🟢 Low Priority Issues (5 items — not consensus, single-agent only)
See inline comments for details. Generated by Dyadbot multi-agent code review (3 independent reviewers with consensus voting) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
github-actions
Bot
added
the
needs-human:review-issue
|
@BugBot run |
|
All contributors have signed the CLA ✍️ ✅ |
🔍 Dyadbot Code Review SummaryFound 7 consensus issue(s) flagged by 2+ of 3 independent reviewers. Summary
Issues to Address
🟢 Low Priority Issues (5 items)
See inline comments for details on MEDIUM issues. Generated by Dyadbot multi-agent code review (3 agents, consensus voting) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| <p className="text-sm text-foreground/90 leading-relaxed"> | ||
| {current.answer} | ||
| </p> |
There was a problem hiding this comment.
🟡 MEDIUM | edge-cases-in-ui
Long answers overflow without wrapping or truncation
The answer text is rendered in a simple <p> tag with no overflow handling. If a user provides a very long free-text answer (e.g., multiple paragraphs), or if checkbox answers are concatenated into a long string, the card will expand vertically without limit, pushing other content down and making the questionnaire response card hard to scan.
💡 Suggestion: Add max-h-32 overflow-y-auto or a "show more" toggle for long answers:
| <p className="text-sm text-foreground/90 leading-relaxed"> | |
| {current.answer} | |
| </p> | |
| <p className="text-sm text-foreground/90 leading-relaxed max-h-32 overflow-y-auto"> | |
| {current.answer} | |
| </p> |
🎭 Playwright Test Results❌ Some tests failed
Summary: 717 passed, 1 failed, 11 flaky, 230 skipped Failed Tests🍎 macOS
📋 Re-run Failing Tests (macOS)Copy and paste to re-run all failing spec files locally: npm run e2e \
e2e-tests/setup_flow.spec.ts
|
wwwillchen
left a comment
wwwillchen
left a comment
There was a problem hiding this comment.
thanks! looks good, just a couple more changes needed:
- Let's have planning questionnaire work for all users (not just pro users) in agent modes.
- The dismiss questionnaire looks a little out of place, especially if the chat width is small (see screenshot below). let's instead move it to the top-right corner.
|
@BugBot run |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| stopWhen: [ | ||
| stepCountIs(25), | ||
| hasToolCall(addIntegrationTool.name), | ||
| // In plan mode, stop immediately after presenting a questionnaire, | ||
| // writing a plan, or exiting plan mode so the agent yields control | ||
| // back to the user. Without this, some models (e.g. Gemini Pro 3) | ||
| // ignore the prompt-level "STOP" instruction and keep calling tools | ||
| // in a loop. | ||
| // In plan mode, also stop after writing a plan or exiting plan mode. | ||
| ...(planModeOnly | ||
| ? [ | ||
| hasToolCall(planningQuestionnaireTool.name), | ||
| hasToolCall(writePlanTool.name), | ||
| hasToolCall(exitPlanTool.name), | ||
| ] | ||
| ? [hasToolCall(writePlanTool.name), hasToolCall(exitPlanTool.name)] | ||
| : []), | ||
| ], |
There was a problem hiding this comment.
Critical: Agent continues after planning_questionnaire completes
planning_questionnaire was removed from stopWhen in both modes. While the tool blocks during user input (via waitForQuestionnaireResponse), once the user responds, the tool returns and the agent can continue making tool calls in the same turn.
This contradicts the PR description: "Always stop after planning_questionnaire in both modes to prevent repeated tool calls."
Expected behavior: Agent should yield control to user after questionnaire
Actual behavior: Agent continues executing after receiving questionnaire responses
The old code correctly included hasToolCall(planningQuestionnaireTool.name) in stopWhen for plan mode. It needs to be added for BOTH modes:
| stopWhen: [ | |
| stepCountIs(25), | |
| hasToolCall(addIntegrationTool.name), | |
| // In plan mode, stop immediately after presenting a questionnaire, | |
| // writing a plan, or exiting plan mode so the agent yields control | |
| // back to the user. Without this, some models (e.g. Gemini Pro 3) | |
| // ignore the prompt-level "STOP" instruction and keep calling tools | |
| // in a loop. | |
| // In plan mode, also stop after writing a plan or exiting plan mode. | |
| ...(planModeOnly | |
| ? [ | |
| hasToolCall(planningQuestionnaireTool.name), | |
| hasToolCall(writePlanTool.name), | |
| hasToolCall(exitPlanTool.name), | |
| ] | |
| ? [hasToolCall(writePlanTool.name), hasToolCall(exitPlanTool.name)] | |
| : []), | |
| ], | |
| stopWhen: [ | |
| stepCountIs(25), | |
| hasToolCall(addIntegrationTool.name), | |
| hasToolCall("planning_questionnaire"), | |
| // In plan mode, also stop after writing a plan or exiting plan mode. | |
| ...(planModeOnly | |
| ? [hasToolCall(writePlanTool.name), hasToolCall(exitPlanTool.name)] | |
| : []), | |
| ], |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/pro/main/ipc/handlers/local_agent/local_agent_handler.ts
Line: 557-564
Comment:
**Critical: Agent continues after `planning_questionnaire` completes**
`planning_questionnaire` was removed from `stopWhen` in both modes. While the tool blocks during user input (via `waitForQuestionnaireResponse`), once the user responds, the tool returns and the agent can continue making tool calls in the same turn.
This contradicts the PR description: "Always stop after `planning_questionnaire` in both modes to prevent repeated tool calls."
**Expected behavior**: Agent should yield control to user after questionnaire
**Actual behavior**: Agent continues executing after receiving questionnaire responses
The old code correctly included `hasToolCall(planningQuestionnaireTool.name)` in `stopWhen` for plan mode. It needs to be added for BOTH modes:
```suggestion
stopWhen: [
stepCountIs(25),
hasToolCall(addIntegrationTool.name),
hasToolCall("planning_questionnaire"),
// In plan mode, also stop after writing a plan or exiting plan mode.
...(planModeOnly
? [hasToolCall(writePlanTool.name), hasToolCall(exitPlanTool.name)]
: []),
],
```
How can I resolve this? If you propose a fix, please make it concise.
- Remove debug onXmlComplete output from planning_questionnaire (HIGH) - Fix tool-error field: read part.error instead of part.output - Add Zod .refine() to enforce options for radio/checkbox questions - Derive PLAN_MODE_ONLY_TOOLS from PLANNING_SPECIFIC_TOOLS to prevent drift - Filter planning_questionnaire from basic agent mode toolset - Add aria-label and aria-expanded to QuestionnaireInput buttons - Update stale snapshot for local_agent_prompt test Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@BugBot run |
| stopWhen: [ | ||
| stepCountIs(25), | ||
| hasToolCall(addIntegrationTool.name), | ||
| // In plan mode, stop immediately after presenting a questionnaire, | ||
| // writing a plan, or exiting plan mode so the agent yields control | ||
| // back to the user. Without this, some models (e.g. Gemini Pro 3) | ||
| // ignore the prompt-level "STOP" instruction and keep calling tools | ||
| // in a loop. | ||
| // In plan mode, also stop after writing a plan or exiting plan mode. | ||
| ...(planModeOnly | ||
| ? [ | ||
| hasToolCall(planningQuestionnaireTool.name), | ||
| hasToolCall(writePlanTool.name), | ||
| hasToolCall(exitPlanTool.name), | ||
| ] | ||
| ? [hasToolCall(writePlanTool.name), hasToolCall(exitPlanTool.name)] | ||
| : []), | ||
| ], |
There was a problem hiding this comment.
Critical: Agent continues after planning_questionnaire completes
planning_questionnaire was removed from stopWhen for both modes. The tool calls waitForQuestionnaireResponse (line 149 in planning_questionnaire.ts) which blocks until user input, but once the user responds and the tool returns, the agent can continue making additional tool calls in the same turn.
This contradicts the PR description: "Always stop after planning_questionnaire in both modes to prevent repeated tool calls."
Expected: Agent yields control to user after questionnaire
Actual: Agent continues executing after receiving responses
Add hasToolCall(PLANNING_QUESTIONNAIRE_TOOL_NAME) to the stopWhen array for BOTH modes:
| stopWhen: [ | |
| stepCountIs(25), | |
| hasToolCall(addIntegrationTool.name), | |
| // In plan mode, stop immediately after presenting a questionnaire, | |
| // writing a plan, or exiting plan mode so the agent yields control | |
| // back to the user. Without this, some models (e.g. Gemini Pro 3) | |
| // ignore the prompt-level "STOP" instruction and keep calling tools | |
| // in a loop. | |
| // In plan mode, also stop after writing a plan or exiting plan mode. | |
| ...(planModeOnly | |
| ? [ | |
| hasToolCall(planningQuestionnaireTool.name), | |
| hasToolCall(writePlanTool.name), | |
| hasToolCall(exitPlanTool.name), | |
| ] | |
| ? [hasToolCall(writePlanTool.name), hasToolCall(exitPlanTool.name)] | |
| : []), | |
| ], | |
| stopWhen: [ | |
| stepCountIs(25), | |
| hasToolCall(addIntegrationTool.name), | |
| hasToolCall(PLANNING_QUESTIONNAIRE_TOOL_NAME), | |
| // In plan mode, also stop after writing a plan or exiting plan mode. | |
| ...(planModeOnly | |
| ? [hasToolCall(writePlanTool.name), hasToolCall(exitPlanTool.name)] | |
| : []), | |
| ], |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/pro/main/ipc/handlers/local_agent/local_agent_handler.ts
Line: 557-564
Comment:
**Critical: Agent continues after `planning_questionnaire` completes**
`planning_questionnaire` was removed from `stopWhen` for both modes. The tool calls `waitForQuestionnaireResponse` (line 149 in planning_questionnaire.ts) which blocks until user input, but once the user responds and the tool returns, the agent can continue making additional tool calls in the same turn.
This contradicts the PR description: "Always stop after `planning_questionnaire` in both modes to prevent repeated tool calls."
**Expected**: Agent yields control to user after questionnaire
**Actual**: Agent continues executing after receiving responses
Add `hasToolCall(PLANNING_QUESTIONNAIRE_TOOL_NAME)` to the `stopWhen` array for BOTH modes:
```suggestion
stopWhen: [
stepCountIs(25),
hasToolCall(addIntegrationTool.name),
hasToolCall(PLANNING_QUESTIONNAIRE_TOOL_NAME),
// In plan mode, also stop after writing a plan or exiting plan mode.
...(planModeOnly
? [hasToolCall(writePlanTool.name), hasToolCall(exitPlanTool.name)]
: []),
],
```
How can I resolve this? If you propose a fix, please make it concise.![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fd18dac76d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| setPendingQuestionnaire((prev) => { | ||
| if (!prev.has(chatId)) return prev; | ||
| const next = new Map(prev); | ||
| next.delete(chatId); | ||
| return next; |
There was a problem hiding this comment.
This unconditionally drops the active questionnaire on any chat:stream:end for the chat, but chat:stream:end is not tied to a specific request and overlapping streams are possible (the stream handler keys only by chatId). If an older stream ends after a newer stream has already opened a questionnaire, the UI banner disappears while waitForQuestionnaireResponse in main is still pending, leaving the user unable to answer and the agent stalled until timeout.
Useful? React with 👍 / 👎.
| const next = new Map(prev); | ||
| next.set(payload.chatId, payload); | ||
| return next; |
There was a problem hiding this comment.
Storing only one questionnaire per chatId means a newer questionnaire overwrites an older one from the same chat. The backend waits per requestId, so if the model emits multiple planning_questionnaire calls in one turn, only the last prompt is answerable in the UI and earlier requests remain unresolved until timeout, blocking completion of that turn.
Useful? React with 👍 / 👎.
🔍 Dyadbot Code Review SummaryVerdict: ✅ YES - Ready to merge Reviewed by 3 independent agents: Correctness Expert, Code Health Expert, UX Wizard. Issues Summary
🟢 Low Priority Notes (7 items)
🚫 Dropped False Positives (6 items)
Generated by Dyadbot multi-agent code review |
| return next; | ||
| }); | ||
| }, 300); | ||
| }, 1700); |
There was a problem hiding this comment.
🟡 MEDIUM | interaction
Nested setTimeout chain leaks on unmount
The nested setTimeout calls in handleSubmit (1700ms outer, then 300ms inner) are fire-and-forget — they are not cleaned up if the component unmounts or the user navigates to a different chat. This can cause setSubmittedChatIds to update state for a chat the user is no longer viewing, and in React strict mode may trigger "can't update unmounted component" warnings.
💡 Suggestion: Store the timeout IDs in a ref and clear them in a useEffect cleanup, or move the animation state machine into a useEffect that watches the questionnaireSubmittedChatIdsAtom state.
| }); | ||
| clearQuestionnaire(); | ||
| }, | ||
| 5 * 60 * 1000, |
There was a problem hiding this comment.
🟡 MEDIUM | interaction
Auto-dismiss after 5 minutes gives no warning to the user
The questionnaire silently disappears after 5 minutes with no countdown or warning. A user who stepped away or is thinking about their answers will return to find the questionnaire gone and their in-progress answers lost. The agent will then receive a null response indicating dismissal, which may confuse the user since they did not intentionally dismiss it.
💡 Suggestion: Show a countdown indicator or at least a warning toast (e.g., at 30 seconds remaining) so the user knows the questionnaire is about to expire.
🎭 Playwright Test Results❌ Some tests failed
Summary: 719 passed, 2 failed, 7 flaky, 230 skipped Failed Tests🍎 macOS
📋 Re-run Failing Tests (macOS)Copy and paste to re-run all failing spec files locally: npm run e2e \
e2e-tests/problems.spec.ts
|
wwwillchen
left a comment
wwwillchen
left a comment
There was a problem hiding this comment.
thanks @azizmejri1 - lgtm, merging now.
A good follow-up is to show some kind of notification to the user when they need to answer a questionnaire:
- Not focused on Dyad app: when the setting "Show notification when chat completes" is toggled on, we show a native notification when the chat stream is done. we should also show a notification when a questionnaire is asked if the dyad app is not focused on or the user is looking at a different app within the dyad app (e.g. different tab).
a lot of times users are multi-tasking so having this UX feedback will nudge them to answer in a timely manner before the 5 minute timeout kicks in.
🎭 Playwright Test Results❌ Some tests failed
Summary: 699 passed, 9 failed, 13 flaky, 230 skipped Failed Tests🍎 macOS
🪟 Windows
📋 Re-run Failing Tests (macOS)Copy and paste to re-run all failing spec files locally: npm run e2e \
e2e-tests/mcp.spec.ts \
e2e-tests/problems.spec.ts \
e2e-tests/setup_flow.spec.ts
|
2 participants
Summary by cubic
Enable planning_questionnaire in normal agent mode and enforce a clarify‑before‑plan workflow with explicit examples. Also adds a one‑pass todo follow‑up so the agent completes remaining tasks before stopping.
New Features
Bug Fixes
Written for commit c6a67ea. Summary will update on new commits.