Skip to content

fix: audit nested ATIF research evidence and coverage - #85

Draft
vincentkoc wants to merge 1 commit into
mainfrom
fix/nested-atif-research-acceptance
Draft

vincentkoc wants to merge 1 commit into
mainfrom
fix/nested-atif-research-acceptance

Conversation

@vincentkoc

Copy link
Copy Markdown
Member

What does this PR do?

Extends the existing native research audit to consume unique nested ATIF trajectories, preserve session/tool lineage, verify receipt integrity and node coverage, and report independent evidence facets. This is an archive-consumer change, not a new execution engine or a generic Harbor schema change.

Fixes #84. Related: #48 (task analysis and upload) and #60 (discovery telemetry). Those features and branches are unchanged. The export-loop edits may require normal reconciliation with #60 when landing.

Why?

The current audit inspects only root steps/models, falls through explicit zero usage, and calls runtime-reported costs exact without billing reconciliation. Nested child/grandchild traces can therefore disappear from the model check and research rows.

Provenance: the root-only consumer, zero fallback, and exact cost labels originated in #51, authored and merged by @vincentkoc on 2026-07-29 (569b5c39c7831347ad37583cbb8251c5238fcfdd). The nested exporter makes the traversal gap visible. This does not claim the historical flattened converter always lost child identity.

Changes

  • enumerate unique embedded node identities; retain all parent edges and source tool-call IDs, including unresolved/external references without following their paths
  • audit node defaults and per-step model switches; retain provider-qualified model annotations and distinguish step evidence from trajectory defaults
  • verify the actual trajectory bytes against the adjacent v1 receipt, root identity, unique session/key/leaf coverage, diagnostics, and opaque wrappers
  • preserve root, node, receipt-family, and observed-step metrics separately; never sum overlapping scopes or allocate family totals to models
  • add trajectory_nodes.csv, trajectory_links.csv, and model_usage_coverage.csv; preserve missing values versus real zero and report per-component coverage denominators
  • expose independent capture, execution, identity, token, cost, resource, and reward evidence, including legitimate zero rewards
  • relabel runtime costs reported_*; retain deprecated exact-count keys as zero and add reported-cost counts
  • document schema version 2, joins, migration, evidence scope, and unavailable proof

Production LOC: +607/-62 (net +545). Tests: +468/-5. The added evidence module owns graph reconciliation and independent evidence checks; it does not duplicate the runner, ATIF converter, task analysis, discovery parser, or archive uploader.

Tests

  • python -m pytest -q — 553 passed, 5 skipped (baseline: 527 passed, 5 skipped)
  • python -m ruff check clawbench app.py scripts tests
  • repository pre-commit hooks for all six changed files
  • five focused regression tests failed on the original consumer before implementation; all 28 focused tests now pass
  • direct compatibility probe against four public exporter golden fixtures: all raw-byte digests and node sets match; the SQLite ACP wrapper correctly remains partial
  • public diff uses synthetic fixtures only; no private task, archive, fleet, credential, or campaign data

Best-fix review and limits

Best-fix verdict: owner-boundary consumer fix. Changing generic Harbor schemas would affect unrelated agents without repairing these CSV/identity paths. Flattening the exporter would discard the hierarchy that the consumer needs. Folding this into #48 or #60 would mix distinct analysis/upload/discovery features.

Code read: research audit, aggregate acceptance boundary, native runtime/execution outcome, harness trajectory conversion, current research runbook, and public exporter mapper/schema/receipt contracts. The current exporter main at 4816624739db14ddd54de2403c59fe8e79deadd3 has unchanged mapper/receipt implementations relative to the inspected public fixture source 7dd7bcc6ec958e61e08e70d1c1df2d185b3a5642; the fixture probe is not a packaged/live run.

This draft is not live campaign qualification. Capture is bounded to selected public exported generations; observed agent steps are not a complete provider-request ledger. Runtime costs remain billing-unreconciled. Judge/reasoning identity, authenticated delegated lifecycle, resource-host measurements, archive restore, and a new scored campaign have not been run. Resource acceptance stays explicitly unavailable until a validated resource artifact contract exists. No provider calls, paid infrastructure, release, or merge is included.

@clawsweeper

clawsweeper Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. 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. labels Sep 21, 2026
@clawsweeper

clawsweeper Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

Codex review: blocked before merge. Reviewed October 4, 2026, 2:22 PM ET / 18:22 UTC (Revision 2).

ClawSweeper review

What this changes

Extends ShellBench’s archived agent-trace audit to include nested sessions, validate export receipts, preserve zero usage, and report independent evidence coverage.

Merge readiness

⛔ Blocked before merge - 2 items remain

This PR addresses defects still present on current main and remains the candidate fix for the linked issue. No discrete introduced bug was found, but schema-v2 downstream upgrade compatibility remains unverified.

Priority: P2
Reviewed head: 6847e10126268279d0c5b22c82f45e6d65452e76

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The consumer repair is focused and well covered, with readiness limited by unverified downstream export compatibility.
Proof confidence 🌊 off-meta tidepool Not applicable: The member-authored PR is exempt from ordinary contributor live-proof requirements. Its reported golden-fixture probe addresses receipt compatibility, but does not establish downstream schema-v2 upgrade compatibility; legacy reading is covered only by synthetic tests.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The member-authored PR is exempt from ordinary contributor live-proof requirements. Its reported golden-fixture probe addresses receipt compatibility, but does not establish downstream schema-v2 upgrade compatibility; legacy reading is covered only by synthetic tests.
Evidence reviewed 9 items Current-main necessity: At the fetched main revision, the audit still reads root steps and models, uses truthiness for token fallback, and labels runtime costs exact. The central requested repair is therefore still absent.
Introduced scope and ownership: Read the pinned merge-base-to-head change and corresponding source. It changes two production Python files, two test files, and two documentation files; execution, aggregation eligibility, dependencies, and infrastructure are untouched.
Compatibility boundary: Schema version 2 requires node-aware turn/tool joins and replaces exact cost classifications with reported classifications while retaining deprecated exact-count keys as zero. The legacy flat-trace test preserves basic reading, but neither it nor the reported exporter-fixture probe demonstrates adaptation of existing downstream consumers.
Findings None None.
Security None None.

