Skip to content

fix(agents): identify Tool Search counter lifetimes - #116070

Merged
steipete merged 5 commits into
mainfrom
fix/tool-search-telemetry-scope
Aug 2, 2026
Merged

fix(agents): identify Tool Search counter lifetimes#116070
steipete merged 5 commits into
mainfrom
fix/tool-search-telemetry-scope

Conversation

@vincentkoc

Copy link
Copy Markdown
Member

Closes #116069

What Problem This Solves

Fixes an issue where Tool Search telemetry consumers could not derive exact operation totals when cumulative counters crossed a catalog replacement or process restart.

Why This Change Was Made

Add a short opaque counterScope to Tool Search telemetry. Fresh and restored catalogs receive a new scope, while appended client tools preserve the current scope and counter lifetime.

User Impact

Operators and benchmark tooling can compute exact search, describe, and call deltas without guessing counter resets. Tool execution behavior and the existing counters are unchanged.

Evidence

  • node scripts/run-vitest.mjs src/agents/tool-search.test.ts src/agents/tool-search-runtime.test.ts src/agents/tool-search-ranking.test.ts src/agents/embedded-agent-runner/run/attempt-prompt-tool-policy.test.ts
    • 176 tests passed across 3 shards
  • git diff --check
  • Fresh autoreview: clean, no accepted/actionable findings (0.91)
  • ShellBench released trace audit before this field: naive cumulative sum 1,217; heuristic 74; exact total unresolved
  • ShellBench consumer update: feat(native-eval): export tool discovery telemetry shellbench#60

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: S maintainer Maintainer-authored PR labels Jul 29, 2026
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal backlog priority with limited blast radius. labels Jul 29, 2026
@clawsweeper

clawsweeper Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed August 2, 2026, 1:39 AM ET / 05:39 UTC.

ClawSweeper review

What this changes

Adds an opaque counterScope to Tool Search telemetry so downstream consumers can distinguish counter lifetimes across catalog replacement and restoration.

Merge readiness

⚠️ Ready for maintainer review - 1 item remains

Keep this PR open for normal protected maintainer merge handling. Current main still lacks the telemetry lifetime identifier, while the branch adds it at the catalog lifecycle owner, exposes it in every telemetry result, documents the exact stability/rotation rules, and has an explicit maintainer acceptance of the new external contract.

Priority: P2
Reviewed head: d889e773fcf50349aa0d9b032f116f827e7d64f8

Review scores

