Skip to content

feat: prototype NVIDIA skill evaluations - #3430

Draft
Patrick-Erichsen wants to merge 17 commits into
mainfrom
pe/nvidia-tier3-clawhub-prototype
Draft

feat: prototype NVIDIA skill evaluations#3430
Patrick-Erichsen wants to merge 17 commits into
mainfrom
pe/nvidia-tier3-clawhub-prototype

Conversation

@Patrick-Erichsen

Copy link
Copy Markdown
Collaborator

Summary

  • prototype the ClawHub sync boundary for allowlisted official NVIDIA skills: unchanged content skips, new content discovers the pinned SkillEvaluator Tier 3 layout and runs the canonical CLI
  • constrain source and evaluator snapshots to exact commits merged into configured official main branches, isolate evaluator subprocess variables, and emit structured provenance plus native result artifacts
  • add an Evals tab directly after SKILL.md that renders SkillEvaluator's structured metrics in the existing skill-detail table style
  • handle completed, pending, skipped/no-evals, and failed local records without introducing a ClawHub scoring rubric or production schema/config

Local smoke demo

  • source: NVIDIA/skills at 0a78f333a1d67c837fbf4288efe6488169dc7140
  • skill: skills/doca-dpa
  • evaluator: NVIDIA/SkillEvaluator 0.1.0 at 4975c97d49e3623eeab739248e52d83c4aa8f582
  • agent/model: Codex with gpt-5.4-mini, one attempt
  • result: 95.9% with skill vs 60.6% baseline; 4/4 vs 2/4 cases passed
  • preview: http://localhost:3000/nvidia/skills/doca-dpa?evaluationDemoCommit=0a78f333a1d67c837fbf4288efe6488169dc7140#evaluation

This proves the local integration path only; it is not a general quality verdict for the skill.

Validation

  • bun run ci:unit — 457 files passed, 1 skipped; 5,994 tests passed, 2 skipped
  • bun run ci:types-build
  • bun run ci:static
  • bunx tsc --noEmit
  • bunx vitest run scripts/prototypes/nvidia-skill-evaluation.test.ts — 21 passed after final lifecycle hardening
  • fresh-checkout unchanged-sync smoke against both pinned NVIDIA commits
  • autoreview: clean, no accepted/actionable findings

Prototype boundary / follow-ups

  • replace local evaluator execution with a production sandbox and explicit credential broker
  • define model credentials, cost/concurrency budgets, retry/timeout policy, and run scheduling
  • persist run state/results durably instead of the local demo artifact index
  • define production result lifecycle and retention before connecting this to the real sync worker

No production/Test deployment, Convex schema, production configuration, or package publication is included.

Visual proof is published in the PR's ClawHub UI Proof comment.

@vercel

vercel Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
clawhub Ready Ready Preview Aug 13, 2026 12:33am

Request Review

@Patrick-Erichsen

Patrick-Erichsen commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator Author

ClawHub UI Proof

Status: passed
Mode: feature
Scenario: NVIDIA SkillEvaluator results on the ClawHub skill page
Provider: local Codex app browser
Baseline: not run for feature proof.
Candidate: 461fe87beb81327cd537c5d881f95c7ef5a227f4

NVIDIA doca-dpa Evals tab — desktop

NVIDIA doca-dpa Evals tab — desktop

Raw proof files: https://github.com/openclaw/clawhub/tree/qa-artifacts/clawhub-ui-proof/pr-3430/2026-08-05-nvidia-evals

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. 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. labels Aug 6, 2026
@Patrick-Erichsen
Patrick-Erichsen force-pushed the pe/nvidia-tier3-clawhub-prototype branch from 5729d38 to 461fe87 Compare August 6, 2026 00:14
@clawsweeper clawsweeper Bot added P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. labels Aug 6, 2026
@clawsweeper

clawsweeper Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed August 12, 2026, 8:34 PM ET / August 13, 2026, 00:34 UTC.

ClawSweeper review

What this changes

This draft PR adds a local NVIDIA SkillEvaluator prototype that records pinned-snapshot results and displays them in a development-only Evals tab for GitHub-backed NVIDIA skills.

Merge readiness

⚠️ Ready for maintainer review - 4 items remain

Keep open for a maintainer security-boundary decision. Current main has no SkillEvaluator feature, while this draft remains a development-only prototype whose local runner can execute an upstream evaluator with model credentials.

