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 22, 2026, 4:06 PM ET / 20:06 UTC.

ClawSweeper review

What this changes

The PR adds normalized native-harness discovery telemetry exports and trace-fidelity metadata to ShellBench research-audit CSVs.

Merge readiness

⚠️ Ready for maintainer review - 3 items remain

Keep open: the earlier P2 blocker remains on the unchanged head. Missing discovery configuration and events are still recorded as disabled, which can recreate the false-zero comparison the export is intended to prevent.

Priority: P2
Reviewed head: 15dce335440688dd25fa45c28495e3fc0daa75a6

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The audit extension has focused tests, but the unresolved unknown-versus-disabled classification can corrupt its core comparison data.
Proof confidence 🌊 off-meta tidepool Not applicable: This MEMBER-authored PR is exempt from the external-contributor real-behavior-proof gate; its body also reports a completed artifact re-export.
Patch quality 🦐 gold shrimp (3/6) 2 actionable review findings remain.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: This MEMBER-authored PR is exempt from the external-contributor real-behavior-proof gate; its body also reports a completed artifact re-export.
Evidence reviewed 5 items Missing control is not disabled evidence: The runner writes tools.toolSearch only when CLAWBENCH_OPENCLAW_TOOL_SEARCH has an explicit value; an unset variable leaves the configuration unchanged.
Current classifier collapses unknown into disabled: The no-event OpenClaw branch returns disabled whenever openclaw_mode is empty, despite no explicit disabled signal.
Prior blocker remains unchanged: The prior review identified this same problem at the same head; the audit files are unchanged since that review.
Findings 2 actionable findings [P2] Keep missing discovery evidence unobservable
[P3] Document the discovery-status contract
Security None None.

Live Verification

Command: python -m scripts.native_eval.research_audit --help

Result: FAIL (failed) — execution before step 1 run: sh -lc pnpm install --ignore-scripts --frozen-lockfile failed: ! Corepack is about to download https://registry.npmjs.org/pnpm/-/pnpm-11.22.0.tgz

sh -lc pnpm install --ignore-scripts --frozen-lockfile failed: ! Corepack is about to download https://registry.npmjs.org/pnpm/-/pnpm-11.22.0.tgz

Assertions:

  • FAIL expect_output: --run-index

How this fits together

The native-evaluation research audit reads completed harness trajectories and manifests, then exports CSV inventories used to compare tool use across harnesses. Discovery telemetry is extracted, normalized, classified, and written to trace and discovery-event outputs.

flowchart LR
  A[Harness trajectory] --> B[Research audit]
  C[Run manifest and config] --> B
  B --> D[Discovery telemetry extraction]
  D --> E[Counter normalization]
  E --> F[Discovery status classification]
  F --> G[Trace inventory CSV]
  E --> H[Discovery events CSV]
Loading

Before merge

  • Keep missing discovery evidence unobservable (P2) - When the run has neither a discovery control nor discovery calls, this returns disabled. The runner only writes tools.toolSearch when the environment explicitly sets it, so absence cannot prove discovery was off; emitting disabled recreates the false-zero comparison this PR is intended to prevent. Return an unobservable state and cover that trace shape.
  • Document the discovery-status contract (P3) - The new CSV exposes non-obvious status and counter-semantics values without a consumer-facing contract. Document when each state is emitted, especially disabled, unobservable, unsupported, and supported-but-unused, so downstream research does not interpret them as zero counts.
  • Resolve merge risk (P1) - The disabled classification can silently publish misleading cross-harness comparison data when configuration and telemetry are absent rather than explicitly disabled.

Findings

  • [P2] Keep missing discovery evidence unobservable — scripts/native_eval/research_audit.py:502
  • [P3] Document the discovery-status contract — scripts/native_eval/research_audit.py:463-505
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Audit export surface production +378/-2, tests +437/-1 across 2 files The new CSV schema and classification logic are concentrated in one exporter and its regression suite.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #59
Summary: This PR is the explicitly linked candidate implementation for the still-open 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. Preserve unknown discovery state (recommended)
    Return an unobservable status unless explicit run metadata proves tool discovery was disabled, then add the regression fixture and status documentation.
  2. Accept ambiguous zero interpretation
    Merge the current classifier only if maintainers intentionally want absent configuration and telemetry recorded as disabled despite the comparison risk.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Keep absent discovery configuration and events unobservable; add regression coverage and document the status contract.

Technical review

Best possible solution:

Preserve absent configuration and events as unobservable unless explicit runner metadata proves discovery was disabled, then document the status contract for CSV consumers.

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

Yes. A run with no CLAWBENCH_OPENCLAW_TOOL_SEARCH value and no discovery events reaches the source-proven disabled branch despite lacking an explicit disabled signal.

Is this the best way to solve the issue?

No. The export approach is useful, but its classifier must preserve missing evidence as unobservable and state that contract before researchers consume the data.

Full review comments:

  • [P2] Keep missing discovery evidence unobservable — scripts/native_eval/research_audit.py:502
    When the run has neither a discovery control nor discovery calls, this returns disabled. The runner only writes tools.toolSearch when the environment explicitly sets it, so absence cannot prove discovery was off; emitting disabled recreates the false-zero comparison this PR is intended to prevent. Return an unobservable state and cover that trace shape.
    Confidence: 0.98
  • [P3] Document the discovery-status contract — scripts/native_eval/research_audit.py:463-505
    The new CSV exposes non-obvious status and counter-semantics values without a consumer-facing contract. Document when each state is emitted, especially disabled, unobservable, unsupported, and supported-but-unused, so downstream research does not interpret them as zero counts.
    Confidence: 0.92

Overall correctness: patch is incorrect
Overall confidence: 0.98

AGENTS.md: not found in the target repository.

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

Labels

Label justifications:

  • P2: Incorrectly labeling unknown discovery evidence as disabled can distort research comparisons but is confined to audit exports.
  • merge-risk: 🚨 other: The PR can silently publish misleading benchmark telemetry classifications that ordinary test success does not resolve.
  • 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: This MEMBER-authored PR is exempt from the external-contributor real-behavior-proof gate; its body also reports a completed artifact re-export.

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:

Likely related people:

  • Vincent Koc: Introduced the current-main research-audit files and authored this extension. (role: original research-audit author and recent area contributor; confidence: high; commits: 569b5c39c783, 15dce3354406; files: scripts/native_eval/research_audit.py, tests/test_native_eval_research_audit.py)
  • scoootscooob: Current main attributes the OpenClaw tool-search configuration boundary to this contributor. (role: adjacent native-evaluation configuration contributor; confidence: medium; commits: 0f1b45e4674b; files: scripts/container_adapter_eval.sh, scripts/container_lane_eval.sh)

Rank-up moves

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

  • Classify absent discovery configuration and events as unobservable and add that regression case.
  • Document the discovery-status and counter-semantics contract for CSV consumers.

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 (73 earlier review cycles; latest 8 shown)
  • reviewed 2026-08-09T12:41:44.062Z sha 15dce33 :: found issues before merge. :: [P2] Preserve absent discovery evidence as unobservable | [P3] Document the discovery-export state contract
  • reviewed 2026-08-09T15:19:18.431Z sha 15dce33 :: found issues before merge. :: [P2] Keep missing discovery-mode evidence unobservable | [P3] Document the discovery-export state contract
  • 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

@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