Measure Result What it means
Overall readiness 🦞 diamond lobster (5/6) A focused, documented lifecycle-owner change with explicit maintainer acceptance and targeted regression coverage.
Proof confidence 🌊 off-meta tidepool Not applicable: This MEMBER-authored, maintainer-protected PR is outside the external-contributor real-behavior-proof gate; the PR body additionally reports focused owner-test coverage.
Patch quality 🦞 diamond lobster (5/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: This MEMBER-authored, maintainer-protected PR is outside the external-contributor real-behavior-proof gate; the PR body additionally reports focused owner-test coverage.
Evidence reviewed 6 items Current-main gap: Current main defines a Tool Search catalog session with entries and three cumulative counters only, and its telemetry projection returns those counters without a lifetime identifier; the central problem remains unresolved on main.
Canonical lifecycle implementation: The branch generates a scope for restored catalogs and generates or preserves it in the catalog registration owner: replacements receive a new scope, while appended client tools inherit the existing lifetime.
Telemetry projection and regression coverage: The branch returns counterScope from the central telemetry projection and tests fresh scope shape, replacement rotation, reuse/restoration rotation, appending stability, and prompt-policy narrowing stability.
Findings None None.
Security None None.

How this fits together

Tool Search builds a per-run catalog used by code-mode tool lookup and emits cumulative search, describe, and call counters with each result. Operator and benchmark consumers read that telemetry to calculate activity across catalog updates and restored runs.

flowchart LR
  A[Agent tool set] --> B[Tool Search catalog]
  B --> C[Catalog lifecycle]
  C --> D[Counter scope and totals]
  D --> E[Tool Search telemetry]
  E --> F[Operator and benchmark consumers]
Loading

Before merge

  • Resolve merge risk (P1) - counterScope is now a documented external telemetry contract: downstream totals depend on preserving its stated rotation and stability semantics in future catalog lifecycle changes.
Agent review details

Security

None.

PR surface

Source +7, Tests +86, Docs +6. Total +99 across 9 files.

View PR surface stats
Area Files Added Removed Net
Source 3 7 0 +7
Tests 4 99 13 +86
Docs 2 6 0 +6
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 9 112 13 +99

Review metrics

Metric Value Why it matters
Telemetry contract 1 additive field: counterScope This externally visible field gives downstream collectors a stable grouping key, so its lifetime semantics become compatibility-sensitive.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #116069
Summary: This PR is the active candidate fix for the linked report: it implements the report's requested opaque lifetime identity at the shared Tool Search catalog boundary.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge-risk options

Maintainer options:

  1. Merge the approved telemetry contract (recommended)
    Retain the documented lifetime rules and merge once the normal protected-PR checks for the current head complete.

Technical review

Best possible solution:

Land the documented scope contract after normal protected-PR checks, retaining the catalog-owner implementation and regression coverage for replacement, restoration, appends, and prompt-policy narrowing.

Do we have a high-confidence way to reproduce the issue?

Yes, source inspection gives a high-confidence path: current main emits cumulative counters without any lifetime identifier, so consumers cannot distinguish a reset from a continuing counter series; executing the runtime was not appropriate in this read-only review.

Is this the best way to solve the issue?

Yes. Adding the identifier at the catalog lifecycle owner, rather than inferring resets in consumers, is the narrow maintainable solution; the branch also covers the lifecycle distinctions that define the contract.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against b08569f6fdc1.

Labels

Label justifications:

  • P2: The change improves operator and benchmark telemetry accuracy without changing Tool Search execution behavior.
  • merge-risk: 🚨 compatibility: The added telemetry field has documented stability and rotation semantics that downstream consumers may rely on for exact totals.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🌊 off-meta tidepool and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: This MEMBER-authored, maintainer-protected PR is outside the external-contributor real-behavior-proof gate; the PR body additionally reports focused owner-test coverage.

Evidence

What I checked:

  • Current-main gap: Current main defines a Tool Search catalog session with entries and three cumulative counters only, and its telemetry projection returns those counters without a lifetime identifier; the central problem remains unresolved on main. (src/agents/tool-search-types.ts:111, b08569f6fdc1)
  • Canonical lifecycle implementation: The branch generates a scope for restored catalogs and generates or preserves it in the catalog registration owner: replacements receive a new scope, while appended client tools inherit the existing lifetime. (src/agents/tool-search-catalog.ts:128, d889e773fcf5)
  • Telemetry projection and regression coverage: The branch returns counterScope from the central telemetry projection and tests fresh scope shape, replacement rotation, reuse/restoration rotation, appending stability, and prompt-policy narrowing stability. (src/agents/tool-search-runtime.ts:279, d889e773fcf5)
  • Documented consumer contract: Both Tool Search and code-mode documentation state that the opaque scope is stable for appends and prompt-policy narrowing, and changes for replacement or restoration. Public docs: docs/tools/tool-search.md. (docs/tools/tool-search.md:330, d889e773fcf5)
  • Feature-history provenance: git blame ties the current catalog restore and registration paths to the August 1 Tool Search lifecycle work; the same area was then refined on this branch, including the accepted visibility and documentation follow-ups. (src/agents/tool-search-catalog.ts:128, 0f7c624a3f65)
  • Maintainer contract decision: The PR discussion explicitly accepts counterScope as an opaque external telemetry compatibility contract and approves its rotation on replacement/restoration and stability on append/narrowing paths. (d889e773fcf5)

Likely related people:

  • steipete: Current-main blame attributes the catalog lifecycle to Peter Steinberger, and the branch's scope-visibility and documentation follow-ups plus the explicit contract decision are associated with steipete. (role: feature owner and contract approver; confidence: high; commits: 0f7c624a3f65, e42348aad1cd, d889e773fcf5; files: src/agents/tool-search-catalog.ts, src/agents/tool-search-runtime.ts, docs/tools/tool-search.md)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (3 earlier review cycles)
  • reviewed 2026-07-29T20:16:21.872Z sha 8d01d1a :: needs maintainer review before merge. :: none
  • reviewed 2026-08-02T05:08:41.762Z sha 1483a0c :: needs maintainer review before merge. :: none
  • reviewed 2026-08-02T05:22:58.764Z sha 67e28b6 :: needs maintainer review before merge. :: none

@openclaw-barnacle openclaw-barnacle Bot added the docs Improvements or additions to documentation label Aug 2, 2026
@steipete
steipete marked this pull request as ready for review August 2, 2026 05:02
@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Aug 2, 2026
@steipete

steipete commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Maintainer decision: counterScope is an explicitly accepted opaque external telemetry compatibility contract. It rotates only when a Tool Search catalog is replaced or restored, and remains stable when entries are appended or prompt-policy narrowing changes visibility. The documented downstream grouping semantics and 177 passing focused/owner tests have been reviewed and approved.

@clawsweeper clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Aug 2, 2026
@steipete
steipete merged commit fb087bf into main Aug 2, 2026
120 checks passed
@steipete
steipete deleted the fix/tool-search-telemetry-scope branch August 2, 2026 05:50
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Aug 2, 2026
* fix(agents): scope Tool Search telemetry counters

* fix(agents): keep tool-search scopes visible

* docs(tool-search): document counter scopes

---------

Co-authored-by: Peter Steinberger <steipete@macos.shared>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling docs Improvements or additions to documentation maintainer Maintainer-authored PR merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Tool Search telemetry cannot identify counter resets

2 participants