feat(harness): ModelSummarizer + fault-tolerant summarizer and thread-title helpers from OpenHuman - #230
Conversation
Updated the vendored dependencies for tinyinference and tinytools to their latest versions, incorporating upstream fixes and improvements. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Refactor the summarization module to integrate a resilient wrapper around model summarization, improving fault tolerance during inference. The change introduces a new resilient layer that retries on transient failures and adds corresponding test coverage, while updating dependency locks to match. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…ilient retry Introduces a new summarization module that provides a model summarizer with resilient retry logic, enabling robust text summarization capabilities within the harness. The module includes a test file to verify the summarizer's behavior. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…agents-harness/src/title/mod.rs Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The model summarizer test was incorrectly using a direct summarizer instead of the resilient wrapper, which caused test failures when the underlying service experienced transient errors. Updated the test to instantiate the resilient summarizer to match the production behavior. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Updated the vendored tinytools dependency to incorporate upstream fixes and improvements. This change ensures the project uses the latest stable version of the library. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Tiny Sweeper reviewTiny Sweeper reviewed this change across 6 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 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. FindingsNo active actionable findings. Could not review: crates/tinyagents-harness/src/summarization/mod.rs, crates/tinyagents-harness/src/summarization/model_summarizer.rs, crates/tinyagents-harness/src/summarization/model_summarizer_test.rs, crates/tinyagents-harness/src/summarization/resilient.rs, crates/tinyagents-harness/src/summarization/types.rs Before merge
Agent review detailscritique
security
tests
commits
description
e2e
Evidence and run details
|
|
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ed9cc77997
ℹ️ 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".
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/src/lib.rs, crates/tinyagents-harness/src/summarization/mod.rs, crates/tinyagents-harness/src/summarization/model_summarizer.rs, crates/tinyagents-harness/src/summarization/model_summarizer_test.rs, crates/tinyagents-harness/src/summarization/resilient.rs, crates/tinyagents-harness/src/title/mod.rs, crates/tinyagents-harness/src/title/test.rs, tinysweeper/tests.
$0.0025 · 39,316 in / 4,013 out · 0 cached (0%) · ladder/vectors, deepseek/deepseek-v4-flash · 1,023 embedded
description: $0.0003 · 21,110 in / 78 out · 0 cached (0%) · deepseek/deepseek-v4-flash
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: ab5287d8db
ℹ️ 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".
| let remaining = previous_token_estimate + estimate_slice_tokens(&messages[start..]); | ||
| if remaining <= self.fallback_trim_budget || start + 1 >= messages.len() { | ||
| break; |
There was a problem hiding this comment.
Enforce the deterministic fallback budget
When the prior checkpoint already exceeds fallback_trim_budget, or the final remaining message is itself oversized, start + 1 >= messages.len() exits this loop while remaining is still over budget. The fallback then emits the entire previous summary and final message, so a summarizer outage can leave the retried model request above its context window and fail the turn this adapter is intended to preserve; truncate or discard over-budget content, including the previous checkpoint, before returning.
Useful? React with 👍 / 👎.
| pub use model_summarizer::{ | ||
| DEFAULT_SUMMARIZE_KEEP_LAST, DEFAULT_SUMMARIZE_THRESHOLD_FRACTION, summarization_policy, | ||
| summarization_policy_with, | ||
| }; |
There was a problem hiding this comment.
Document the new summarization surface
These new public policy builders, along with ModelSummarizer and FaultTolerantCachingSummarizer, are absent from summarization/README.md: its public-surface section still describes only ConcatSummarizer, and its file table omits both new implementation files. Update the module documentation so consumers can discover the intended construction, caching, and failure behavior, as required for public API changes.
AGENTS.md reference: AGENTS.md:L78-L82
Useful? React with 👍 / 👎.
| #[cfg(test)] | ||
| mod model_summarizer_test; |
There was a problem hiding this comment.
Keep module-local tests in test.rs
This registers unit tests from the new sibling model_summarizer_test.rs, although the repository requires module-local unit tests to live in a dedicated test.rs. Move these cases into the existing summarization test.rs, or make model_summarizer a module directory containing its own test.rs, so the new feature follows the required module layout.
AGENTS.md reference: AGENTS.md:L18-L20
Useful? React with 👍 / 👎.
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/src/summarization/mod.rs, crates/tinyagents-harness/src/summarization/model_summarizer.rs, crates/tinyagents-harness/src/summarization/model_summarizer_test.rs, crates/tinyagents-harness/src/summarization/resilient.rs, crates/tinyagents-harness/src/summarization/types.rs.
$0.0089 · 118,586 in / 16,707 out · 48,128 cached (41%) · ladder/vectors, deepseek/deepseek-v4-flash · 1,056 embedded
tests: $0.0054 · 58,915 in / 5,158 out · 27,648 cached (47%) · deepseek/deepseek-v4-flash
description: $0.0026 · 19,551 in / 4,807 out · 0 cached (0%) · deepseek/deepseek-v4-flash
Summary
tinyagents-harness::summarization:ModelSummarizer(LLM-backedSummarizer),FaultTolerantCachingSummarizer(per-turn circuit breaker, deterministic-trim fallback, single-slot content-hash cache) andsummarization_policy/summarization_policy_with. The 0.90 threshold and keep-last-8 are now parameters (DEFAULT_SUMMARIZE_THRESHOLD_FRACTION,DEFAULT_SUMMARIZE_KEEP_LASTare the defaults), with tests usingScriptedModel.tinyagents-harness::title: thread-title shaping (shorten_title,sanitize_generated_title,title_from_user_message, placeholder detection) andbuild_title_request, moved with their 30+ tests from OpenHuman'sthreads/title.rs. The[threads:title]log prefix stays in the host.vendor/tinytools(newtinytools-std: feat(tinytools-std): file_state, url_guard, detect_tools from OpenHuman; parser regression tests tinytools#33) andvendor/tinyinference(scrub_credentials: feat(core): scrub_credentials in sanitize (from OpenHuman) tinyinference#40).config::required_outputalready existed here; OpenHuman's duplicate copy is being deleted host-side.Stacking
Base is
oh-dedupe-time-goals-tools(#229, itself stacked on #228). The two nested submodule PRs above are likewise stacked on their pinned branches; merge them first, innermost first.Verification
cargo fmt --check,cargo clippy -p tinyagents-harness --all-targets -- -D warnings,cargo test -p tinyagents-harness(1439 lib tests passed).Co-authored-by: Medulla medulla@tinyhumans.ai