feat(harness): no-progress escalation primitive (NoProgressTracker)#7
Conversation
Adds `harness::no_progress`, a reusable harness-agnostic detector that breaks the "retry the same failing tool call" loop. It tracks recent (tool, args, error) outcomes across a turn and returns Continue / Nudge / Halt. The escalation ladder caps same-strategy retries before halting: an identical repeated failure first nudges the model with a structured "no progress since step X" signal so it changes approach, and only halts if it keeps re-issuing the same failing call. A varied-failure backstop catches "different commands all failing", a hard policy-reject rung trips fastest, and any success resets the ladder. Pure state machine (no harness types) so it unit-tests in isolation and is reusable by higher-level reliability layers; a driver (typically a middleware) feeds outcomes into `record` and maps the verdict onto steering (nudge) or a halt. Re-exported from `harness`; additive only. Extracted from OpenHuman (tinyhumansai/openhuman#4389) as part of converging generic harness mechanics into this crate. 7 unit tests cover: nudge->halt on identical repeats, success reset, changed-args clears the streak, hard-reject halt, varied-failure nudge->backstop, unknown-tool exclusion, and the halt-threshold clamp.
📝 WalkthroughWalkthroughAdds a new ChangesNo-progress tracker feature
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant NoProgressTracker
participant LadderState
Caller->>NoProgressTracker: record(step, attempt)
NoProgressTracker->>LadderState: update streak counters
alt success
NoProgressTracker->>LadderState: reset()
NoProgressTracker-->>Caller: Continue
else hard_reject or identical retries exhausted
NoProgressTracker->>LadderState: reset()
NoProgressTracker-->>Caller: Halt(message)
else identical cap reached or varied failures accumulate
NoProgressTracker-->>Caller: Nudge(message)
else
NoProgressTracker-->>Caller: Continue
end
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/harness/mod.rs (1)
27-27: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider making
no_progressprivate to keep a single public path.
pub mod no_progresscombined with the flatpub usere-export exposes the same types via two public paths (harness::no_progress::NoProgressandharness::NoProgress). Since the intended surface is the flat re-export, droppingpubon the module keeps the API to the smallest useful set.As per coding guidelines: "The module root should wire the pieces together and expose the smallest useful API."
♻️ Proposed change
-pub mod no_progress; +mod no_progress;Also applies to: 52-54
🤖 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/harness/mod.rs` at line 27, The `no_progress` module is being exposed twice, once through `harness::no_progress` and again via the flat re-export, so reduce the public API surface by making `no_progress` private in `harness::mod` and keeping the existing `NoProgress` re-export as the single public entry point. Update the module declaration and preserve the current re-export wiring so callers continue using `harness::NoProgress` while `no_progress` remains internal.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@src/harness/mod.rs`:
- Line 27: The `no_progress` module is being exposed twice, once through
`harness::no_progress` and again via the flat re-export, so reduce the public
API surface by making `no_progress` private in `harness::mod` and keeping the
existing `NoProgress` re-export as the single public entry point. Update the
module declaration and preserve the current re-export wiring so callers continue
using `harness::NoProgress` while `no_progress` remains internal.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cefa8524-f71a-4e1e-be77-e7aa18e6789b
📒 Files selected for processing (2)
src/harness/mod.rssrc/harness/no_progress.rs
Summary
harness::no_progress— a reusable, harness-agnostic detector that breaks the "retry the same failing tool call" loop. It tracks recent(tool, args, error)outcomes across a turn and returns aContinue/Nudge/Haltverdict.NoProgressTracker::recordand maps the verdict onto steering (nudge) or a halt.Context
Extracted from OpenHuman (tinyhumansai/openhuman#4389) as part of converging generic harness mechanics into this crate. After the OpenHuman →
tinyagentsharness migration, loop-control behavior like this belongs here so every crate consumer gets it; the app-specific driver (wiring the verdict onto steering) and policy stay in the consumer.Public API
NoProgressTracker::new(identical_halt_threshold)/record(step, &ToolAttempt) -> NoProgress/reset()ToolAttempt<'a>—{ tool, arg_fingerprint, error, hard_reject, recoverable_miss }enum NoProgress { Continue, Nudge(String), Halt(String) }DEFAULT_IDENTICAL_HALT_THRESHOLDRe-exported from
harness. Additive only — no existing APIs change.Tests
7 unit tests: nudge→halt on identical repeats, a success resets the ladder, changed-args clears the identical streak, hard-reject halts on the 2nd repeat, varied-failure nudge→backstop, unknown-tool recovery excluded from the backstop, and the halt-threshold clamp (nudge always precedes halt).
Summary by CodeRabbit
New Features
Bug Fixes