Skip to content

feat(native-eval): export tool discovery telemetry - #60

Open
vincentkoc wants to merge 2 commits into
mainfrom
fix/native-tool-discovery-telemetry
Open

feat(native-eval): export tool discovery telemetry#60
vincentkoc wants to merge 2 commits into
mainfrom
fix/native-tool-discovery-telemetry

Conversation

@vincentkoc

Copy link
Copy Markdown
Member

What does this PR do?

Exports native-harness tool discovery operations separately from final tool
calls and records the trace fidelity needed to interpret missing events.

Why?

Fixes #59.

The existing research audit records tool_search_code as one wrapper call but
drops OpenClaw's embedded catalog and search/describe/call counters. It also
cannot distinguish disabled discovery from unsupported or unobservable native
events, which makes cross-harness comparisons produce fake zeros.

Changes

  • add discovery_events.csv with normalized search, describe, and call counts
  • extract OpenClaw code-bridge telemetry and structured Tool Search controls
  • report disabled, observed, supported-but-unused, unobservable, and unsupported states
  • add runner commit and trace fidelity to each task inventory row
  • leave operation success unknown unless the trace proves it

Tests

  • python -m pytest -q tests/test_native_eval_research_audit.py
  • python -m ruff check scripts/native_eval/research_audit.py tests/test_native_eval_research_audit.py
  • re-exported the completed released-off ablation artifacts and verified
    all three tasks report the expected runner SHA, session fidelity,
    disabled discovery state, and zero discovery operations

@vincentkoc
vincentkoc marked this pull request as ready for review July 29, 2026 18:04
@vincentkoc
vincentkoc requested a review from a team as a code owner July 29, 2026 18:04
@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Jul 29, 2026
@clawsweeper

clawsweeper Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codex review: blocked before merge. Reviewed September 5, 2026, 2:55 AM ET / 06:55 UTC.

ClawSweeper review

What this changes

Adds a separate research CSV for tool discovery operations, counter normalization, and task-level discovery status, runner identity, and trace fidelity.

Merge readiness

Blocked before merge - 5 items remain

Keep open: current main does not implement the linked request, and the branch still permits misleading discovery totals. The previous classification and documentation findings remain unresolved.

Priority: P2
Reviewed head: 15dce335440688dd25fa45c28495e3fc0daa75a6

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The focused implementation and substantial tests are useful, but evidence classification and counting defects prevent reliable research output.
Proof confidence 🌊 off-meta tidepool Not applicable: The MEMBER-authored PR is exempt from the ordinary contributor-proof gate. Its captured body reports the production exporter processing three released-off artifacts successfully, covering disabled-state output but not positive counter normalization; no authority-chain proof requirement applies.
Patch quality 🦐 gold shrimp (3/6) 3 actionable review findings remain.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The MEMBER-authored PR is exempt from the ordinary contributor-proof gate. Its captured body reports the production exporter processing three released-off artifacts successfully, covering disabled-state output but not positive counter normalization; no authority-chain proof requirement applies.
Evidence reviewed 13 items Verified introduced changes: The complete local merge-base-to-head diff contains only the exporter and its tests: production +378/-2 lines and tests +437/-1 lines.
Still absent on current main: Current main exports the existing inventory, usage, tool-call, and identity tables without discovery events or discovery classifications. Its exporter and tests are unchanged from the pinned PR base.
Missing evidence becomes disabled: With no discovery rows, telemetry, control calls, or mode metadata, the new fallback returns disabled. The existing export fixture explicitly asserts this classification despite recording no disabled setting.
Findings 3 actionable findings [P2] Preserve absent discovery evidence as unobservable
[P2] Normalize observed scoped counters without requiring mode metadata
[P3] Document the discovery-status and count-semantics contract
Security None None.

How this fits together

ShellBench’s research exporter reads benchmark run metadata and normalized agent traces. It produces CSV tables that researchers use to compare tool use, model identity, and resource consumption across harnesses.

flowchart TD
  A[Run metadata] --> C[Research exporter]
  B[Agent traces] --> C
  C --> D[Extract discovery evidence]
  D --> E[Validate scopes and normalize counters]
  E --> F[Discovery operations CSV]
  E --> G[Task inventory and summary]
Loading