How this fits together

ShellBench’s research audit reads archived benchmark results and agent trajectories, then exports tables for model identity, tool use, usage, and research evidence. These tables inform downstream analysis without changing execution or leaderboard eligibility.

flowchart TD
  A[Archived benchmark results] --> C[Research audit]
  B[Nested agent traces] --> C
  D[Export receipts] --> E[Integrity and coverage checks]
  C --> E
  E --> F[Identity and evidence statuses]
  C --> G[Research CSV tables]
  F --> G
Loading

Before merge

  • Resolve merge risk (P1) - Existing consumers joining turn/tool rows by task and turn, or interpreting exact-cost classifications and counts, must adapt to schema v2; no downstream upgrade validation is supplied.
  • Complete next step (P2) - Provide legacy-archive re-export and downstream schema-v2 upgrade evidence covering node-aware joins and reported-cost replacements.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test delta production +607/-62; tests +468/-5 Production growth is justified by graph reconciliation and independent evidence checks, with separate regression coverage.
Export contract schema v2; 3 new CSV tables Downstream consumers must distinguish node-local turns and reported costs.

Merge-risk options

Maintainer options:

  1. Validate the export upgrade (recommended)
    Provide a legacy-archive re-export and downstream consumer check covering node-aware joins and the reported-cost replacements.

Technical review

Best possible solution:

Keep the repair in the archive consumer and demonstrate that legacy archives and downstream analysis can transition safely to node-aware schema-v2 tables.

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

Yes, source establishes the current-main root-only traversal and explicit-zero fallback defects; nested traces and zero-valued harness usage are concrete triggers. No execution was performed in this read-only review.

Is this the best way to solve the issue?

Yes, repairing the archive consumer is the appropriate boundary, and no introduced correctness defect was found. Downstream schema-v2 upgrade validation is still needed.

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning medium; reviewed against e3f8d25a01f4.

Labels

Label changes:

  • add merge-risk: 🚨 compatibility: Schema-v2 node-local joins and cost classifications change how existing exported tables must be consumed.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • remove rating: 🐚 platinum hermit: Current PR rating is rating: 🦐 gold shrimp, so this older rating label is no longer current.

Label justifications:

  • P2: This is a bounded research-audit correctness repair without an urgent runtime or channel outage.
  • merge-risk: 🚨 compatibility: Schema-v2 node-local joins and cost classifications change how existing exported tables must be consumed.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The member-authored PR is exempt from ordinary contributor live-proof requirements. Its reported golden-fixture probe addresses receipt compatibility, but does not establish downstream schema-v2 upgrade compatibility; legacy reading is covered only by synthetic tests.

Evidence

What I checked:

  • Current-main necessity: At the fetched main revision, the audit still reads root steps and models, uses truthiness for token fallback, and labels runtime costs exact. The central requested repair is therefore still absent. (scripts/native_eval/research_audit.py:160, e3f8d25a01f4)
  • Introduced scope and ownership: Read the pinned merge-base-to-head change and corresponding source. It changes two production Python files, two test files, and two documentation files; execution, aggregation eligibility, dependencies, and infrastructure are untouched. (scripts/native_eval/research_audit.py:466, 6847e1012626)
  • Compatibility boundary: Schema version 2 requires node-aware turn/tool joins and replaces exact cost classifications with reported classifications while retaining deprecated exact-count keys as zero. The legacy flat-trace test preserves basic reading, but neither it nor the reported exporter-fixture probe demonstrates adaptation of existing downstream consumers. (docs/native_research_evidence.md:9, 6847e1012626)
  • Regression coverage inspected: Inspected synthetic coverage for nested models, duplicate identities, raw-byte digest mismatch, unresolved references, missing versus zero usage, independent execution/reward evidence, and legacy flat traces. Tests were not executed during this read-only review; the body reports 553 passed and 5 skipped. (tests/test_native_eval_research_audit.py:15, 6847e1012626)
  • Affirmative exporter dependency: The new consumer explicitly interprets openclaw-atif-receipt-v1, including its digest, root, node identities, diagnostics, and relationships. This establishes a dependency on the OpenClaw ATIF export contract, rather than on the Codex runtime. (scripts/native_eval/research_evidence.py:275, 6847e1012626)
  • Exporter contract verified: Verified repository ownership through GitHub metadata and inspected the pinned receipt interface, mapper, SQLite golden trajectory, and receipt. Their nested trajectory, session/key/leaf identity, metrics, and opaque-wrapper fields agree with the consumer’s intended boundary. Read the dependency’s full root guidance; no dependency release or execution was attempted. (src/models/receipt.ts:11, 4816624739db)

Likely related people:

  • Vincent Koc: Raw commit 569b5c3 adds scripts/native_eval/research_audit.py:200 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 569b5c39c783; files: scripts/native_eval/research_audit.py)

Rank-up moves

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

  • Show a legacy-archive re-export and downstream schema-v2 consumer check for node-aware joins and reported-cost keys.

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 (1 earlier review cycle)
  • reviewed 2026-09-21T14:33:06.825Z sha 6847e10 :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added 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. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Oct 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. 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.

Audit nested ATIF families and independent research evidence coverage

1 participant