fix(flows): guarantee a terminal state on every flows_build turn (no silent trail-off)#4887
Conversation
The workflow_builder copilot could end a turn with no proposal, no error,
and no cap hit — just a status-dump reply ("Done so far: checked
connections...") when the model ran out of steam mid-build. The frontend
renders whatever assistant_text comes back, so the user was left staring
at an unactionable note with nothing to click or answer.
flows_build now detects this "trail-off" state explicitly:
!capped && proposal.is_none() && run_error.is_none()
When it fires and the model's own text doesn't already read as a question
(text_looks_like_question — conservative substring/last-line heuristic),
the assistant_text is replaced with a synthesized fallback
(build_trail_off_fallback) that scans the turn's tool history for the last
builder-authoring-tool result that reads as a blocker (a hard-gate
rejection, or a dry_run_workflow/validate_workflow "ok": false body) and
asks the user a concrete question about it, or falls back to a generic
"what should I focus on" question when no such blocker is found. This is a
backend invariant, not a prompt hope: the user is never left with silence.
The proposal/capped computation was reordered to run BEFORE the streamed
chat-bubble finalization (finalize_flow_stream), so the copilot pane's live
stream renders the same guaranteed text as the RPC response — patching only
the return value would still leave an interactive user staring at the
original silent turn.
Adds a `trail_off` field to the flows_build response for observability.
Tests: text_looks_like_question (trailing '?', markdown noise, accepted
false-negative on a trailing pleasantry line), build_trail_off_fallback
(always yields a question; surfaces the last dry-run/gate-rejection
blocker; ignores unrelated read-tool output and successful proposals;
picks the most recent of multiple blockers).
|
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 (2)
📝 WalkthroughWalkthrough
ChangesBuilder convergence
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant BuilderTurn
participant flows_build
participant BuilderToolHistory
participant RPCClient
BuilderTurn->>flows_build: complete agent turn
flows_build->>BuilderToolHistory: extract proposal and inspect results
BuilderToolHistory-->>flows_build: proposal or latest blocker
flows_build->>flows_build: synthesize terminal question when needed
flows_build-->>RPCClient: stream final text and trail_off state
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
coderabbitai
Bot
added
bug
rust-core
![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: baaccf467f
ℹ️ 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".
| if let Some(desc) = describe_tool_result_blocker(&result.content) { | ||
| return Some(crate::openhuman::util::truncate_with_ellipsis(&desc, 500)); |
There was a problem hiding this comment.
In a trail-off turn where the builder first gets a failure from validate_workflow/dry_run_workflow, then fixes it and gets a later { "ok": true } result but still ends without a proposal, this reverse scan skips the success and continues to the older failure. The fallback will tell the user “here's where I got stuck” with an issue that was already resolved, which can send them down the wrong path; once the latest relevant builder-tool result is a success/progress payload, the scan should stop or otherwise avoid returning earlier blockers from the same turn.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in c7e43fa. last_builder_tool_blocker now returns as soon as it finds the MOST RECENT authoring-belt tool result (whether it reads as a blocker or a success), instead of skipping past a resolved success to an older failure. Added a regression test (build_trail_off_fallback_does_not_resurface_a_resolved_blocker) that pins the failure→fix→trail-off scenario you described.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/openhuman/flows/ops.rs (1)
3760-3772: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd correlation fields to the new/changed trail-off logs.
Both the trail-off
warn!(wholly new) and the turn-completeinfo!(changed to addtrail_off) omitflow_id/correlation fields that the entry log at Line 3636-3643 already includes, making these harder to grep/correlate to a specific turn in production logs.🔧 Proposed fix
tracing::warn!( target: "flows", + flow_id = req.flow_id.as_deref().unwrap_or("<none>"), original_len = assistant_text.len(), fallback_len = fallback.len(), "[flows] flows_build: trail-off detected (no proposal, no cap, no question) — \ guaranteeing a fallback question instead of silence" );tracing::info!( target: "flows", + flow_id = req.flow_id.as_deref().unwrap_or("<none>"), has_proposal = proposal.is_some(), hit_cap, capped, trail_off, "[flows] flows_build: workflow_builder turn complete" );Based on learnings/guidelines: "For new or changed flows, log entry and exit, branches, external calls, retries/timeouts, state transitions, and errors using stable grep-friendly prefixes and correlation fields."
Also applies to: 3787-3794
🤖 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 `@src/openhuman/flows/ops.rs` around lines 3760 - 3772, Add the same flow_id and correlation fields used by the entry log to the trail-off tracing::warn! block and the turn-complete tracing::info! block in flows_build. Preserve their existing messages and metrics while ensuring both logs can be correlated to the specific turn.Source: Coding guidelines
🤖 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 `@src/openhuman/flows/ops.rs`:
- Around line 3760-3772: Add the same flow_id and correlation fields used by the
entry log to the trail-off tracing::warn! block and the turn-complete
tracing::info! block in flows_build. Preserve their existing messages and
metrics while ensuring both logs can be correlated to the specific turn.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f0c58ce4-93b6-4df5-af1b-9c1e75572c98
📒 Files selected for processing (2)
src/openhuman/flows/ops.rssrc/openhuman/flows/ops_tests.rs
Addresses @chatgpt-codex-connector on src/openhuman/flows/ops.rs:3909 (P2): last_builder_tool_blocker scanned backward past a resolved success straight to an older failure, so the trail-off fallback could tell the user "here's where I got stuck" about a problem already fixed earlier in the same turn. The scan now stops at the most recent authoring-belt tool result and reports it (blocker or not) instead of walking further back. Also addresses @coderabbitai nitpick on ops.rs:3760-3772/3787-3794: add flow_id to the trail-off warn! and turn-complete info! logs to match the entry log's correlation fields.
|
Addressed @coderabbitai's nitpick (flow_id correlation fields on the trail-off Also fixed @chatgpt-codex-connector's P2 finding (inline reply on that thread): |
|
To use Codex here, create a Codex account and connect to github. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
cd /repo 2>/dev/null || true
git log --oneline -5 2>/dev/null
git show c7e43facf --stat 2>/dev/null | head -30Length of output: 1661 Confirmed — commit c7e43fa addresses both points cleanly: [resolve] |
coderabbitai
Bot
removed
the
rust-core
1 participant
Summary
flows_build(theworkflow_buildercopilot) could end a turn withOk{proposal:null}— no proposal, no question, no cap hit. The copilot pane then freezes on a half-written message and the user can't re-send. This guarantees every turn ends in a terminal state: a proposal, or a clear next-step/question — never silence.Problem
Observed live: a build (calendar → Slack DM) grounded fully, reasoned aloud, then just stopped mid-thought.
has_proposal=false hit_cap=false capped=false. The UI has no settled state to render, so the pane hangs. #4881 does not address this (its own TODO notes the flow_discovery terminal-tool gap).Solution
Backstop in
flows_build(src/openhuman/flows/ops.rs): when the agent turn ends without a proposal and without hitting the iteration cap, synthesize a terminal outcome so the caller always receives either a proposal or an explicit, plain-language next-step — never a silent null.Testing
cargo test --lib openhuman::flows::→ 340 passed.GGML_NATIVE=OFF cargo checkclean;cargo fmtapplied.flows_build/extract_workflow_proposal).Acceptance criteria
Ok{proposal:null}silent trail-off; turn always terminalops_tests.rscovers the terminal-state guaranteeSummary by CodeRabbit
Bug Fixes
Tests