feat(harness): host-agnostic context-ladder middleware and embedding tool ranker - #234
Conversation
Removed six middleware library modules that were no longer referenced anywhere in the codebase, cleaning up dead code and reducing compilation targets. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
When the wrap_up field is absent from the middleware configuration, the system now defaults to a no-op behavior instead of panicking. This change improves robustness by allowing configurations that omit optional wrap_up settings to function correctly. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
When the wrap_up middleware encounters a missing state, it now returns an error instead of panicking. This prevents crashes in edge cases where the state has not been properly initialized. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…ap up The argument recovery middleware now properly handles the repeat progress and wrap up middleware by restoring their arguments from the call stack. Previously, these middlewares were not being restored correctly, which could lead to incorrect state when replaying agent interactions. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Remove the credential_scrub and repeat_progress middleware modules from the harness library, as they are no longer needed for the current agent execution pipeline. This simplifies the middleware stack and reduces unnecessary processing overhead. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…ware types Make the credential scrub middleware configurable by extracting the per-tool scrub logic into a `ToolScrubber` callback, allowing hosts to protect host-minted fields in specific tool outputs while falling back to the default `scrub_with_notice` for everything else. Also expose all library middleware types and their key constants publicly so that external consumers can import and compose them without relying on internal module paths. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Introduces an embedding ranker module that scores and ranks discovered tools by semantic similarity to a query. This enables more relevant tool selection during agent planning by leveraging vector embeddings rather than relying solely on keyword matching. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The embedding ranker now returns an empty result when given an empty list of embeddings, instead of panicking or producing undefined behavior. This ensures the discovery tool can gracefully handle cases where no embeddings are available for ranking. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…/arg_recovery_test.rs,crates/ti Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…al scrub coverage Moved the middleware test files from a single monolithic test module into individual test modules declared in mod.rs, making the test structure consistent with the rest of the crate. Replaced the placeholder credential scrub integration test with proper tests that exercise the middleware stack end-to-end, including verification that the scrubber annotation appears on scrubbed results, clean results are left untouched, and a host-supplied scrubber receives the tool name. Also cleaned up trailing blank lines and reformatted a few assertions for consistency. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…ddleware Add three new test modules to verify the behavior of the arg recovery, artifact table of contents, and wrap up middleware components. These tests ensure the middleware functions correctly in isolation and cover edge cases for each component. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Introduce a new test module for the artifact table of contents middleware to verify its behavior and ensure correctness in handling artifact metadata. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The test was previously checking the wrong variable, comparing the trimmed image against the original instead of the expected result. This change fixes the assertion to validate the correct output, ensuring the test properly verifies the trimming behavior. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Replace the manual ceiling division pattern `(n + d - 1) / d` with the standard `div_ceil` method, and convert nested `if let` chains to the more idiomatic let-chain syntax. These changes improve readability and align the codebase with modern Rust conventions without altering any runtime behaviour. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
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. |
![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: 0f99737565
ℹ️ 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".
Tiny Sweeper review
Last completed reportTiny Sweeper reviewTiny Sweeper reviewed this change across 6 lane(s) and found 1 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below. State: Incomplete Review snapshot
Completeness: Incomplete What changedThe review could not produce a supported behavioral summary; inspect the cited changed surface and lane details below. FeaturesNone identified with supported citations. TestsNo supported feature-to-test mapping was produced. Test execution is not inferred. Findings
Could not review: crates/tinyagents-harness/Cargo.toml, crates/tinyagents-harness/src/middleware/library/arg_recovery.rs, crates/tinyagents-harness/src/middleware/library/arg_recovery_test.rs, crates/tinyagents-harness/src/middleware/library/artifact_toc.rs, crates/tinyagents-harness/src/middleware/library/artifact_toc_test.rs, crates/tinyagents-harness/src/middleware/library/credential_scrub.rs, crates/tinyagents-harness/src/middleware/library/credential_scrub_test.rs, crates/tinyagents-harness/src/middleware/library/image_trim.rs, crates/tinyagents-harness/src/middleware/library/image_trim_test.rs, crates/tinyagents-harness/src/middleware/library/mod.rs, crates/tinyagents-harness/src/middleware/library/repeat_progress.rs, crates/tinyagents-harness/src/middleware/library/repeat_progress_test.rs, crates/tinyagents-harness/src/middleware/library/wrap_up.rs, crates/tinyagents-harness/src/middleware/library/wrap_up_test.rs, crates/tinyagents-harness/src/tool/discover/README.md, crates/tinyagents-harness/src/tool/discover/embedding_ranker.rs, crates/tinyagents-harness/src/tool/discover/embedding_ranker_test.rs, crates/tinyagents-harness/src/tool/discover/mod.rs Before merge
Agent review detailscritique
security
tests
commits
description
e2e
Evidence and run details
|
…harge tokens for structured con The credential scrub middleware now iterates over every content block in a tool result instead of only the first text block, applying redactions to both text and JSON blocks. A redaction notice is appended to the result rather than replacing the entire content, preserving other blocks. The image trim middleware's token estimator now accounts for JSON, thinking, and provider extension blocks so that large structured tool results cannot bypass the prompt budget. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Previously, the artifact table-of-contents middleware silently skipped entries when the underlying store returned an error, making it impossible to diagnose storage failures. The change now logs a warning with the affected key and error details before continuing, so operators can detect and investigate persistent store issues without halting the middleware. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…nd token estimation The credential scrub middleware now skips image and file content blocks instead of panicking when encountering them, and the image trim middleware's token estimator accounts for audio, video, and document blocks by adding a fixed token cost, matching the existing treatment of image blocks. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a counter for failed store reads during artifact index table construction and log a warning when every key fails to load, producing an empty table. This helps operators distinguish between a genuinely empty index and one that is unreachable due to storage errors. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Replace the single atomic boolean with a mutex-guarded set of instance IDs so that the `fired` state is correctly scoped to each run context rather than shared across all contexts. This fixes a race condition where concurrent runs could incorrectly observe the wrap-up as having fired for the wrong context. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…eware The repeat-progress middleware previously used single global state for tracking successful repeats, pending call batches, and recorded tool results. This change replaces those singletons with per-run-id maps, allowing multiple agent runs to execute concurrently without interfering with each other's repeat detection and eviction logic. A new `after_agent` hook cleans up per-run state when a run completes. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The test helper `ctx()` now assigns a non-zero instance id to the run context, matching the behaviour of the agent loop where hook-level fixtures reuse the same logical invocation across phases. This ensures the repeat-progress middleware sees a realistic context during testing. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Replace nested if-let statements with Rust's let-chain syntax to reduce indentation and improve readability. The change applies to two locations in the repeat progress middleware where a guard condition and a lock acquisition were previously nested, and now use the `&&` operator within a single if-let expression. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f186c59373
ℹ️ 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".
| fn halt(&self, summary: String) { | ||
| if let Ok(mut slot) = self.halt_summary.lock() { | ||
| *slot = Some(summary); | ||
| } | ||
| self.handle.send(SteeringCommand::Pause); |
There was a problem hiding this comment.
When one shared middleware instance serves concurrent runs, halt still writes to a single HaltSummarySlot and sends a root-targeted pause through one stored SteeringHandle; whichever run reaches a steering checkpoint first can consume the pause, while another halt can overwrite the summary. Fresh evidence after the earlier tracker isolation fix is that these two control outputs remain instance-global rather than keyed or targeted by ctx.instance_id(), so the run that actually repeated may continue while an unrelated run pauses.
Useful? React with 👍 / 👎.
| if let Some((scrubbed, count)) = (self.scrubber)(&tool_name, &text) { | ||
| if is_json { | ||
| if let Ok(value) = serde_json::from_str(&scrubbed) { | ||
| if let tinytools::ToolContent::Json { data } = block { | ||
| *data = value; | ||
| } | ||
| } else if let tinytools::ToolContent::Json { data } = block { | ||
| *data = serde_json::Value::String(scrubbed); |
There was a problem hiding this comment.
For a ToolContent::Json block containing a credential, the default scrub_with_notice appends human-readable notice text to the serialized JSON, so serde_json::from_str(&scrubbed) necessarily fails and this fallback converts the entire object into Value::String. Fresh evidence after the prior preservation fix is that its default scrubber output cannot take the successful parse branch; scrub the JSON value without appending prose, then add the notice as the separate text block already emitted below.
Useful? React with 👍 / 👎.
| ContentBlock::Image(_) | ||
| | ContentBlock::Audio(_) | ||
| | ContentBlock::Video(_) | ||
| | ContentBlock::Document(_) => { | ||
| total = total.saturating_add(IMAGE_MARKER_TOKEN_COST); |
There was a problem hiding this comment.
Every native ContentBlock::Image is already charged by count_native_image_blocks above, but this match charges it another IMAGE_MARKER_TOKEN_COST. Requests containing native images are therefore estimated at 2,400 tokens per image rather than the documented 1,200, which can evict otherwise fitting conversation history; keep this second charge only for audio, video, and document blocks.
Useful? React with 👍 / 👎.
| let absolute_idx = removable_positions.remove(0); | ||
| // Subsequent positions shift left by one for every prior removal. | ||
| let remove_at = absolute_idx - removed; | ||
| messages.remove(remove_at); |
There was a problem hiding this comment.
When the system messages plus the newest user request exceed the budget, this loop eventually removes every non-system message, including the request the model is supposed to answer, and then sends a system-only request—possibly still over budget. This occurs with a large current prompt or an unshrinkable system prefix; treat the newest user turn as non-evictable and surface/truncate an oversized current input instead of silently producing an unrelated answer.
Useful? React with 👍 / 👎.
| pub use arg_recovery::ArgRecoveryMiddleware; | ||
| pub use artifact_toc::{ | ||
| ARTIFACT_INDEX_NAMESPACE, ArtifactIndexTocMiddleware, FOOTER_ALLOWANCE, NO_WINDOW_ALLOWANCE, | ||
| split_input_allowance, | ||
| }; |
There was a problem hiding this comment.
These exports add a substantial public middleware surface, but middleware/library/README.md still lists only the pre-existing middleware and files, and no harness module design document is updated with the ordering and operational constraints described only in source comments. Update the module README/design docs so the repository's documented public catalog and usage remain aligned with these APIs.
AGENTS.md reference: AGENTS.md:L78-L82
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/tinyagents-harness/Cargo.toml, crates/tinyagents-harness/src/middleware/library/arg_recovery.rs, crates/tinyagents-harness/src/middleware/library/arg_recovery_test.rs, crates/tinyagents-harness/src/middleware/library/artifact_toc.rs, crates/tinyagents-harness/src/middleware/library/artifact_toc_test.rs, crates/tinyagents-harness/src/middleware/library/credential_scrub.rs, crates/tinyagents-harness/src/middleware/library/credential_scrub_test.rs, crates/tinyagents-harness/src/middleware/library/image_trim.rs and 10 more.
$0.0174 · 178,309 in / 9,914 out · 5,632 cached (3%) · ladder/vectors, deepseek/deepseek-v4-flash · 1,142 embedded
tests: $0.0063 · 63,746 in / 4,273 out · 2,816 cached (4%) · deepseek/deepseek-v4-flash
description: $0.0052 · 54,960 in / 2,326 out · 2,304 cached (4%) · deepseek/deepseek-v4-flash
tinysweeper
Bot
added
the
priority: p2
1 participant
Summary
Moves generic middleware and the embedding tool ranker out of OpenHuman core into the harness. Stacked on
oh-extract-leaves(#230); branched at the OpenHuman pin (an ancestor of that branch tip).middleware::library(all generic over the run-context payloadC, none read it):ImageAwareMessageTrimMiddlewareplusestimate_text_tokens/estimate_message_tokens/legacy_max_input_tokens(flat per-image cost, system messages never evicted, order kept, orphaned leading tool results dropped)ArtifactIndexTocMiddlewareandsplit_input_allowanceFinalCallWrapUpMiddlewareRepeatProgressMiddlewareandRepeatEvictionObserver(host adapter over the existingSuccessfulRepeatTracker)ArgRecoveryMiddlewareCredentialScrubMiddleware(usestinyinference_core::sanitize::scrub_credentials; new dependency ontinyinference-core)tool::discover::EmbeddingToolRanker: semanticToolRankerover anyEmbeddingModel, cached in memory and optionally on disk keyed by the embedding-space signature.Seams (host policy stays a constructor argument)
DEFAULT_CLEARED_PLACEHOLDER([Old tool result content cleared]) withwith_cleared_placeholderFinalCallWrapUpMiddleware::with_deliverable_tools(empty by default, which leaves the call ordinary)CapturedOutcomestraitRepeatExemptionclosureArtifactIndexTocMiddleware::new(budget, store_name)ToolScrubber(defaultscrub_with_notice)The existing
TrimStrategy/MicrocompactMiddlewareandtoken_estimationare unchanged: the image-marker-aware estimate has different constants and semantics fromtoken_estimation, so it lives beside the trim that needs it.Tests
cargo test -p tinyagents-harness: 1496 lib tests pass, plus 43 others.cargo clippy -p tinyagents-harness --all-targets -- -D warningsandcargo fmt --all -- --checkare clean.