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: needs changes before merge. Reviewed August 4, 2026, 8:21 PM ET / August 5, 2026, 00:21 UTC.

ClawSweeper review

What this changes

The PR adds normalized native-harness discovery telemetry, including a new CSV export and trace-fidelity metadata, to the research audit.

Merge readiness

⚠️ Ready for maintainer review - 3 items remain

Keep open: current main does not export this telemetry, and the PR has a focused, source-backed implementation. One previously reported P3 documentation gap remains before merge.

Priority: P3
Reviewed head: 15dce335440688dd25fa45c28495e3fc0daa75a6

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) Focused implementation, coverage, and stated artifact proof are solid; one small campaign-contract update remains.
Proof confidence 🐚 platinum hermit (4/6) Sufficient (live_output): The PR supplies after-fix completed-artifact re-export results for three tasks, and the provided context marks that proof sufficient.
Patch quality 🐚 platinum hermit (4/6) 1 actionable review finding remain.

Verification

Check Result Evidence
Real behavior Verified Sufficient (live_output): The PR supplies after-fix completed-artifact re-export results for three tasks, and the provided context marks that proof sufficient.
Evidence reviewed 4 items New export is absent from current main: The PR adds the discovery-events export and its summary entry; current main has neither, so the requested capability is not already implemented.
Campaign contract still lists only the prior outputs: The normative runbook describes four CSV exports plus the JSON summary and its delivery tree omits discovery_events.csv, despite the PR producing it.
Feature-history provenance: The current research-audit exporter and campaign runbook were introduced together by the same area contributor in the main-branch campaign-runbook commit.
Findings 1 actionable finding [P3] Document discovery_events.csv in the campaign runbook
Security None None.

How this fits together

ShellBench’s native-evaluation audit consumes a run index and extracted harness traces, then produces campaign tables used to assess reproducibility and comparability. This change adds separate discovery-operation evidence alongside existing task, turn, tool, and model-identity outputs.

flowchart LR
A[Run index] --> C[Research audit exporter]
B[Extracted native traces] --> C
C --> D[Discovery telemetry normalization]
C --> E[Existing audit tables]
D --> F[Discovery events CSV]
E --> G[Campaign audit]
F --> G
Loading

Before merge

  • Document discovery_events.csv in the campaign runbook (P3) - Add this new retained output to the runbook’s research-audit list and delivery tree, and define its status, count-semantics, and trace-fidelity fields. The current contract still lists only the older outputs, so operators can discard or misread this evidence.
  • Resolve merge risk (P1) - Without updating the campaign runbook, operators can omit, discard, or misinterpret the new discovery evidence and its count/fidelity states.
  • Complete next step (P2) - A narrow, mechanical documentation repair can resolve the remaining merge risk without changing telemetry behavior.

Findings

  • [P3] Document discovery_events.csv in the campaign runbook — scripts/native_eval/research_audit.py:965
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production versus test delta production +378/-2; tests +437/-1 The telemetry parser is covered by a slightly larger focused test expansion, but the campaign contract remains unchanged.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #59
Summary: This PR is the concrete implementation candidate for the open normalized-discovery-telemetry request.

Members:

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

Merge-risk options

Maintainer options:

  1. Document the retained discovery artifact (recommended)
    Add discovery_events.csv and its interpretation rules to the campaign runbook before merge so operators preserve and read the new evidence consistently.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Update the campaign runbook’s research-audit output list and delivery tree for discovery_events.csv, including discovery status, count semantics, and trace-fidelity meanings; run the focused audit test and a diff check.

Technical review

Best possible solution:

Keep the separate telemetry export and document its retention location plus discovery-state, count-semantics, and trace-fidelity meanings in the campaign contract.

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

Not applicable: this PR adds a research-audit capability rather than repairing an established failing runtime path; its stated after-fix artifact export is the relevant validation.

Is this the best way to solve the issue?

No: the exporter is a suitable implementation, but the campaign runbook must define the new retained artifact and how to interpret its fields.