Before merge

  • Preserve absent discovery evidence as unobservable (P2) - When an OpenClaw artifact contains no discovery calls or telemetry and neither metadata source records a mode, this fallback emits disabled. Both the plan writer and run-manifest writer omit that field, so absence does not establish a disabled configuration. This retained finding still produces the false-zero interpretation the feature intends to prevent; require explicit disabled evidence and otherwise preserve an unobservable state.
  • Normalize observed scoped counters without requiring mode metadata (P2) - A tool_search_code observation with valid counterScope telemetry is parsed even when mode metadata is absent, but this guard skips normalization. Its rows remain cumulative_scoped, _discovery_status returns observed, and _discovery_operation_count excludes every row, yielding zero for positive telemetry. Removing the two mode fields from the new integration fixture exposes this path. Normalize from the extracted counter semantics instead. This is a late discovery on code unchanged since the previous reviewed head.
  • Document the discovery-status and count-semantics contract (P3) - The new table mixes event counts, deltas, zero-count scope markers, unresolved cumulative values, and invalid scopes, while the summary excludes several categories. The research runbook still lists only the old outputs and provides no interpretation rules, so consumers cannot safely distinguish measured zero from unavailable totals or know which rows are additive. The previous documentation finding remains unresolved; add these rules beside the existing research-export instructions.
  • Resolve merge risk (P1) - Research comparisons could mistake missing discovery evidence for measured zero, or omit positive scoped observations from operation totals.
  • Complete next step (P2) - Fix missing-mode classification and scoped-counter normalization, add legacy-artifact regression coverage, and document the discovery export contract.

Findings

  • [P2] Preserve absent discovery evidence as unobservable — scripts/native_eval/research_audit.py:502
  • [P2] Normalize observed scoped counters without requiring mode metadata — scripts/native_eval/research_audit.py:832-835
  • [P3] Document the discovery-status and count-semantics contract — scripts/native_eval/research_audit.py:965
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test LOC Production +378/-2; tests +437/-1 Production growth implements a separate discovery export and counter validation, with substantial accompanying regression coverage.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #59
Summary: This PR is the implementation candidate for the same-author discovery-export request, which remains open.

Members:

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

Merge-risk options

Maintainer options:

  1. Repair evidence classification and counting (recommended)
    Preserve unknown discovery states, normalize observed scoped counters without requiring mode metadata, and document which rows contribute to totals.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Fix missing-mode discovery classification and normalize valid scoped telemetry independently of optional mode metadata; add exporter regressions for legacy artifacts and document discovery statuses and count semantics without changing harness defaults.

Technical review

Best possible solution:

Keep discovery exports separate, preserve unknown evidence states, and normalize valid scoped telemetry independently of optional mode metadata.

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

Yes, the PR-head source provides clear triggers: omit mode metadata with no discovery evidence, or retain scoped telemetry while omitting mode metadata. No target code or tests were executed during this read-only review.

Is this the best way to solve the issue?

The separate export is appropriate, but its evidence handling needs repair. Explicit mode persistence is necessary only for claims about configured state; older artifacts can remain safely unobservable while their valid scoped telemetry is normalized.

Full review comments:

  • [P2] Preserve absent discovery evidence as unobservable — scripts/native_eval/research_audit.py:502
    When an OpenClaw artifact contains no discovery calls or telemetry and neither metadata source records a mode, this fallback emits disabled. Both the plan writer and run-manifest writer omit that field, so absence does not establish a disabled configuration. This retained finding still produces the false-zero interpretation the feature intends to prevent; require explicit disabled evidence and otherwise preserve an unobservable state.
    Confidence: 0.99
  • [P2] Normalize observed scoped counters without requiring mode metadata — scripts/native_eval/research_audit.py:832-835
    A tool_search_code observation with valid counterScope telemetry is parsed even when mode metadata is absent, but this guard skips normalization. Its rows remain cumulative_scoped, _discovery_status returns observed, and _discovery_operation_count excludes every row, yielding zero for positive telemetry. Removing the two mode fields from the new integration fixture exposes this path. Normalize from the extracted counter semantics instead. This is a late discovery on code unchanged since the previous reviewed head.
    Confidence: 0.99
    Late finding: first raised on code an earlier review cycle already covered.
  • [P3] Document the discovery-status and count-semantics contract — scripts/native_eval/research_audit.py:965
    The new table mixes event counts, deltas, zero-count scope markers, unresolved cumulative values, and invalid scopes, while the summary excludes several categories. The research runbook still lists only the old outputs and provides no interpretation rules, so consumers cannot safely distinguish measured zero from unavailable totals or know which rows are additive. The previous documentation finding remains unresolved; add these rules beside the existing research-export instructions.
    Confidence: 0.95

Overall correctness: patch is incorrect
Overall confidence: 0.97

AGENTS.md: not found in the target repository.

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

Labels

