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: found issues before merge. Reviewed August 3, 2026, 12:10 AM ET / 04:10 UTC.

ClawSweeper review

What this changes

The branch adds normalized OpenClaw tool-discovery telemetry, trace-fidelity and runner provenance fields, a new discovery_events.csv export, and focused audit-export tests.

Merge readiness

⚠️ Ready for maintainer review - 4 items remain

Keep this member-authored PR open: current main still lacks discovery-event exports, but the new emitted artifact is not yet included in ShellBench’s normative campaign runbook or expected research-directory tree. The linked feature request remains open and needs maintainer confirmation of this expanded research-output contract.

Priority: P3
Reviewed head: 15dce335440688dd25fa45c28495e3fc0daa75a6
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The implementation is focused and CI-backed, but the new research artifact is not yet incorporated into the normative campaign contract.
Proof confidence 🌊 off-meta tidepool Not applicable: The PR author is a repository member, so the external-contributor proof gate does not apply; the PR body describes a real artifact re-export, but it does not link a redacted output artifact for independent inspection.
Patch quality 🦐 gold shrimp (3/6) 1 actionable review finding remain.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The PR author is a repository member, so the external-contributor proof gate does not apply; the PR body describes a real artifact re-export, but it does not link a redacted output artifact for independent inspection.
Evidence reviewed 5 items Current main lacks the requested export: Current main writes four CSV outputs plus research_audit.json; it contains no discovery-event export, discovery status, runner commit, or trace-fidelity fields, so the central capability is not already implemented.
Branch adds a fifth research artifact: The proposed exporter writes discovery_events.csv and includes it in the generated JSON output map.
Normative runbook omits the new artifact: The campaign contract still lists only trace_inventory.csv, model_identity_audit.csv, turn_usage.csv, tool_calls.csv, and research_audit.json both in its output list and expected directory tree.
Findings 1 actionable finding [P3] Document discovery_events.csv in the campaign runbook
Security None None.

How this fits together

ShellBench’s native-evaluation research audit converts extracted benchmark traces and run manifests into CSV tables used to validate model identity, tools, usage, and campaign provenance. This change adds discovery-operation evidence so cross-harness analysis can distinguish observed zero use from unsupported or unobservable discovery events.

flowchart LR
  A[Run manifests] --> C[Research audit exporter]
  B[Extracted task traces] --> C
  C --> D[Discovery telemetry parser]
  D --> E[Normalized discovery events]
  C --> F[Trace inventory]
  E --> G[Research CSV directory]
  F --> G
  G --> H[Campaign analysis and audit]
Loading

Decision needed

Question Recommendation
Should ShellBench make normalized discovery telemetry a required, documented research artifact for native benchmark campaigns? Adopt the artifact contract: Accept discovery telemetry as a research output, then require the PR to document discovery_events.csv in both campaign artifact lists before merge.

Why: This PR expands the normative campaign output contract rather than repairing an established broken export, and the linked feature request is explicitly awaiting product-direction review.

Before merge

  • Document discovery_events.csv in the campaign runbook (P3) - Add the new artifact and a short purpose description to both normative output lists. The exporter now writes discovery_events.csv, but operators following the documented campaign tree will retain only the previous four CSV tables; this continues the prior review’s unresolved finding.
  • Resolve merge risk (P1) - Operators following the normative campaign contract will not know to retain or audit discovery_events.csv, leaving the PR’s new telemetry absent from the documented research artifact set.
  • Resolve merge risk (P1) - The linked request proposes a new durable research-output surface; accepting its schema and semantics without maintainer confirmation could create a contract that later campaign tooling must preserve.

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
Code and test growth production +378, tests +437 The feature adds a substantial new audit schema and parser, so its documented campaign-contract integration matters before merge.
Export surface 1 new CSV artifact discovery_events.csv expands the documented research directory consumed by campaign operators.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #59
Summary: This PR is the active implementation candidate for the open feature request to export normalized native-harness discovery telemetry.

Members:

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

Merge-risk options

Maintainer options:

  1. Decide the mitigation before merge
    Confirm that normalized discovery telemetry belongs in the campaign’s durable research schema, then add discovery_events.csv and its purpose to both normative runbook artifact lists and retain focused export coverage.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Technical review

Best possible solution:

Confirm that normalized discovery telemetry belongs in the campaign’s durable research schema, then add discovery_events.csv and its purpose to both normative runbook artifact lists and retain focused export coverage.

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

Not applicable as a bug reproduction: source inspection confirms that current main emits only the existing four CSV audit tables, while the branch adds a distinct fifth artifact. The branch’s test coverage exercises synthetic exports, but no additional live failure reproduction is needed to establish the documentation-contract gap.

Is this the best way to solve the issue?

No—not yet. The parser and normalization approach is focused, but adding a durable campaign artifact without updating the normative runbook and confirming the research-schema direction leaves the solution incomplete.

Full review comments:

  • [P3] Document discovery_events.csv in the campaign runbook — scripts/native_eval/research_audit.py:965
    Add the new artifact and a short purpose description to both normative output lists. The exporter now writes discovery_events.csv, but operators following the documented campaign tree will retain only the previous four CSV tables; this continues the prior review’s unresolved finding.
    Confidence: 0.99

Overall correctness: patch is correct
Overall confidence: 0.91

AGENTS.md: found and applied where relevant.

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

Labels

Label justifications:

  • P3: This is a useful but non-urgent research-audit feature whose remaining blocker is documentation and product-contract confirmation, not an active runtime regression.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The PR author is a repository member, so the external-contributor proof gate does not apply; the PR body describes a real artifact re-export, but it does not link a redacted output artifact for independent inspection.

Evidence

What I checked:

Likely related people:

  • vincentkoc: Introduced the existing research-audit exporter and normative campaign runbook on current main, and the proposed branch extends that same owned audit surface. (role: native-evaluation audit and runbook author; confidence: high; commits: 569b5c39c783, 7eefe9fe0c3b, 15dce3354406; files: scripts/native_eval/research_audit.py, .agents/skills/shellbench-research-runbook/references/runbook.md, tests/test_native_eval_research_audit.py)

Rank-up moves

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

  • Add discovery_events.csv and its purpose to both runbook artifact lists.
  • Include a redacted sample export or linked artifact in the PR body if maintainers want independently inspectable runtime evidence.

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 (23 earlier review cycles; latest 8 shown)
  • reviewed 2026-08-02T13:16:58.014Z sha 15dce33 :: needs maintainer review before merge. :: none
  • reviewed 2026-08-02T14:57:47.419Z sha 15dce33 :: needs maintainer review before merge. :: none
  • reviewed 2026-08-02T17:06:45.965Z sha 15dce33 :: needs maintainer review before merge. :: none
  • reviewed 2026-08-02T19:14:43.259Z sha 15dce33 :: needs maintainer review before merge. :: none
  • reviewed 2026-08-02T20:38:00.954Z sha 15dce33 :: needs maintainer review before merge. :: none
  • reviewed 2026-08-02T22:56:01.766Z sha 15dce33 :: needs maintainer review before merge. :: none
  • reviewed 2026-08-03T00:19:37.810Z sha 15dce33 :: found issues before merge. :: [P3] Document the new audit artifact in the campaign runbook
  • reviewed 2026-08-03T01:48:38.839Z sha 15dce33 :: needs changes before merge. :: [P3] Document the new audit artifact 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. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. 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

P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

1 participant