Priority: P3
Reviewed head: 0d98e6cc5e924ed9c94112abcb19e1251a3274b4
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The prototype is bounded and well-covered, but current-head runtime proof and a maintainer trust-boundary decision are still needed.
Proof confidence 🦐 gold shrimp (3/6) Not applicable: Real behavior proof is not required for maintainer- or bot-authored pull requests.
Patch quality 🐚 platinum hermit (4/6) Security review found an item that needs attention.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: Real behavior proof is not required for maintainer- or bot-authored pull requests.
Evidence reviewed 6 items Current main does not contain the feature: The PR head is not an ancestor of current main, and a current-main search found no SkillEvaluator, evaluation-report, or demo-commit implementation in the app, backend, or scripts.
Development-only display boundary: The Evals source is constructed only when the app is in development mode and the GitHub source is NVIDIA/skills, so this branch does not expose the tab in a production build.
Local execution and credential boundary: The prototype intentionally allows scoped OpenAI credential variables into a spawned SkillEvaluator process, then invokes validation and evaluation against local snapshots; its own comments defer a production sandbox and credential broker.
Findings None None.
Security Needs attention Decide the evaluator credential boundary: The runner forwards scoped OpenAI and evaluator key variables into a spawned third-party evaluator process. The branch is currently manual and development-only, but sandboxing and a credential broker are required before this becomes sync or production behavior.

How this fits together

ClawHub indexes GitHub-backed skill metadata while installs remain pinned to upstream commits. This prototype evaluates an approved NVIDIA skill snapshot locally, writes local artifacts, and has the development skill-detail page render their metrics.

flowchart LR
  A[Official NVIDIA skill commit] --> B[Prototype approval checks]
  C[Local model credentials] --> D[Local SkillEvaluator run]
  B --> D
  D --> E[Local evaluation artifacts]
  E --> F[Development skill page]
  F --> G[Evals tab]
Loading

Decision needed

Question Recommendation
Should ClawHub retain and eventually ship this evaluator path when its local mode executes approved upstream code with model credentials, or should the work pause until the production sandbox and credential boundary are designed? Pause merge for a production-boundary design: Keep the prototype as research and do not merge or connect it to sync until sandboxing, credential brokerage, lifecycle, retention, and score-policy decisions are documented.

Why: The branch intentionally defers the controls that determine where third-party evaluation code runs, which credentials it can use, and how public results become durable product data; that policy cannot be inferred from code review.

Before merge

  • Resolve security concern: Decide the evaluator credential boundary - The runner forwards scoped OpenAI and evaluator key variables into a spawned third-party evaluator process. The branch is currently manual and development-only, but sandboxing and a credential broker are required before this becomes sync or production behavior.
  • Resolve merge risk (P1) - The manual prototype runs an external evaluator on a developer host while forwarding scoped model credentials; it must not be connected to sync or a production worker before sandboxing and credential brokerage are agreed.
  • Resolve merge risk (P1) - The available browser screenshot is for an earlier candidate rather than the current head, so it does not prove the latest loader and provenance behavior.

Findings

  • [medium] Decide the evaluator credential boundary — scripts/prototypes/nvidia-skill-evaluation.ts:466
Agent review details

Security

Needs attention: The prototype has an explicit but developer-host-only trust boundary: it launches SkillEvaluator with scoped model credentials and therefore needs maintainer approval before any production path is contemplated.

Review metrics

Metric Value Why it matters
Prototype size production +1,875/-2; tests +971/-1 Most of the 12-file change is new local execution, artifact handling, UI rendering, and coverage, so the boundary decision materially affects maintenance scope.

Merge-risk options

Maintainer options:

  1. Hold for containment design (recommended)
    Keep the draft unmerged until the evaluator has an approved sandbox and credential-broker contract rather than relying on a developer-host execution boundary.
  2. Accept the limited prototype boundary
    Maintain the current development-only guard and explicitly prohibit production sync, persistent results, or shared credentials until a separate production design lands.

Technical review

Best possible solution:

Keep the experiment developer-only until maintainers define an isolated execution environment, credential broker, durable result lifecycle, retention, and the product meaning of displayed scores.

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

Not applicable: this is a feature prototype, not a reported bug. A local browser screenshot demonstrates an earlier candidate, but no current-head runtime proof was supplied.