Label justifications:

  • P2: This is a bounded research-export improvement with correctness defects that can distort benchmark interpretation.
  • merge-risk: 🚨 other: The introduced classifications and counting gate can produce misleading discovery measurements despite passing existing tests.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: The MEMBER-authored PR is exempt from the ordinary contributor-proof gate. Its captured body reports the production exporter processing three released-off artifacts successfully, covering disabled-state output but not positive counter normalization; no authority-chain proof requirement applies.

Evidence

Acceptance criteria:

  • [P1] python -m pytest -q tests/test_native_eval_research_audit.py.
  • [P1] python -m ruff check scripts/native_eval/research_audit.py tests/test_native_eval_research_audit.py.

What I checked:

  • Verified introduced changes: The complete local merge-base-to-head diff contains only the exporter and its tests: production +378/-2 lines and tests +437/-1 lines. (scripts/native_eval/research_audit.py:373, 15dce3354406)
  • Still absent on current main: Current main exports the existing inventory, usage, tool-call, and identity tables without discovery events or discovery classifications. Its exporter and tests are unchanged from the pinned PR base. (scripts/native_eval/research_audit.py:589, c1a79f731541)
  • Missing evidence becomes disabled: With no discovery rows, telemetry, control calls, or mode metadata, the new fallback returns disabled. The existing export fixture explicitly asserts this classification despite recording no disabled setting. (scripts/native_eval/research_audit.py:502, 15dce3354406)
  • Metadata producer does not establish mode: The current-main run manifest does not write openclaw_tool_search_mode; the inspected plan writer also omits it. At the PR head, native OpenClaw configuration leaves tool-search behavior to the runtime default. (scripts/native_eval/run_job.py:238, c1a79f731541)
  • Scoped observations can disappear from totals: Normalization requires mode=code even after tool_search_code telemetry has been extracted. Without that optional metadata, scoped rows remain cumulative_scoped, receive observed status, and are excluded from discovery_event_count. (scripts/native_eval/research_audit.py:832, 15dce3354406)
  • Export interpretation remains undocumented: The research runbook lists the old outputs but does not explain the new discovery table, statuses, scope markers, or exclusion of unresolved counters from totals. (.agents/skills/shellbench-research-runbook/references/runbook.md:417, 15dce3354406)

Likely related people:

  • unknown: The claimed source-line change could not be verified from bounded local history. (role: source history unknown; confidence: low)
  • Peter Steinberger: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Add export regressions for absent mode metadata, both without discovery evidence and with positive scoped telemetry.
  • Document discovery statuses, additive count semantics, and excluded or unavailable totals.

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 (75 earlier review cycles; latest 8 shown)
  • reviewed 2026-08-09T19:45:48.262Z sha 15dce33 :: found issues before merge. :: [P2] Preserve absent discovery evidence as unobservable | [P3] Document the discovery-export state contract
  • reviewed 2026-08-09T22:01:20.305Z sha 15dce33 :: needs changes before merge. :: [P2] Preserve absent discovery evidence as unobservable | [P3] Document the discovery-state contract
  • reviewed 2026-08-11T23:12:00.046Z sha 15dce33 :: needs changes before merge. :: [P2] Preserve absent discovery evidence as unobservable | [P3] Document the discovery-status contract
  • reviewed 2026-08-12T01:18:18.423Z sha 15dce33 :: needs changes before merge. :: [P2] Preserve absent discovery evidence as unobservable | [P3] Document the discovery-status contract
  • reviewed 2026-08-12T06:19:14.411Z sha 15dce33 :: found issues before merge. :: [P2] Keep absent discovery evidence unobservable | [P3] Document the discovery-status contract
  • reviewed 2026-08-14T09:38:11.166Z sha 15dce33 :: needs changes before merge. :: [P2] Preserve absent discovery evidence as unobservable | [P3] Document the discovery-status contract
  • reviewed 2026-08-22T20:06:47.257Z sha 15dce33 :: needs changes before merge. :: [P2] Keep missing discovery evidence unobservable | [P3] Document the discovery-status contract
  • reviewed 2026-08-29T12:58:53.995Z sha 15dce33 :: needs changes before merge. :: [P2] Preserve absent discovery evidence as unobservable | [P3] Document the discovery-status contract

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jul 29, 2026
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. 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. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed 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. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jul 29, 2026
@clawsweeper clawsweeper Bot added status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. proof: sufficient Contributor real behavior proof is sufficient. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal priority bug or improvement with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. proof: sufficient Contributor real behavior proof is sufficient. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Aug 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(native-eval): export normalized tool discovery telemetry

1 participant