Full review comments:

  • [P3] Document discovery_events.csv in the campaign runbook — scripts/native_eval/research_audit.py:965
    Add this new retained output to the runbook’s research-audit list and delivery tree, and define its status, count-semantics, and trace-fidelity fields. The current contract still lists only the older outputs, so operators can discard or misread this evidence.
    Confidence: 0.99

Overall correctness: patch is correct
Overall confidence: 0.95

AGENTS.md: found, but no applicable review policy affected this item.

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

Labels

Label justifications:

  • P3: This is a bounded research-audit enhancement with no evidence of a user-facing benchmark runtime regression.
  • merge-risk: 🚨 other: The PR adds a retained research deliverable, while the normative campaign artifact contract still omits it.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR supplies after-fix completed-artifact re-export results for three tasks, and the provided context marks that proof sufficient.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR supplies after-fix completed-artifact re-export results for three tasks, and the provided context marks that proof sufficient.

Evidence

Acceptance criteria:

  • [P1] python -m pytest -q tests/test_native_eval_research_audit.py.
  • [P1] git diff --check.

What I checked:

  • New export is absent from current main: The PR adds the discovery-events export and its summary entry; current main has neither, so the requested capability is not already implemented. (scripts/native_eval/research_audit.py:965, 15dce3354406)
  • Campaign contract still lists only the prior outputs: The normative runbook describes four CSV exports plus the JSON summary and its delivery tree omits discovery_events.csv, despite the PR producing it. (.agents/skills/shellbench-research-runbook/references/runbook.md:417, 569b5c39c783)
  • Feature-history provenance: The current research-audit exporter and campaign runbook were introduced together by the same area contributor in the main-branch campaign-runbook commit. (scripts/native_eval/research_audit.py:1, 569b5c39c783)
  • After-fix behavior evidence: The PR body reports re-exporting completed released-off ablation artifacts and observing the expected runner SHA, session fidelity, disabled discovery state, and zero operations across three tasks.

Likely related people:

  • vincentkoc: Authored the main-branch commit that introduced both the audit exporter and its normative campaign runbook, in addition to contributing this PR. (role: introduced the research-audit and campaign-contract surface; confidence: high; commits: 569b5c39c783; files: scripts/native_eval/research_audit.py, .agents/skills/shellbench-research-runbook/references/runbook.md)

Rank-up moves

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

  • Document discovery_events.csv and its interpretation in the normative research-campaign runbook.

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 (31 earlier review cycles; latest 8 shown)
  • reviewed 2026-08-03T04:10:11.149Z sha 15dce33 :: found issues before merge. :: [P3] Document discovery_events.csv in the campaign runbook
  • reviewed 2026-08-03T09:05:54.364Z sha 15dce33 :: needs changes before merge. :: [P3] Document discovery_events.csv in the campaign runbook
  • reviewed 2026-08-03T20:14:23.938Z sha 15dce33 :: needs changes before merge. :: [P3] Document discovery_events.csv in the campaign runbook
  • reviewed 2026-08-04T10:01:46.306Z sha 15dce33 :: needs changes before merge. :: [P3] Document discovery_events.csv in the campaign runbook
  • reviewed 2026-08-04T15:03:05.095Z sha 15dce33 :: needs changes before merge. :: [P3] Document discovery_events.csv in the campaign runbook
  • reviewed 2026-08-04T18:03:39.181Z sha 15dce33 :: found issues before merge. :: [P3] Document discovery_events.csv in the campaign runbook
  • reviewed 2026-08-04T21:04:51.074Z sha 15dce33 :: found issues before merge. :: [P3] Document discovery_events.csv in the campaign runbook
  • reviewed 2026-08-04T23:02:40.484Z sha 15dce33 :: needs changes before merge. :: [P3] Document discovery_events.csv in the campaign runbook

@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 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. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. proof: sufficient Contributor real behavior proof is sufficient. and removed 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. 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. labels Jul 31, 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. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. proof: sufficient Contributor real behavior proof is sufficient. 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

1 participant