Is this the best way to solve the issue?

No for a production feature: developer-host execution with scoped credentials is useful for exploration but is not the durable isolation and lifecycle design required for sync-backed evaluations.

AGENTS.md: found and applied where relevant.

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

Labels

Label changes:

  • remove proof: sufficient: Current real behavior proof status is not_applicable, not sufficient.
  • remove proof: 📸 screenshot: Current real behavior proof evidence kind is not_applicable.

Label justifications:

  • P3: This is a development-only feature prototype with no current production path or reported user regression.
  • merge-risk: 🚨 security-boundary: The new local runner invokes an external evaluator while explicitly forwarding model credential variables, before the promised sandbox and broker exist.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: Real behavior proof is not required for maintainer- or bot-authored pull requests.

Evidence

Security concerns:

  • [medium] Decide the evaluator credential boundary — scripts/prototypes/nvidia-skill-evaluation.ts:466
    The runner forwards scoped OpenAI and evaluator key variables into a spawned third-party evaluator process. The branch is currently manual and development-only, but sandboxing and a credential broker are required before this becomes sync or production behavior.
    Confidence: 0.96

What I checked:

  • Current main does not contain the feature: The PR head is not an ancestor of current main, and a current-main search found no SkillEvaluator, evaluation-report, or demo-commit implementation in the app, backend, or scripts. (60b02c09f936)
  • Development-only display boundary: The Evals source is constructed only when the app is in development mode and the GitHub source is NVIDIA/skills, so this branch does not expose the tab in a production build. (src/components/SkillDetailPage.tsx:280, 0d98e6cc5e92)
  • Local execution and credential boundary: The prototype intentionally allows scoped OpenAI credential variables into a spawned SkillEvaluator process, then invokes validation and evaluation against local snapshots; its own comments defer a production sandbox and credential broker. (scripts/prototypes/nvidia-skill-evaluation.ts:466, 0d98e6cc5e92)
  • Existing GitHub-backed contract: Current design intent says these catalog entries keep upstream GitHub bytes as the install artifact and use exact commit/content-hash identity, supporting the PR's provenance focus but not a production evaluation lifecycle. (specs/github-backed-skills.md:3, 60b02c09f936)
  • Feature provenance and likely owner: The current GitHub-backed skill model appears to date to Patrick Erichsen's source-sync commit, followed by his recent detail-page and specification work. (src/components/SkillDetailPage.tsx:270, 9cf9e12bdd95)
  • Visual proof was inspected but predates the current head: The downloaded screenshot directly shows the Evals tab and metrics, but its PR proof comment identifies candidate 461fe87; the current 0d98e6 head subsequently changes the evaluator, loader, report, and detail-page files. (0d98e6cc5e92)

Likely related people:

  • Patrick-Erichsen: Introduced the GitHub-backed source-sync model and most recently maintained its detail-page and specification surfaces; the current PR builds on that same boundary. (role: GitHub-backed skill feature owner and recent area contributor; confidence: high; commits: 9cf9e12bdd95, 87ca030c30f3; files: convex/lib/githubSkillSync.ts, src/components/SkillDetailPage.tsx, specs/github-backed-skills.md)

Rank-up moves

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

  • Post redacted browser and local-run proof from the current 0d98e6 head, including the exact NVIDIA snapshot used.
  • Obtain a maintainer decision on sandboxing, credential brokerage, result retention, and whether developer-only prototype code should land.

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 (2 earlier review cycles)
  • reviewed 2026-08-06T00:18:17.889Z sha 461fe87 :: needs maintainer review before merge. :: none
  • reviewed 2026-08-07T00:39:09.362Z sha 1f33b1a :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Aug 7, 2026
@blacksmith-sh

blacksmith-sh Bot commented Aug 13, 2026

Copy link
Copy Markdown

Found 2 test failures on Blacksmith runners:

Failures

Test View Logs
[chromium] › e2e/local-auth/
malicious-skill-ban-flow.pw.test.ts:102:1 › malicious prepublication retries keep the c
lean latest visible
View Logs
e2e/local-auth/malicious-skill-ban-flow.pw.test.ts/
malicious prepublication retries keep the clean latest visible
View Logs

Fix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

@clawsweeper clawsweeper Bot removed proof: sufficient Contributor real behavior proof is sufficient. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. labels Aug 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. 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.

1 participant