Repository navigation
feat(no_progress): volatility-aware outcome fingerprinting for repeat detection #304
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
e972aa3
e4be3b0
9d8fdf9
e1b2924
a5706f1
e7ebab3
5f835c9
be7c05d
f379aaa
d3b535d
111f1cc
11cbf79
e630b07
ac0a498
c8c139c
a6f3bb4
477f5d2
436d608
2ca7e7e
f414e30
4e64e6c
135b789
555f09c
d103154
073105c
7f15159
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -11,7 +11,10 @@ use async_trait::async_trait; | |||||
| use crate::context::RunContext; | ||||||
| use crate::error::Result as TaResult; | ||||||
| use crate::middleware::{Middleware, ToolInvocationIdentity}; | ||||||
| use crate::no_progress::{SuccessfulRepeat, SuccessfulRepeatTracker, fingerprint_arguments}; | ||||||
| use crate::no_progress::{ | ||||||
| OutcomeFingerprinter, SuccessfulRepeat, SuccessfulRepeatTracker, VolatileSpanNormalizer, | ||||||
| fingerprint_arguments, | ||||||
| }; | ||||||
| use crate::steering::{SteeringCommand, SteeringHandle}; | ||||||
| use tinyinference_llm::message::{ContentBlock, Message}; | ||||||
| use tinyinference_llm::model::{ModelRequest, ModelResponse}; | ||||||
|
|
@@ -143,6 +146,8 @@ pub struct RepeatProgressMiddleware { | |||||
| halt_summary: HaltSummarySlot, | ||||||
| exempt: RepeatExemption, | ||||||
| state: Arc<RepeatState>, | ||||||
| /// Reduces a tool result to the identity the recurrence ledger keys on. | ||||||
| fingerprinter: Arc<dyn OutcomeFingerprinter>, | ||||||
| /// Batch bookkeeping bridging `after_model` → `after_tool` for the call guard. | ||||||
| pending: Mutex<HashMap<u64, PendingCallBatch>>, | ||||||
| } | ||||||
|
|
@@ -161,10 +166,20 @@ impl RepeatProgressMiddleware { | |||||
| halt_summary, | ||||||
| exempt, | ||||||
| state: Arc::new(RepeatState::new(DEFAULT_CLEARED_PLACEHOLDER)), | ||||||
| fingerprinter: Arc::new(VolatileSpanNormalizer), | ||||||
| pending: Mutex::default(), | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| /// Replaces the fingerprinter the recurrence ledger uses to compare tool | ||||||
| /// results. The default ignores volatile spans (timestamps, durations, | ||||||
| /// request ids), so a result that differs only by those still counts as | ||||||
| /// the same result. | ||||||
| pub fn with_fingerprinter(mut self, fingerprinter: Arc<dyn OutcomeFingerprinter>) -> Self { | ||||||
| self.fingerprinter = fingerprinter; | ||||||
| self | ||||||
| } | ||||||
|
|
||||||
| /// Override the placeholder body treated as an evicted tool result. Must be | ||||||
| /// called before [`eviction_observer`](Self::eviction_observer). | ||||||
| pub fn with_cleared_placeholder(mut self, placeholder: impl Into<String>) -> Self { | ||||||
|
|
@@ -340,6 +355,8 @@ impl<C: Send + Sync> Middleware<(), C> for RepeatProgressMiddleware { | |||||
| ) -> TaResult<()> { | ||||||
| let tool_name = invocation.tool_name(); | ||||||
| let call_id = invocation.call_id().to_string(); | ||||||
| // Fingerprint outside the mutexes below: it scans the whole result. | ||||||
| let identity = (!result.is_error).then(|| self.fingerprinter.fingerprint(&result.output())); | ||||||
|
senamakel marked this conversation as resolved.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fingerprint error results instead of collapsing them to empty Error results still bypass the fingerprinter and are recorded under the empty identity (
Suggested change
[RULE] error-collapsed-identity · |
||||||
| // Fold this result into the pending batch; the call guard only acts once | ||||||
| // the batch is complete so it sees whole-batch success. | ||||||
| let (already_halted, recurrence, completed) = { | ||||||
|
|
@@ -362,7 +379,7 @@ impl<C: Send + Sync> Middleware<(), C> for RepeatProgressMiddleware { | |||||
| recurrence = trackers | ||||||
| .entry(ctx.instance_id()) | ||||||
| .or_default() | ||||||
| .record_call_outcome(&sig, &result.output()); | ||||||
| .record_call_identity(&sig, identity.as_deref().unwrap_or_default()); | ||||||
| } | ||||||
| if let Ok(mut recorded) = self.state.recorded.lock() { | ||||||
| recorded | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep attempt-label capitalization identical
Exporting
VolatileSpanNormalizerexposes the default fingerprinting behavior, whose case-insensitive attempt rule replaces labels such asAttempt 1andattempt 1with the same<attempt>placeholder. Those can be distinct tool outcomes, so the no-progress tracker can incorrectly classify progress as a repeat. Preserve the original capitalization when normalizing the attempt label, or otherwise include it in the fingerprint.[RULE] identity-collision ·