Repository navigation
embed: steer a running turn and hand back unapplied steers - #7853
Conversation
Auto-committed-on: macbook Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds the agent chat operations to the host runtime, wiring up the inference entry points needed to run chat requests through the host runtime. Auto-committed-on: macbook Co-authored-by: Medulla <medulla@tinyhumans.ai>
Move the turn steering logic out of the embed crate's main file into a dedicated module so the steering behaviour is easier to find and maintain. No functional change is intended. Auto-committed-on: macbook Co-authored-by: Medulla <medulla@tinyhumans.ai>
Move the turn steering logic out of the embed crate's main module into a dedicated file so the steering behaviour can be tested and evolved independently. No behaviour change. Auto-committed-on: macbook Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds coverage for turn steering behaviour in the embed crate, exercising the steering paths that previously had no direct tests. Auto-committed-on: macbook Co-authored-by: Medulla <medulla@tinyhumans.ai>
Introduce turn control and turn type modules to the embed crate, giving callers a structured way to manage and describe turns. The lib module is updated to expose the new pieces. Auto-committed-on: macbook Co-authored-by: Medulla <medulla@tinyhumans.ai>
The turn outcome test fixture now initializes the new unapplied_steers field so it matches the updated struct. Auto-committed-on: macbook Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add integration tests covering turn steering in the embed crate, exercising the steering flow end to end to guard against regressions. Auto-committed-on: macbook Co-authored-by: Medulla <medulla@tinyhumans.ai>
Reduce the slow provider delay in the cancelled-turn scenario from 30s to 4s so the test no longer stalls for half a minute while still exercising the same cancellation path. Auto-committed-on: macbook Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a README for the embed crate and its tests directory, and introduce a public API test that exercises the crate's exported surface. The docs explain how to run the tests and what the public API test covers. Auto-committed-on: macbook Co-authored-by: Medulla <medulla@tinyhumans.ai>
Auto-committed-on: macbook Co-authored-by: Medulla <medulla@tinyhumans.ai>
Reformatted the steering handle call and the unapplied-steers assertion so each fits on one line, matching rustfmt output. No test behaviour changed. Auto-committed-on: macbook Co-authored-by: Medulla <medulla@tinyhumans.ai>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: 🔵 Low · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 55.26% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 38 functions across 10 files. (4 skipped:… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely summarizes the main changes: steering a running turn and returning unapplied steers.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 55.26% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 38 functions across 10 files. (4 skipped: 4 unsupported.)
- Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
-
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
A rabbit queues a thought to share
It waits until the turn can hear
If some words stay unapplied,
They’re gathered safely at the side
Then off the rabbit hops, ears high.
Comment @coderabbitai help to get the list of available commands.
Tiny Sweeper reviewTiny Sweeper reviewed this change across 5 lane(s) and found 0 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below. State: Incomplete Review snapshot
Completeness: Incomplete FeaturesNone identified with supported citations. TestsNo supported feature-to-test mapping was produced. Test execution is not inferred. FindingsNo active actionable findings. Could not review: crates/openhuman-core/src/inference/host_runtime/ops/agent_chat.rs, crates/openhuman-embed/README.md, crates/openhuman-embed/src/lib.rs, crates/openhuman-embed/src/turn.rs, crates/openhuman-embed/src/turn_control.rs, crates/openhuman-embed/src/turn_steering.rs, crates/openhuman-embed/src/turn_steering_tests.rs, crates/openhuman-embed/src/turn_tests.rs, crates/openhuman-embed/src/turn_types.rs, crates/openhuman-embed/tests/README.md, crates/openhuman-embed/tests/public_api.rs, crates/openhuman-embed/tests/turn_steering.rs, docs/TEST-COVERAGE-MATRIX.md Before merge
How this fits togetherflowchart LR
n0["AgentChatEnv<br/>changed"]:::changed
n1["agent_chat_reply_for<br/>changed"]:::changed
n2["a_turn_outcome_carries_what_the_turn_spent<br/>changed"]:::changed
n3["send_inner"]:::impacted
n4["Result"]:::impacted
n5["agent_chat_for"]:::impacted
n6["dispatch"]:::impacted
n7["agent_chat_for_inner"]:::impacted
n8["as_ref"]:::impacted
n1 -->|uses| n0
n1 -->|uses| n4
n1 -->|calls| n8
n2 -->|calls| n8
n2 -->|tests| n8
n3 -->|uses| n0
n3 -->|uses| n4
n3 -->|calls| n8
n5 -->|uses| n0
n5 -->|uses| n4
n5 -->|calls| n7
n6 -->|calls| n1
n6 -->|uses| n4
n6 -->|calls| n5
n6 -->|calls| n8
n7 -->|uses| n0
n7 -->|calls| n1
n7 -->|uses| n4
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Agent review detailscritique
security
tests
commits
description
Evidence and run details
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![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: 62f892b747
ℹ️ 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".
| self.0.queue.push_now( | ||
| QueueLane::Steer, | ||
| QueuedTurn { | ||
| id: uuid::Uuid::new_v4().to_string(), | ||
| text, |
There was a problem hiding this comment.
When steering a normal (non-untrusted_input) embed turn with end-user text, this direct RunQueue push bypasses the prompt-injection enforcement that ordinary embed turns execute in agent_chat_reply_for (agent_chat.rs:392-405); the web queue path likewise calls enforce_prompt_input before enqueueing (start_chat.rs:223-232). A string that Turn::send rejects (as exercised by structured_turns.rs:27-29,293-299) is therefore accepted by steer() and reaches a tool-capable running agent. Apply the same core policy before queuing, while retaining the explicit host-only untrusted_input exception.
AGENTS.md reference: AGENTS.md:L53-L54
Useful? React with 👍 / 👎.
| async fn send_inner(mut self, usage: &UsageSink) -> Result<TurnOutcome, CoreError> { | ||
| // Declared first so it drops last, after the dispatch future: by then | ||
| // every steer the turn did not apply is back in the queue. | ||
| let steering = crate::turn_steering::SteeringScope::new(self.steering.take()); |
There was a problem hiding this comment.
When turn.stream() is dropped before its spawned task first polls (or any already-cancelled token enters send_controlled), the biased cancellation branch completes without polling send_inner, so this SteeringScope is never constructed. Dropping the unpolled send_inner future then merely drops the Turn's Option<TurnSteering>: the external handle remains finished == false, later steer() calls incorrectly return Ok, and take_unapplied() can never return the stranded text. Install an owner/drop guard before the cancellable select so pre-start cancellation closes and hands back the queue.
Useful? React with 👍 / 👎.
| @@ -1 +1 @@ | |||
| Subproject commit aa5476fa48092d9bf051fbd2d1bf07f14cc9f331 | |||
| Subproject commit b6b7f994a54f43a7b7bac106dec082d56c05fbef | |||
There was a problem hiding this comment.
The commit's Related section identifies b6b7f994a as PR #462's branch head and explicitly says it must be replaced after merge. Pinning this gitlink as-is makes OpenHuman depend on an unmerged branch object rather than an independently reviewable canonical upstream commit; merge #462 first and update the gitlink to the resulting upstream commit before landing this change.
AGENTS.md reference: AGENTS.md:L537-L545
Useful? React with 👍 / 👎.
| pub use session_store::{InMemorySessionStores, SessionStoreProvider}; | ||
| pub use turn::{absolute, Route, Turn, TurnOutcome, TurnRequest}; | ||
| pub use turn_cancellation::TurnCancellation; | ||
| pub use turn_steering::{SteerRefused, TurnSteering}; |
There was a problem hiding this comment.
This new public export changes the host-facing contract, but the generated api-index.md/api-index.json still omit SteerRefused and TurnSteering, and llms-full.txt still contains the old README. Checking the candidate tree with node scripts/generate-embed-docs.mjs --check while supplying the existing capability payload reports all three files as drift, so pnpm docs:check will reject the change and the published embed index hides the new API. Run the documentation generator and commit its outputs.
AGENTS.md reference: AGENTS.md:L771-L773
Useful? React with 👍 / 👎.
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
There was a problem hiding this comment.
tinysweeper found nothing blocking, but could not review everything, so this is not an approval: crates/openhuman-core/src/inference/host_runtime/ops/agent_chat.rs, crates/openhuman-embed/README.md, crates/openhuman-embed/src/lib.rs, crates/openhuman-embed/src/turn.rs, crates/openhuman-embed/src/turn_control.rs, crates/openhuman-embed/src/turn_steering.rs, crates/openhuman-embed/src/turn_steering_tests.rs, crates/openhuman-embed/src/turn_tests.rs and 5 more.
$0.0052 · 163,282 in / 14,733 out · 0 cached (0%) · ladder/vectors-oai3, glm-5.3-flash, · 726 embedded
security: $0.0005 · 56,384 in / 748 out · 0 cached (0%) · glm-5.3-flash,
tests: $0.0005 · 49,900 in / 510 out · 0 cached (0%) · glm-5.3-flash
description: $0.0041 · 56,998 in / 13,475 out · 0 cached (0%) · glm-5.3-flash
tinysweeper
Bot
added
the
priority: p3
1 participant
Summary
Turn::steering_handle()toopenhuman-embed. It returns aTurnSteeringhandle (Clone + Send + Sync) that adds user messages to a turn while the turn runs.AgentChatEnvgainsrun_queue.agent_chat_reply_forattaches it withOpenHumanSessionHost::set_run_queue, so the existing steering forwarder delivers queued steers into the running loop. Both embed turn targets get this: runtime-owned agents and the orchestrator of a caller-built runtime.TurnOutcome::unapplied_steers. After a failure or cancellation they come fromTurnSteering::take_unapplied(). A steer sent after the turn ended is refused withSteerRefused, which carries the text.vendor/tinyagentsto Requeue unapplied steers synchronously when a forwarder guard drops tinyagents#462.Problem
Embedders could cancel a turn but could not add to one. A message that arrived while a turn was running had to wait for it to finish and then run as a separate turn, often against a reply the model wrote without that message. Web chat already steers turns through the session run queue. The embed dispatch path (
agent_chat_for) never attached a queue.Solution
TurnSteeringowns aRunQueue<QueuedTurn>.steer()pushes onto the Steer lane. The core's forwarder injects each steer as[User steering message]: <text>, and the agent loop applies it at its next safe checkpoint:send();settle_natural_finishkeeps the draft as an assistant message, appends the steer and calls the model again, bounded by the iteration cap. So a steer sent during a plain chat turn's only model call still shapes that turn's reply.SteeringScopedeclared first inTurn::send_innerdrops after the dispatch future. Closing it and pushing a new steer take the same lock, so a steer is either refused or reported, never lost.Public API
Adding a field to
TurnOutcomebreaks struct literals outside the crate. No in-tree crate builds one outsideopenhuman-embed's own tests.Submission Checklist
crates/openhuman-embed/tests/turn_steering.rs(end to end against a wiremock provider): a steer sent during a plain chat turn's only model call follows the draft and earns a second call in the same turn; a steer queued before send reaches the first call; a steer after the end is refused with its text; a cancelled turn hands its delivered steer back.src/turn_steering_tests.rscovers ordering, closing, the failure path, a late requeue, and thatDebugleaves out message text.Validation run locally
cargo test -p openhuman-embed --test turn_steering: pass.cargo test -p openhuman-embed --lib turn: 34 passed.cargo clippy -p openhuman -p openhuman-embed --all-targets -- -D warnings: no findings in the changed files. The lint run itself fails on pre-existing unused imports in untouched files (for exampleagent/artifacts/schemas.rs).cargo fmt -p openhuman-embed -- --check: clean. Workspacecargo fmt --checkflags about 50 untouched core files that are already unformatted onmain.node scripts/ci/check-openhuman-rust-layout.mjs: pass.cargo test -p openhuman-embed --no-fail-fast: three suites fail locally,completion_routing(finish reasonlengthvsMAX_TOKENS),tool_hook_context(shellHOMEin the sandbox) andcron_host_delivery(job did not fire). None of them takes a steering handle, and without one the turn runs withrun_queue: None, which matchesmain. I treat them as environmental or pre-existing.Impact
Related
vendor/tinyagentsgitlink points at that PR's branch head (b6b7f994a). Do not merge this PR before fix(subconscious): seed defaults into per-user workspace + fix Intelligence page stale log #462. tinyagents merges with merge commits, so b6b7f994a will be on tinyagentsmainonce fix(subconscious): seed defaults into per-user workspace + fix Intelligence page stale log #462 lands. If fix(subconscious): seed defaults into per-user workspace + fix Intelligence page stale log #462 is squashed or rebased instead, the gitlink has to move to the merged commit first.Summary by CodeRabbit