From 810222b1c2ae5a58d45f6dbeabe62d8221097bcd Mon Sep 17 00:00:00 2001 From: Lakshman Patel Date: Mon, 24 Aug 2026 14:58:30 +0530 Subject: [PATCH] feat(engine): add tool-result clearing and approval pause timing --- README.md | 2 + internal/engine/compact_clear.go | 131 ++++++++++++++++++ internal/engine/compact_clear_test.go | 79 +++++++++++ internal/engine/context_governor.go | 5 + internal/permissions/approval_workflow.go | 8 ++ .../permissions/approval_workflow_test.go | 21 +++ 6 files changed, 246 insertions(+) create mode 100644 internal/engine/compact_clear.go create mode 100644 internal/engine/compact_clear_test.go diff --git a/README.md b/README.md index 460432f7..193f8b80 100644 --- a/README.md +++ b/README.md @@ -295,6 +295,8 @@ Features adopted from open-source agent projects. All are off by default unless | Smart turn routing | `internal/smartrouting` | Deterministic simple/strong turn classifier with fail-toward-strong safety. Wired into per-turn model selection (`settings.smart_routing`) | | Conversation arc | `internal/conversationarc` | Durable sidecar memory of goals/decisions/milestones/phase with a byte-stable summary. Wired into sessions (loaded on open, saved on close, injected into the system prompt) | | Relevance pruning | `internal/relevanceprune` | Token-budgeted context pruning preserving recent turns/tool calls/errors. Wired into compaction as a `relevance` strategy | +| Tool-result clearing | `internal/engine` (`ClearOldToolResults`) | Two-tier context management: at 80% of the context window, stale tool-result content is replaced with `[output cleared]` placeholders (tool_use kept intact) before compacting — a gentler tier below compaction | +| Approval pause timing | `internal/permissions` | Approval requests record decision timestamp + human deliberation duration (`DecisionAt`/`PauseDuration`) for approval-latency observability | ## Usage diff --git a/internal/engine/compact_clear.go b/internal/engine/compact_clear.go new file mode 100644 index 00000000..7d4fee3f --- /dev/null +++ b/internal/engine/compact_clear.go @@ -0,0 +1,131 @@ +package engine + +import ( + "context" + "sort" + + "github.com/GrayCodeAI/hawk/internal/types" +) + +// Two-tier context management (adopted from herm): before compacting, "clear" +// the content of old tool results into a short placeholder to reclaim context +// gently. The tool_use blocks are left intact, so the model still knows what was +// called, and can re-read files if needed. Only when clearing is insufficient +// does the compaction strategy chain run. + +const ( + // clearThresholdFraction is the fraction of the context window at which old + // tool results start getting cleared. 0.8 = clear when input tokens > 80%. + clearThresholdFraction = 0.8 + // clearKeepRecent is the number of most-recent tool-result messages kept intact. + clearKeepRecent = 4 + // outputClearedPlaceholder replaces cleared tool-result content. + outputClearedPlaceholder = "[output cleared]" +) + +// clearOldToolResults returns a copy of msgs with the content of old tool +// results replaced by a short placeholder, biggest-first, keeping the most +// recent clearKeepRecent tool-result messages intact, until estimatedTokens +// drops below threshold. It returns the new messages and the estimated tokens +// freed. Adopted from herm's clearOldToolResults. +func clearOldToolResults(msgs []types.EyrieMessage, estimatedTokens, threshold int) ([]types.EyrieMessage, int) { + if threshold <= 0 || estimatedTokens <= 0 || len(msgs) == 0 { + return msgs, 0 + } + type candidate struct { + idx int + size int + } + var candidates []candidate + for i := range msgs { + if size := toolResultBytes(msgs[i]); size > 0 { + candidates = append(candidates, candidate{idx: i, size: size}) + } + } + if len(candidates) <= clearKeepRecent { + return msgs, 0 + } + clearable := candidates[:len(candidates)-clearKeepRecent] + sort.Slice(clearable, func(i, j int) bool { return clearable[i].size > clearable[j].size }) + + out := cloneToolResults(msgs) + tokens := estimatedTokens + freed := 0 + for _, c := range clearable { + if tokens < threshold { + break + } + f := clearMessageToolResults(&out[c.idx]) + if f == 0 { + continue + } + tokens -= f + freed += f + } + return out, freed +} + +// ClearOldToolResults is the session-level entry point: it clears old tool +// results when token usage crosses 80% of the context window. Returns true when +// anything was cleared. It is a gentler tier below compaction. +func (s *Session) ClearOldToolResults(ctx context.Context) bool { + if s == nil { + return false + } + raw := s.Persistence().RawMessages() + if len(raw) == 0 { + return false + } + window := s.ContextWindowSize() + if window <= 0 { + return false + } + tokens := EstimateTokens(raw) + threshold := int(float64(window) * clearThresholdFraction) + if tokens < threshold { + return false + } + cleared, freed := clearOldToolResults(raw, tokens, threshold) + if freed > 0 { + s.Persistence().SetRawMessages(cleared) + return true + } + return false +} + +// toolResultBytes returns the total byte size of a message's tool results. +func toolResultBytes(m types.EyrieMessage) int { + total := 0 + for _, tr := range m.ToolResults { + total += len(tr.Content) + } + return total +} + +// clearMessageToolResults replaces each non-empty tool-result content with the +// placeholder and returns the estimated tokens freed (~4 bytes per token). +func clearMessageToolResults(m *types.EyrieMessage) int { + freedBytes := 0 + for i := range m.ToolResults { + c := m.ToolResults[i].Content + if c == "" || c == outputClearedPlaceholder { + continue + } + freedBytes += len(c) - len(outputClearedPlaceholder) + m.ToolResults[i].Content = outputClearedPlaceholder + } + return freedBytes / 4 +} + +// cloneToolResults copies msgs, deep-copying the ToolResults slices so callers +// can mutate the copy without touching the persisted originals. +func cloneToolResults(msgs []types.EyrieMessage) []types.EyrieMessage { + out := make([]types.EyrieMessage, len(msgs)) + for i, m := range msgs { + out[i] = m + if m.ToolResults != nil { + out[i].ToolResults = append([]types.ToolResult(nil), m.ToolResults...) + } + } + return out +} diff --git a/internal/engine/compact_clear_test.go b/internal/engine/compact_clear_test.go new file mode 100644 index 00000000..86b91cc3 --- /dev/null +++ b/internal/engine/compact_clear_test.go @@ -0,0 +1,79 @@ +package engine + +import ( + "testing" + + "github.com/GrayCodeAI/hawk/internal/types" +) + +func mkMsg(role string, trs []types.ToolResult) types.EyrieMessage { + return types.EyrieMessage{Role: role, ToolResults: trs} +} + +func TestClearOldToolResultsNoOpWhenBelowThreshold(t *testing.T) { + msgs := []types.EyrieMessage{ + mkMsg("user", []types.ToolResult{{Content: "aaaa"}}), + } + out, freed := clearOldToolResults(msgs, 100, 1000) + if freed != 0 || len(out) != 1 { + t.Fatalf("freed=%d len=%d, want no-op", freed, len(out)) + } +} + +func TestClearOldToolResultsClearsBiggestFirst(t *testing.T) { + // 6 tool-result messages; threshold low so clearing triggers; keep last 4. + msgs := make([]types.EyrieMessage, 6) + for i := range msgs { + msgs[i] = mkMsg("user", []types.ToolResult{{Content: "12345678"}}) // 8 bytes + } + out, freed := clearOldToolResults(msgs, 100000, 50000) + if freed == 0 { + t.Fatal("expected tokens freed") + } + // Keep last 4 intact; first 2 clearable messages get cleared. + if out[0].ToolResults[0].Content != outputClearedPlaceholder { + t.Fatalf("oldest result not cleared: %q", out[0].ToolResults[0].Content) + } + // Most recent 4 intact. + if out[5].ToolResults[0].Content == outputClearedPlaceholder { + t.Fatal("most recent result should be kept intact") + } + // Original slice untouched (we cloned). + if msgs[0].ToolResults[0].Content == outputClearedPlaceholder { + t.Fatal("input slice must not be mutated") + } +} + +func TestClearOldToolResultsStopsWhenUnderThreshold(t *testing.T) { + // Huge contents, small threshold: it clears until under threshold, then stops. + msgs := make([]types.EyrieMessage, 6) + for i := range msgs { + msgs[i] = mkMsg("user", []types.ToolResult{{Content: "12345678"}}) + } + out, _ := clearOldToolResults(msgs, 48, 20) + cleared := 0 + for i := 0; i < 2; i++ { + if out[i].ToolResults[0].Content == outputClearedPlaceholder { + cleared++ + } + } + if cleared == 0 { + t.Fatalf("expected at least the oldest result cleared: %+v", out[0]) + } +} + +func TestClearOldToolResultsSkipsAlreadyCleared(t *testing.T) { + msgs := []types.EyrieMessage{ + mkMsg("user", []types.ToolResult{{Content: outputClearedPlaceholder}}), + mkMsg("user", []types.ToolResult{{Content: outputClearedPlaceholder}}), + mkMsg("user", []types.ToolResult{{Content: outputClearedPlaceholder}}), + mkMsg("user", []types.ToolResult{{Content: outputClearedPlaceholder}}), + mkMsg("user", []types.ToolResult{{Content: outputClearedPlaceholder}}), + mkMsg("user", []types.ToolResult{{Content: outputClearedPlaceholder}}), + } + out, freed := clearOldToolResults(msgs, 100, 50) + if freed != 0 { + t.Fatalf("already-cleared results should free nothing, freed=%d", freed) + } + _ = out +} diff --git a/internal/engine/context_governor.go b/internal/engine/context_governor.go index 32713d0f..017a9546 100644 --- a/internal/engine/context_governor.go +++ b/internal/engine/context_governor.go @@ -129,6 +129,11 @@ func (s *Session) ManageContextBeforeTurn(ctx context.Context) (strategy string, } s.Persistence().SetRawMessages(ctxmgr.CollapseRepeatedMessages(s.Persistence().RawMessages())) + // Clear tier (0.8): replace old tool-result content with a placeholder before + // compacting, so short-term overflow is reclaimed gently without a full + // compaction. Only when clearing is insufficient does compaction run. + s.ClearOldToolResults(ctx) + s.EnsureAutoCompactor() if compactStrategy, ok := s.Persistence().AutoCompactor().AutoCompactIfNeeded(ctx, s); ok { return compactStrategy, true // recordCompaction emitted inside AutoCompactIfNeeded diff --git a/internal/permissions/approval_workflow.go b/internal/permissions/approval_workflow.go index c2333996..41001ffe 100644 --- a/internal/permissions/approval_workflow.go +++ b/internal/permissions/approval_workflow.go @@ -20,6 +20,10 @@ type ApprovalRequest struct { Status string // "pending", "approved", "denied", "expired" ExpiresAt time.Time Reason string + // DecisionAt and PauseDuration record how long the human deliberated before + // deciding, for approval-latency observability (adopted from herm). + DecisionAt time.Time + PauseDuration time.Duration } // ApprovalPolicy defines rules for how approval requests are handled. @@ -190,6 +194,8 @@ func (wf *ApprovalWorkflow) Approve(id, reason string) error { } req.Status = "approved" req.Reason = reason + req.DecisionAt = time.Now() + req.PauseDuration = req.DecisionAt.Sub(req.CreatedAt) wf.Pending = append(wf.Pending[:i], wf.Pending[i+1:]...) wf.History = append(wf.History, req) return nil @@ -210,6 +216,8 @@ func (wf *ApprovalWorkflow) Deny(id, reason string) error { } req.Status = "denied" req.Reason = reason + req.DecisionAt = time.Now() + req.PauseDuration = req.DecisionAt.Sub(req.CreatedAt) wf.Pending = append(wf.Pending[:i], wf.Pending[i+1:]...) wf.History = append(wf.History, req) return nil diff --git a/internal/permissions/approval_workflow_test.go b/internal/permissions/approval_workflow_test.go index 34b57e5a..46239d39 100644 --- a/internal/permissions/approval_workflow_test.go +++ b/internal/permissions/approval_workflow_test.go @@ -545,3 +545,24 @@ func TestApprovalConcurrentAccess(t *testing.T) { t.Errorf("expected 10 history entries, got %d", len(wf.History)) } } + +func TestApprovalRecordsPauseDuration(t *testing.T) { + wf := NewApprovalWorkflow(nil) + req, err := wf.RequestApproval("Bash", map[string]interface{}{"command": "make build"}, "MEDIUM") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if err := wf.Approve(req.ID, "ok"); err != nil { + t.Fatalf("approve error: %v", err) + } + // The request should now carry decision metadata (non-zero duration). + if req.Status != "approved" { + t.Fatalf("status = %s", req.Status) + } + if req.DecisionAt.IsZero() { + t.Fatal("expected DecisionAt to be set") + } + if req.PauseDuration <= 0 { + t.Fatalf("expected positive pause duration, got %s", req.PauseDuration) + } +}