docs(flows): fix contract-drift comments and repair the workflow-builder prompt structure - #5290
Merged
Conversation
…t the actual code (PR I) Zero behaviour change except where noted (T-m1). Findings from my_docs/flows_review_2026-07-30.md / _FIXPLAN.md: - P-m1: prompt.md's "## Your authoring loop" opened a numbered list (steps 1-2) then wedged three full H2 sections in before steps 3-6, so the tail of the loop rendered as if it belonged to "Inference provider readiness". Moved the three H2 sections below the complete authoring loop. - P-m4 (coupled with P-m1): builder_prompt.rs's STANDING_PROMPT regression tests asserted exact hard-wrapped substrings of prompt.md, which the P-m1 reflow would otherwise break. Added a whitespace-normalizing contains_normalized() helper and routed every STANDING_PROMPT assertion through it, preserving each assertion's original intent (auto-save tinyhumansai#4596, phantom review card B27, offer-then-refuse Bld §4, self-DM platform_user_id, etc.) while making the suite resilient to future semantically-neutral rewraps. - P-m5: deleted prompt.rs's near-tautological prompt_teaches_the_propose_never_persist_invariant test (body.contains("propose") is true regardless of correctness — the word appears dozens of times regardless of whether the invariant actually holds). Real, falsifiable coverage of the propose-only contract already lives in builder_prompt.rs (create_prompt_frames_propose_only, build_is_propose_only_and_injects_flow_id_as_context, and the standing_prompt_* tests), so this was redundant rather than strengthened. - T-m1 (code change): DryRunWorkflowTool.security was written but never read, and its doc claimed "Autonomy-tier gated ... read-only tier refuses" while permission_level() returns None and dry_run_allowed_under_readonly_tier pins the opposite. Removed the dead field/constructor param (and the now-unused SecurityPolicy import + test policy() helper), updated the one call site in tools/ops.rs and all 21 test call sites, and rewrote the doc to state the actual (intentional, F7) ungated contract. - T-m2: builder_tools.rs's module-doc table listed 11 of 22 tools, gave DryRunWorkflowTool the wrong permission, and claimed "the agent still cannot *create* a flow" directly above CreateWorkflowTool. Rewrote the table from the actual `impl Tool for` registrations, fixed the same stale "no tool to create a flow" claim inside SaveWorkflowTool's own doc, and fixed flows/agents/mod.rs's workflow_builder doc which still said the agent "never persists" despite create_workflow/duplicate_flow/ save_workflow being on its belt. - E-m7: tinyflows/caps.rs's http_cred_name doc claimed "No host-side HTTP credential store exists yet" — false; HttpCredentialsStore exists and is load-bearing in the very next function (resolve_http_credential). Corrected. - R-m9: flows/store.rs's prune_flow_runs_conn doc said it deletes completed/failed/cancelled rows, but the SQL (status NOT IN ('running', 'pending_approval')) also prunes interrupted and completed_with_warnings. The code was right; fixed the comment to match. - Optional (T-m2 follow-up): added module_doc_tool_table_matches_registered_tools, a mechanical drift guard that derives the tool list straight from `impl Tool for` in the same source file (no hardcoded second list to go stale) and fails if the module-doc table and the registered tools ever diverge again, in the spirit of propose_workflow_description_matches_typed_node_contracts. Verified: GGML_NATIVE=OFF cargo check, cargo test --lib openhuman::flows::agents (23 passed), cargo test --lib openhuman::flows::builder_tools (91 passed, incl. the new drift test), plus openhuman::flows::store and openhuman::tinyflows::caps for the doc-only E-m7/R-m9 changes. No Cargo.toml/Cargo.lock changes.
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThe PR updates workflow-builder documentation and prompt guidance, makes prompt regression checks whitespace-insensitive, removes ChangesWorkflow builder contracts
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
Comment |
This was referenced Jul 30, 2026
graycyrus
marked this pull request as ready for review
July 31, 2026 05:58
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
graycyrus has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
coderabbitai
Bot
added
the
rust-core
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
1 participant
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
workflow_builderprompt where steps 3–6 of the authoring loop rendered as if they belonged to a different section.Problem
Documentation in this area had drifted far enough to actively mislead both humans and agents:
prompt.md— "## Your authoring loop" opened a numbered list (1, 2), then three full H2 sections were wedged in, so steps "3. Build the graph" … "6. Debugging a broken saved flow" rendered under Inference provider readiness.builder_prompt.rs— 49 pin assertions matched exact hard-wrapped substrings, several spanning a specific wrap column. Any reflow ofprompt.mdbroke the suite for no semantic reason, which discourages exactly the prompt maintenance the file needs.prompt.rs— a near-tautological test (body.contains("propose")) that cannot meaningfully fail.builder_tools.rs— the module doc table listed 11 of ~22 tools, gaveDryRunWorkflowToolthe wrong permission, and asserted "the agent still cannot create a flow" directly aboveCreateWorkflowTool. A second stale "cannot create" claim also sat insideSaveWorkflowTool's own doc.agents/mod.rs— still claimedworkflow_builder"never persists", thoughsave_workflow/create_workflow/duplicate_floware all on its belt.caps.rs—http_cred_nameclaimed "No host-side HTTP credential store exists yet", whileHttpCredentialsStoreis load-bearing immediately below it.store.rs— the prune doc listed onlycompleted/failed/cancelled, but the SQL (NOT IN ('running','pending_approval')) also prunesinterruptedandcompleted_with_warnings.DryRunWorkflowTool.security— a dead field, never read, whose doc claimed tier-gating that bothpermission_level()and the pinneddry_run_allowed_under_readonly_tiertest contradict.Solution
STANDING_PROMPT.contains(...)assertion through a whitespace-normalizing helper. Every assertion's intent is unchanged; only the comparison is now wrap-insensitive. The pins for Rejecting a seeded flow build leaves the graph persisted #4596 auto-save, B27 phantom review card, Bld §4 offer-then-refuse, and the self-DMplatform_user_idrule all still pin exactly what they did.builder_prompt.rs) and justified it in a comment.impl Tool forregistrations, removed the deadsecurityfield and its stale doc, and corrected the remaining false claims.module_doc_tool_table_matches_registered_tools, which derives the tool list fromimpl Tool formatches in the same source file — no hardcoded second list — and asserts both directions. This turns a recurring manual problem into a mechanical one.Submission Checklist
cargo test --lib "openhuman::flows::"= 549 passed, 0 failedN/A: documentation + dead-code removal, no feature rows added/removed/renamed## Related—N/A: no matrix feature rows affectedN/A: no runtime behaviour changeCloses #NNN—N/A: found by code review, no tracking issue filed yetImpact
workflow_builderstanding prompt is restructured (content move only), so the model now reads a coherent 6-step authoring loop instead of steps 3–6 apparently nested under an unrelated section.Related
N/Abuilder_tools.rs+builder_tools_tests.rswith the authorization-boundaries PR, andstore.rswith the resume-lifecycle PR. Merge this one last of the three; it rebases cleanly.AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
docs/flows-contract-drift52e9f5bbeValidation Run
pnpm --filter openhuman-app format:check— N/A, no frontend files changedpnpm typecheck— N/A, no TypeScript changedGGML_NATIVE=OFF cargo test --lib "openhuman::flows::"→ 549 passed, 0 failed (incl.openhuman::flows::agents23,builder_tools91,store34;tinyflows::caps112 run separately for the doc-only change there)GGML_NATIVE=OFF cargo checkclean;rustfmt --checkclean on all touched filesapp/src-tauriuntouchedValidation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
Parity Contract
dry_run_allowed_under_readonly_tierstill passes after the dead-field removal, confirming the tool's gating posture is genuinely unchanged.Duplicate / Superseded PR Handling
Summary by CodeRabbit