feat(flows): builder clarifies ambiguous inputs instead of guessing (orchestrator posture)#4630
Conversation
…orchestrator posture)
The workflow_builder emitted a complete graph in one shot and GUESSED ambiguous
required inputs — e.g. hardcoding Slack channel:"#general" for "send to me", which
should be a DM. It never asked. Give it the orchestrator's posture (same harness
loop, purely prompt directives): clarify, verify, don't stop until the graph is
RIGHT — without regressing the fast "just build it" feel.
prompt.md (Style section) — the ask-vs-just-build decision rule: once get_tool_contract
gives a node's required_args, sort each into
1. WIRED — an upstream node produces it → bind, no question
2. INFERABLE — the request implies it ("to me" → the user's OWN DM, NEVER #general;
one connected account → that account; obvious default) → fill it +
name the choice in the summary so it's correctable
3. AMBIGUOUS — a required target/recipient the user never gave, no upstream produces,
multiple reasonable values → ask ONE question and STOP the turn
(no propose/save this turn); continue after the user answers.
Ask ONLY on bucket 3 (~1-2 max); "zero questions is the happy path." Plus a verify
loop (dry_run_workflow → fix → retry until clean BEFORE proposing) and a summary
directive to state inferred choices.
useWorkflowBuilderChat.ts — render a clarifying-question turn: the blocking fallback
dropped assistantText when there's no proposal/error; now surfaces it as an agent
message (WorkflowCopilotPanel already renders it), guarded against double-appending
when the streamed chat_done path already delivered it.
typecheck/eslint clean; cargo check clean (prompt.md is include_str!); vitest 8/8.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR updates workflow builder chat handling to return proposal status, avoid duplicating streamed assistant text, preserve pending asks across clarifying turns, and revise the workflow builder prompt with structured ambiguity and verification rules. ChangesWorkflow builder chat flow
Workflow builder prompt rules
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
![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: 3d2e3145c3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| connected and no hint which). **Ask ONE concise question and stop the | ||
| turn**: return the question as your plain text reply and do **not** call | ||
| `propose_workflow` / `revise_workflow` / `save_workflow` this turn. Wait | ||
| for the user's answer on the next turn before building further. |
There was a problem hiding this comment.
When this branch is hit, the next user reply cannot be resolved as intended: WorkflowCopilotPanel submits follow-up text as mode: 'revise', instruction: trimmed, graph, flowId, and render_prompt only injects the current graph plus that latest instruction, not this thread's prior messages or original request. Because these lines forbid proposing/revising before stopping, the graph stays blank/old, so an answer like #eng is rendered to the builder as “Revise it as follows: #eng”, losing the original “post a daily summary to Slack” context and causing clarification flows to fail or build nonsense. Either carry the original brief/thread history into the next turn or preserve enough draft context before asking.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in cbcd731 — the follow-up submit in WorkflowCopilotPanel now tracks the unresolved original ask (pendingAskRef). useWorkflowBuilderChat's send() resolves { proposed: boolean } so the panel knows whether a turn actually produced a proposal; while it hasn't, the next revise turn's instruction is prefixed with the original ask (e.g. "post a daily summary to slack") plus the user's short answer ("#eng"), instead of sending the bare answer alone. Cleared once a proposal lands, since the graph then carries the state. Covered by a new panel test (carries the original ask forward across a clarifying-question turn...).
coderabbitai
Bot
added
the
feature
![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 `@app/src/hooks/useWorkflowBuilderChat.ts`:
- Around line 200-226: Add debug tracing to the new assistant-message dedup path
in useWorkflowBuilderChat.send so the append-or-skip decision is observable in
production. Reuse the existing log(...) pattern used in nearby branches and log
with a stable prefix plus correlation fields like targetThreadId, result/turn
identifiers, and whether alreadyStreamed was true or false. Emit one message
before the check and one when skipping or appending, without including secrets
or full message content, so duplicate-message reports can be diagnosed.
In `@src/openhuman/flows/agents/workflow_builder/prompt.md`:
- Around line 391-403: The clarification rule in prompt.md is too narrow and
allows multiple questions, which conflicts with the one-question turn behavior.
Update the “GENUINELY AMBIGUOUS” guidance so it applies to any required arg in
required_args that is truly ambiguous (not just target/recipient/destination),
and keep the behavior to exactly one concise question before stopping the turn.
Make sure the workflow_builder prompt’s ambiguity policy stays aligned with the
existing propose_workflow / revise_workflow / save_workflow turn rules.
🪄 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: CHILL
Plan: Pro
Run ID: 669af9ae-7c85-4d08-9ac2-247d0ff38a4f
📒 Files selected for processing (3)
app/src/hooks/useWorkflowBuilderChat.test.tsapp/src/hooks/useWorkflowBuilderChat.tssrc/openhuman/flows/agents/workflow_builder/prompt.md
…addresses @chatgpt-codex-connector on prompt.md:397) WorkflowCopilotPanel's follow-up submit always sent mode:'revise' with only the latest short answer as `instruction`. When the builder asks a genuinely- ambiguous clarifying question (prompt.md's new bucket-3 branch) and stops without proposing, the graph stays blank/stale, so the next turn rendered as "Revise it as follows: #eng" with no idea the original ask was "post a daily summary to Slack" — exactly the failure Codex flagged. useWorkflowBuilderChat's `send` now resolves `{ proposed: boolean }` so the panel knows whether a turn actually produced a proposal. The panel tracks the unresolved ask in `pendingAskRef`, prepends it to the next turn's instruction until a proposal lands, then drops it (the graph carries state from then on).
…bitai on useWorkflowBuilderChat.ts:236) The new alreadyStreamed dedup branch decided whether to append or skip a message with no logging, unlike its sibling branches in send(). Per CLAUDE.md's debug-logging rule, add a log() call so duplicate-message reports are diagnosable in production.
… fix question-count contradiction (addresses @coderabbitai on prompt.md:403) Bucket 3 was scoped to TARGET/RECIPIENT/DESTINATION args only, missing other genuinely-ambiguous required args (subject, filename, timezone, etc.). The surrounding text also contradicted itself: "Ask ONE concise question" vs. "Cap yourself at 1-2 questions per turn". Broaden the bucket and standardize on exactly one question per turn, matching the closing "Style" section's "a single clarifying question".
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/src/components/flows/WorkflowCopilotPanel.tsx (1)
149-168: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd debug logging for the new pending-ask tracking flow.
This is a brand-new conditional flow (bucket-3 clarifying turns), but none of the three call sites that set/clear
pendingAskRef(repair auto-send, build auto-send,submit) emit any debug log. The sibling hook (useWorkflowBuilderChat.ts) already uses a stable-prefixedlog(...)convention (e.g.send: assistantText fallback thread=%s alreadyStreamed=%s) for comparable branching — this new flow should follow the same pattern (e.g.log('copilot: pendingAsk %s thread=%s', proposed ? 'cleared' : 'set', threadId)), without logging the raw instruction/answer text (PII).While addressing this, consider extracting the repeated
pendingAskRef.current = proposed ? null : X;pattern (three occurrences) into a small helper that also does the logging, to avoid drift.♻️ Suggested helper
+ const updatePendingAsk = useCallback( + (proposed: boolean, ask: string) => { + log('copilot: pendingAsk %s thread=%s', proposed ? 'cleared' : 'set', threadId); + pendingAskRef.current = proposed ? null : ask; + }, + [threadId] + );Then replace each
pendingAskRef.current = proposed ? null : X;withupdatePendingAsk(proposed, X);.As per coding guidelines: "On new or changed flows, add verbose debug logging with stable prefixes, correlation fields, and no secrets or full PII; incomplete changes are those lacking logging."
Also applies to: 181-196, 204-220
🤖 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 `@app/src/components/flows/WorkflowCopilotPanel.tsx` around lines 149 - 168, The new pending-ask branching in WorkflowCopilotPanel lacks debug logging at each place where pendingAskRef is set or cleared, so add stable-prefixed debug logs to the repair auto-send effect, the build auto-send path, and submit using the existing log convention from useWorkflowBuilderChat.ts with correlation fields like threadId. While updating those call sites, extract the repeated pendingAskRef.current = proposed ? null : X pattern into a small helper in WorkflowCopilotPanel that both updates the ref and logs whether the pending ask was set or cleared, and make sure the helper never logs raw instruction or answer text.Source: Coding guidelines
app/src/components/flows/WorkflowCopilotPanel.test.tsx (1)
79-137: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGood coverage of the manual Q&A path; consider covering the seed-originated paths too.
This test verifies
pendingAskRefpropagation only for thesubmit()-driven flow. TherepairSeed/buildSeedauto-send effects (lines 149-168, 181-196 inWorkflowCopilotPanel.tsx) also setpendingAskReffrom{ proposed: false }, but that code path isn't exercised by any test — e.g. rendering withbuildSeedset, resolvingsendwith{ proposed: false }, then confirming a subsequent free-text answer carriesbuildSeed.descriptionforward.🤖 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 `@app/src/components/flows/WorkflowCopilotPanel.test.tsx` around lines 79 - 137, The current test only covers the manual submit flow, but it misses the auto-send seed paths that also populate pendingAskRef. Add coverage in WorkflowCopilotPanel.test.tsx for the buildSeed/repairSeed-driven effect by rendering WorkflowCopilotPanel with a seed present, mocking hookState.send to first return { proposed: false }, then sending a follow-up free-text answer and asserting the second call’s request.instruction includes the seed description plus the new user text. Use the existing WorkflowCopilotPanel, hookState.send, and pendingAskRef-related behavior to locate and validate the auto-send path.
🤖 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 `@app/src/components/flows/WorkflowCopilotPanel.test.tsx`:
- Around line 79-137: The current test only covers the manual submit flow, but
it misses the auto-send seed paths that also populate pendingAskRef. Add
coverage in WorkflowCopilotPanel.test.tsx for the buildSeed/repairSeed-driven
effect by rendering WorkflowCopilotPanel with a seed present, mocking
hookState.send to first return { proposed: false }, then sending a follow-up
free-text answer and asserting the second call’s request.instruction includes
the seed description plus the new user text. Use the existing
WorkflowCopilotPanel, hookState.send, and pendingAskRef-related behavior to
locate and validate the auto-send path.
In `@app/src/components/flows/WorkflowCopilotPanel.tsx`:
- Around line 149-168: The new pending-ask branching in WorkflowCopilotPanel
lacks debug logging at each place where pendingAskRef is set or cleared, so add
stable-prefixed debug logs to the repair auto-send effect, the build auto-send
path, and submit using the existing log convention from
useWorkflowBuilderChat.ts with correlation fields like threadId. While updating
those call sites, extract the repeated pendingAskRef.current = proposed ? null :
X pattern into a small helper in WorkflowCopilotPanel that both updates the ref
and logs whether the pending ask was set or cleared, and make sure the helper
never logs raw instruction or answer text.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a84ff551-e156-4ea1-8c01-f92ae186f82e
📒 Files selected for processing (5)
app/src/components/flows/WorkflowCopilotPanel.test.tsxapp/src/components/flows/WorkflowCopilotPanel.tsxapp/src/hooks/useWorkflowBuilderChat.test.tsapp/src/hooks/useWorkflowBuilderChat.tssrc/openhuman/flows/agents/workflow_builder/prompt.md
🚧 Files skipped from review as they are similar to previous changes (3)
- app/src/hooks/useWorkflowBuilderChat.ts
- app/src/hooks/useWorkflowBuilderChat.test.ts
- src/openhuman/flows/agents/workflow_builder/prompt.md
… (addresses CodeRabbit nitpicks on WorkflowCopilotPanel) The new pendingAskRef flow (added to preserve context across a clarifying- question turn) had no debug logging and its repair/build seed call sites weren't covered by a test — both flagged as nitpicks on the re-review. Extract pendingAskRef.current = proposed ? null : X into a shared updatePendingAsk() helper that also logs the set/cleared decision (thread correlation only, never the raw ask/answer text), and add a test exercising the buildSeed auto-send -> clarifying question -> follow-up path.
|
Addressed the two nitpicks from CodeRabbit's re-review (12:47:46 UTC) in d822804:
All 3 prior actionable comments (1 Codex P1 + 2 CodeRabbit) were already fixed and replied to individually in earlier commits (cbcd731, 2cad9ae, 29ca974). |
coderabbitai
Bot
removed
the
feature
Reconcile the two changes to useWorkflowBuilderChat.send(): - This branch (tinyhumansai#4628) returned an outcome (dispatched/skipped/failed) so a seeded auto-send retries only on skipped, never on a real dispatch error. - upstream tinyhumansai#4630 returned { proposed } so a looping caller knows whether the turn is still unresolved (clarifying-question carry-forward). These are orthogonal signals, so send() now returns WorkflowBuilderSendResult { outcome, proposed } and both callers read the field they need. Kept upstream's THREAD_NOT_FOUND cleanup in the catch path. Merged both sides' panel tests.
1 participant
The problem
The
workflow_builderemitted a full graph in one shot and guessed ambiguous required inputs — e.g. hardcoded Slackchannel: "#general"for "send the summary to me", which should be a DM. It never asked. The orchestrator, on the same harness loop, clarifies/verifies until things are right — the difference is purely prompt posture.The fix (prompt + ~15-line frontend, no new architecture)
Decision rule (
prompt.md) — onceget_tool_contractgives a node'srequired_args, sort each:#general; one connected account → that account; obvious default) → fill it + name the choice in the summary so it's correctable.Ask only on bucket 3 (~1–2 max) — "zero questions is the happy path." Plus a verify loop (
dry_run_workflow→ fix → retry until clean before proposing) and a summary directive stating inferred choices.Frontend (
useWorkflowBuilderChat.ts) — render a clarifying-question turn: the blocking fallback droppedassistantTextwhen there was no proposal/error; now surfaces it as an agent message (the panel already renders it), de-dup-guarded so it doesn't double-append when the streamedchat_donepath already delivered it.Builds on today's grounding
Leans on the
ToolContract.required_args(live-catalog grounding) to know what's required → know when to ask.Verification
cargo checkclean (prompt.md isinclude_str!) ·typecheck/eslintclean · vitest 8/8 (new: clarifying-question renders; proposal path doesn't double-append). LLM-behavior calibration (the ask-only-when-needed threshold) is for live testing.Summary by CodeRabbit
New Features
Bug Fixes