fix(flows): builder turn-budget wiring + prompt guidance + capped UX (B31-B34)#4865
Conversation
…aph guidance B32: replace "Interpreting dry-run results honestly" with an expanded version that adds an authoritative per-node-type sandbox mock behavior table (trigger/agent/tool_call/http_request/code echo shapes vs. transform/condition/switch/split_out/merge real execution), verified against SchemaAwareMockAgentRunner (tinyflows/caps.rs), the vendored tinyflows mock (vendor/tinyflows/src/caps/mock.rs), and the envelope/ non-envelope node executors in vendor/tinyflows/src/nodes/. Adds an explicit "NEVER run isolated probe graphs" directive — in the observed incident the agent burned 3 of its 10-step budget re-discovering documented-but-undocumented mock behavior. B33: add a "Graph complexity — prefer the minimal viable graph" subsection (fold formatting into the agent node, skip split/merge for single-item flows, chain reasoning steps in one agent node, target 3-6 nodes) — the agent previously built an 11-node graph for a task that needs ~4. Zero code risk: prompt-only change.
…on cap (B31) flows_build built the workflow_builder agent via Agent::from_config_for_agent, which stamps config.agent.clone() onto the session unconditionally — reading the GLOBAL config.agent.max_tool_iterations (default 10) rather than the workflow_builder AgentDefinition's effective_max_iterations() (50, from agent.toml's iteration_policy = "extended"). Only the sub-agent-runner path (spawn_subagent, used when workflow_builder runs as a chat delegate) applied the definition's cap; the direct flows_build RPC path silently got 10, burning its whole budget on a handful of dry-run cycles for anything past a trivial graph. Add apply_builder_iteration_cap, which clones the config and overrides agent.max_tool_iterations from the resolved AgentDefinition before building the agent, with a debug log of the override and a safe fallback to the global default if the registry/definition isn't available. Also raise FLOW_BUILD_TIMEOUT_SECS 300 -> 600 to match FLOW_RUN_TIMEOUT_SECS: at 50 iterations x ~10s, a worst-case run can now approach 500s, which the old 300s bound would have clipped before the (now-correct) iteration cap ever got a chance to. Unit test asserts the builder's resolved cap is 50 (effective_max_iterations()), not the global default of 10, both directly and through the actually-built Agent's agent_config().
…hit (B34)
When a flows_build turn hits the tool-call budget with no proposal
yet, flows_build returned { proposal: null, assistant_text: "<Done so
far / Next steps checkpoint>", error: null } — indistinguishable, in
the response shape, from the agent voluntarily asking a clarifying
question. The frontend rendered the checkpoint as a plain agent
bubble with no signal that the turn was budget-capped and no
one-click way to resume.
Backend: add last_turn_hit_cap (session/types.rs), set from the
tinyagents outcome right after it resolves in turn/core.rs (before
any early return, so it always reflects the latest turn), and expose
it via a new Agent::last_turn_hit_cap() accessor (runtime.rs).
flows_build (ops.rs) reads it after the run and adds a `capped` field
to its response JSON, true only when the turn hit the cap AND
produced no proposal (a turn that squeezed out a proposal despite
hitting the cap has nothing left to continue).
Frontend: BuilderTurnResult gains `capped` (flowsApi.ts);
useWorkflowBuilderChat exposes a `capped` state, reset at the start
of every send() and set from the turn result so it never reflects a
stale prior turn; WorkflowCopilotPanel renders a "Continue building"
card (amber-bordered, matching the existing ocean proposal / coral
error card pattern) when capped, with a button that routes through
the same `submit` path a typed follow-up would — the existing
pendingAskRef mechanism (a capped turn also has proposed === false)
automatically carries the original ask forward so the resumed turn
keeps full context.
i18n: flows.copilot.cappedNotice / flows.copilot.continueBuilding
added to en.ts with real (non-placeholder) translations in all 13
other locales; pnpm i18n:check passes with 0 missing/extra keys.
Tests: Vitest coverage for the hook (capped set/cleared/reset-on-new-turn)
and the panel (card renders only when capped && !proposal, click
dispatches a follow-up turn).
|
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 (17)
🚧 Files skipped from review as they are similar to previous changes (12)
📝 WalkthroughWalkthroughThe workflow builder now detects iteration-cap pauses, exposes capped status through the API and chat hook, and renders a localized “Continue building” action when no proposal is pending. Backend timeout and effective-cap handling, tests, and builder prompt guidance were also updated. ChangesWorkflow builder cap handling
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant WorkflowCopilotPanel
participant useWorkflowBuilderChat
participant buildWorkflow
participant flows_build
participant Agent
WorkflowCopilotPanel->>useWorkflowBuilderChat: click Continue building
useWorkflowBuilderChat->>buildWorkflow: submit revise request with baseGraph
buildWorkflow->>flows_build: send builder turn
flows_build->>Agent: run with effective iteration cap
Agent-->>flows_build: cap state and proposal result
flows_build-->>buildWorkflow: return capped response
buildWorkflow-->>useWorkflowBuilderChat: update capped state
useWorkflowBuilderChat-->>WorkflowCopilotPanel: render continuation state
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
coderabbitai
Bot
added
agent
![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: 1d0b8a1b72
ℹ️ 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".
| const continueBuilding = useCallback(() => { | ||
| void submit(t('flows.copilot.continueBuilding')); |
There was a problem hiding this comment.
When a builder turn hits the cap before producing a proposal, the canvas graph is still the old draft and flows_build creates a fresh workflow_builder agent for each RPC, so the prior tool history/checkpoint text is not included in the next BuilderTurnRequest. In that case this button sends only the generic continue text through submit() (plus the original ask via pendingAskRef), which restarts from the stale graph rather than picking up from the capped turn and can repeatedly hit the same cap. Include the checkpoint/prior transcript or a partial graph in the follow-up before advertising this as a resume action.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 216e98b. Verified: WorkflowCopilotPanel's submit already sends the Continue turn as mode: 'revise' over the CURRENT graph prop and the existing flowId (never a blank create) — so it does resume ON the built-so-far draft, not from scratch. That said, flows_build (ops.rs) spins up a fresh workflow_builder agent per RPC with no server-side tool-history/checkpoint to reattach to, so this was never a literal seamless mid-thought resume — the copy overpromised that. Reworded flows.copilot.cappedNotice (all 14 locales) and the surrounding code comments in WorkflowCopilotPanel.tsx to accurately say Continue builds forward from the current draft (graph + accumulated instruction via pendingAskRef), not that it resumes the exact in-flight reasoning. Added a regression test asserting the Continue turn carries flowId.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
src/openhuman/flows/ops.rs (1)
3310-3364: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff
ops.rscontinues to grow past the repo's file-size guideline.This PR adds
apply_builder_iteration_capto an already very largesrc/openhuman/flows/ops.rs(well over 3500 lines by the shown line numbers). As per coding guidelines, "**/*.rs: Keep Rust files near or below approximately 500 lines and favor small, single-responsibility modules." Not blocking for this PR, but worth tracking for a future extraction pass (e.g. splitting builder-turn concerns into their own module).🤖 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 3310 - 3364, Track the file-size guideline concern for src/openhuman/flows/ops.rs as a future refactoring item rather than changing this PR’s implementation. When extracting later, move builder-turn responsibilities such as apply_builder_iteration_cap into a focused module while preserving its existing fallback and iteration-cap behavior.Source: Coding guidelines
app/src/hooks/useWorkflowBuilderChat.ts (1)
362-367: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a diagnostic log when a capped turn is detected.
Every other state transition in this hook (thread creation, rehydrate skip, send blocked/ignored, dispatch failure) is logged via
log(...), but the new cap-hit detection (setCapped(result.capped && !result.proposal)) is silent. This is exactly the kind of new state transition that benefits from a grep-friendly log line when debugging capped-turn/continuation behavior.As per path instructions,
**/*.{rs,ts,tsx}: "New or changed flows must include verbose, grep-friendly diagnostics for entry/exit, branches, external calls, retries/timeouts, state transitions, and errors."🩹 Proposed diagnostic addition
- setCapped(result.capped && !result.proposal); + const nextCapped = result.capped && !result.proposal; + if (nextCapped) { + log('send: turn capped (no proposal) thread=%s', targetThreadId); + } + setCapped(nextCapped);🤖 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/hooks/useWorkflowBuilderChat.ts` around lines 362 - 367, Add a verbose, grep-friendly diagnostic via the hook’s existing log(...) helper when cap-hit detection occurs near setCapped(result.capped && !result.proposal). Log the relevant capped and proposal values and the resulting continuation state, while preserving the current state update behavior.Source: Path instructions
🤖 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/lib/i18n/de.ts`:
- Around line 4221-4222: Update the German translation for
flows.copilot.cappedNotice to state that the iteration limit was reached before
the workflow completed, while preserving the existing message that the Builder
can continue from where it stopped.
In `@app/src/lib/i18n/es.ts`:
- Around line 4175-4177: Update the Spanish translation for
flows.copilot.cappedNotice to describe reaching the builder’s iteration or turn
limit before producing a proposal, rather than running out of workflow steps.
Keep flows.copilot.continueBuilding unchanged and preserve the instruction that
users can continue from where they stopped.
In `@app/src/lib/i18n/it.ts`:
- Around line 4172-4173: Update the Italian translation for
flows.copilot.cappedNotice to describe reaching the iteration limit rather than
exhausting workflow steps, while preserving the existing message that the
builder can resume from where it stopped.
In `@app/src/lib/i18n/pt.ts`:
- Around line 4167-4169: Update the Portuguese translation for
flows.copilot.cappedNotice to describe reaching the iteration/turn limit before
completing the workflow, replacing the wording that indicates running out of
steps. Preserve the meaning that continuing resumes from where the builder
stopped.
In `@app/src/lib/i18n/ru.ts`:
- Around line 4139-4141: Update the Russian translation for
flows.copilot.cappedNotice to replace “шаги” with terminology referring to
iterations or turns, while preserving the message’s meaning that the builder can
continue from where it stopped. Leave flows.copilot.continueBuilding unchanged.
---
Nitpick comments:
In `@app/src/hooks/useWorkflowBuilderChat.ts`:
- Around line 362-367: Add a verbose, grep-friendly diagnostic via the hook’s
existing log(...) helper when cap-hit detection occurs near
setCapped(result.capped && !result.proposal). Log the relevant capped and
proposal values and the resulting continuation state, while preserving the
current state update behavior.
In `@src/openhuman/flows/ops.rs`:
- Around line 3310-3364: Track the file-size guideline concern for
src/openhuman/flows/ops.rs as a future refactoring item rather than changing
this PR’s implementation. When extracting later, move builder-turn
responsibilities such as apply_builder_iteration_cap into a focused module while
preserving its existing fallback and iteration-cap behavior.
🪄 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: 9ab54078-68f3-490c-ab54-bace9795eb83
📒 Files selected for processing (26)
app/src/components/flows/WorkflowCopilotPanel.test.tsxapp/src/components/flows/WorkflowCopilotPanel.tsxapp/src/hooks/useWorkflowBuilderChat.test.tsapp/src/hooks/useWorkflowBuilderChat.tsapp/src/lib/i18n/ar.tsapp/src/lib/i18n/bn.tsapp/src/lib/i18n/de.tsapp/src/lib/i18n/en.tsapp/src/lib/i18n/es.tsapp/src/lib/i18n/fr.tsapp/src/lib/i18n/hi.tsapp/src/lib/i18n/id.tsapp/src/lib/i18n/it.tsapp/src/lib/i18n/ko.tsapp/src/lib/i18n/pl.tsapp/src/lib/i18n/pt.tsapp/src/lib/i18n/ru.tsapp/src/lib/i18n/zh-CN.tsapp/src/services/api/flowsApi.tssrc/openhuman/agent/harness/session/builder/setters.rssrc/openhuman/agent/harness/session/runtime.rssrc/openhuman/agent/harness/session/turn/core.rssrc/openhuman/agent/harness/session/types.rssrc/openhuman/flows/agents/workflow_builder/prompt.mdsrc/openhuman/flows/ops.rssrc/openhuman/flows/ops_tests.rs
…t/pt/ru) CodeRabbit review: in these five locales the cappedNotice translated 'ran out of steps' with a word that literally means workflow steps/nodes (Schritte/pasos/passaggi/etapas/шаги), which reads as if the workflow graph is incomplete rather than the builder hitting its turn/iteration budget. Reword to convey 'reached its limit' — accurate, non-technical, and free of the workflow-domain collision. EN source and unflagged locales unchanged.
Maintainer review changesSynced onto fresh Fixed — CodeRabbit
|
M3gA-Mind
left a comment
M3gA-Mind
left a comment
There was a problem hiding this comment.
Reviewed — CI green and no outstanding review comments. Approving and merging.
…aft (tinyhumansai#4865) CodeRabbit (5 locales, same issue): the capped-turn notice said the builder "ran out of steps" — confusing against graph nodes, which are also called "steps" elsewhere in the UI. Reword to name the real condition: the builder hit its iteration limit before producing a proposal. Fixed en.ts first, then retranslated all 14 locales. Codex (WorkflowCopilotPanel.tsx:342): verified "Continue building" already carries the CURRENT draft graph + the existing flowId through a `revise` turn (never a blank `create` restart) — `flows_build` has no server-side session/tool-history checkpoint to resume (a fresh `workflow_builder` agent runs per RPC), so this was never a literal seamless resume. What actually carries forward is the live draft graph (untouched by a capped turn, since revise_workflow/propose_workflow never persist without reaching this panel) plus the accumulated instruction text via `pendingAskRef`. Reworded the capped notice and the surrounding code comments to say "continuing builds from the current draft" instead of implying continuity the architecture doesn't provide, and added a regression test asserting the Continue turn carries `flowId`. No Rust changes — flows_build's revise-mode wiring was already correct.
216e98b
The B31 fix bumped FLOW_BUILD_TIMEOUT_MS 310_000 -> 610_000 (matching the backend FLOW_BUILD_TIMEOUT_SECS 300 -> 600), but two buildWorkflow test assertions in flowsApi.test.ts still expected the old 310_000. Discover tests correctly stay at 310_000 (FLOW_DISCOVER unchanged).
coderabbitai
Bot
removed
the
feature
2 participants
Summary
flows_buildbuilt theworkflow_builderagent viaAgent::from_config_for_agent, which stamps the session's iteration cap from the GLOBALconfig.agent.max_tool_iterations(default 10) — not theworkflow_builderAgentDefinition'seffective_max_iterations()(50, fromagent.toml'siteration_policy = "extended"). Only the sub-agent-runner path (chat delegate) applied the definition's cap; the direct RPC path silently got 10. Fixed by overridingconfig.agent.max_tool_iterationsfrom the resolved definition before building the agent. Also bumpedFLOW_BUILD_TIMEOUT_SECS300 → 600 to matchFLOW_RUN_TIMEOUT_SECS(50 iterations × ~10s can now approach 500s).SchemaAwareMockAgentRunner, the vendoredtinyflowsmock, and the envelope/non-envelope node executors) + an explicit "NEVER run isolated probe graphs" directive.last_turn_hit_capaccessor onAgent, acappedfield onflows_build's response, and a "Continue building" card inWorkflowCopilotPanel(with i18n in all 14 locales) that one-click resumes the turn.Problem
The
workflow_builderagent frequently failed to complete non-trivial workflow builds. Root cause investigation (see commit messages) found the effective iteration budget was silently 10, not the 50 the agent's ownagent.tomlintends (B31) — and that budget was further wasted on the agent re-discovering sandbox mock behavior through probe graphs (B32) and building needlessly large graphs (B33). When the budget did run out, the UI gave no signal that the turn was capped rather than finished (B34).Solution
See per-bug summary above; each bug is an isolated commit:
2fb0c8a9b— B32 + B33, prompt-only, zero code risk.8e5be721d— B31,apply_builder_iteration_capoverride insrc/openhuman/flows/ops.rs::flows_build+ timeout bump, with a unit test asserting the resolved cap is 50 (not the global default of 10), both directly and through the actually-builtAgent.1d0b8a1b7— B34,last_turn_hit_capthreaded from the tinyagents turn outcome (session/types.rs,session/turn/core.rs,session/runtime.rs) throughflows_build'scappedresponse field, toBuilderTurnResult/useWorkflowBuilderChat/WorkflowCopilotPanelon the frontend, with i18n inen.ts+ all 13 other locales.Verification
GGML_NATIVE=OFF cargo check --manifest-path Cargo.toml— clean.GGML_NATIVE=OFF cargo check --manifest-path app/src-tauri/Cargo.toml— clean.cargo test --manifest-path Cargo.toml --lib openhuman::flows:: -- --test-threads=1— 288 passed, 0 failed.cargo test --manifest-path Cargo.toml --lib openhuman::agent::harness::session -- --test-threads=1— 136 passed, 0 failed, excluding two confirmed pre-existing flakes reproduced identically on the base commit before any of this PR's changes:turn_dispatches_spawn_subagent_through_full_path(stack overflow — deep async recursion in a debug build, unrelated to this diff) andturn_triggers_configured_memory_agent_before_parent_prompt(aSequenceProviderresponse-ordering assertion, unrelated to this diff — fails in isolation onmaintoo).pnpm typecheck— clean.pnpm lint(fullapp/) — 0 errors (pre-existing unrelated warnings only; 0 in touched files).WorkflowCopilotPanel.test.tsx,useWorkflowBuilderChat.test.ts) — 52 passed.pnpm i18n:check— 0 missing/extra keys across all 14 locales.cargo fmt+pnpm format— applied, no diffs.Confirmed manually against source (not just the plan):
apply_builder_iteration_capresolves the builder's cap to 50 (workflow_builder'sagent.tomldeclaresiteration_policy = "extended",max(max_iterations, EXTENDED_MAX_TOOL_ITERATIONS)=max(12, 50)= 50), and the B32 sandbox table was verified row-by-row againstSchemaAwareMockAgentRunner(src/openhuman/tinyflows/caps.rs), the vendored mock (vendor/tinyflows/src/caps/mock.rs), and the envelope/non-envelope node executors (vendor/tinyflows/src/nodes/**) — including acode-node subtlety the plan draft implied correctly but that took aItem/envelope trace to confirm (code does not envelope;.item.resultresolves directly).Impact
FLOW_BUILD_TIMEOUT_SECSraised 300s → 600s — aflows_buildRPC call can now legitimately run up to 10 minutes server-side; the client-side timeout (flowsApi.ts) was raised in lockstep.Submission Checklist
cappedstate (set/cleared/reset-on-new-turn) and the "Continue building" card (renders only whencapped && !proposal, does not render for a normal turn, does not render alongside a stale-capped + proposal turn, click dispatches a follow-up).last_turn_hit_capplumbing, frontendcappedstate/card) is covered by the tests above;pnpm test:coverage/pnpm test:rustnot run in full locally (long-running), but every changed code path has an explicit assertion.docs/TEST-COVERAGE-MATRIX.mdfor the internalworkflow_buildercopilot RPC; this is a bug fix to existing untracked behavior, not a new user-facing feature row.Related
build_session_agent_innercould applyAgentDefinition.effective_max_iterations()for every agent built viafrom_config_for_agent, not just this one call site — is intentionally deferred as a broader, separate change.AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix/flows-builder-turn-budgetValidation Run
pnpm --filter openhuman-app format:check— clean (ranpnpm format, no diff produced).pnpm typecheck— clean.WorkflowCopilotPanel.test.tsx,useWorkflowBuilderChat.test.ts(52 passed);cargo test --lib openhuman::flows::(288 passed).cargo fmt --manifest-path Cargo.toml,cargo check --manifest-path Cargo.toml— clean.cargo check --manifest-path app/src-tauri/Cargo.toml— clean.Validation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
workflow_buildercopilot now gets its intended 50-iteration budget instead of the global 10-iteration default; the prompt steers it away from wasted probe graphs and over-large graphs; a cap-hit turn is now visually distinct with a one-click resume.Parity Contract
spawn_subagent) path forworkflow_builderis untouched — it already appliedeffective_max_iterations()correctly. Non-capped turns (proposal or clarifying question) are unaffected;cappedisfalsefor both.apply_builder_iteration_capfalls back to the unmodified global config if the agent registry or theworkflow_builderdefinition isn't available (same as pre-fix behavior), rather than failing the RPC.Duplicate / Superseded PR Handling
Summary by CodeRabbit