Skip to content

docs: design deterministic fuel evidence - #1196

Open
steipete wants to merge 1 commit into
mainfrom
codex/280-perf-design
Open

docs: design deterministic fuel evidence#1196
steipete wants to merge 1 commit into
mainfrom
codex/280-perf-design

Conversation

@steipete

Copy link
Copy Markdown
Contributor

Summary

  • define Crabbox's workflow action-pin Go tests as the first real deterministic workload, with exact wasip1/wasm build and guest commands
  • specify Wasmtime fuel measurement, determinism constraints, and a provider-neutral v1 evidence schema with stability rules
  • recommend a subprocess metering helper, document the runtime/security boundary, and phase implementation through the existing artifact-schema gate before adding product flags
  • cross-link the existing contract from docs: define deterministic perf evidence #857 and resolve the four preconditions in Explore deterministic profiling gates for Crabbox runs #280

This is documentation only. It adds no runtime code, flags, dependencies, release files, or changelog entry.

Verification

Proposed workload compilation:

$ GOTOOLCHAIN=go1.26.5 GOOS=wasip1 GOARCH=wasm go test -c -trimpath -o /tmp/crabbox-scripts.test.wasm ./scripts
$ ls -lh /tmp/crabbox-scripts.test.wasm
-rwxr-xr-x  1 steipete  staff   5.5M Jul 27 20:29 /tmp/crabbox-scripts.test.wasm
$ shasum -a 256 /tmp/crabbox-scripts.test.wasm
9d03b5d0ff1047a45ad490915d88e89b477ae4393ba804558ef436b4d64fb60f  /tmp/crabbox-scripts.test.wasm

Repository gates:

$ gofmt -w $(git ls-files '*.go')
$ go vet ./...
[exit 0]

$ go test -race ./...
ok  github.com/openclaw/crabbox/internal/cli  456.068s
ok  github.com/openclaw/crabbox/scripts       1.208s
[all packages passed; exit 0]

$ scripts/check-docs.sh
checked 56 command docs: command surface ok
checked provider matrix: 79 providers
checked 238 markdown files: internal links ok
built docs site: dist/docs-site
tests 14; pass 14; fail 0
normalized navigation counts across 233 pages; 79 providers

Embedded JSON proof:

parsed 2 JSON blocks; sample invariants hold

Structured review:

$ /Users/steipete/Projects/agent-skills/skills/autoreview/scripts/autoreview --mode local
trufflehog: clean
autoreview clean: no accepted/actionable findings reported
overall: patch is correct (0.99)

Live CLI proof is not applicable because the change is docs-only. The proposed real workload was compiled to a WebAssembly test binary as shown above; executing it under Wasmtime is deliberately phase-one implementation work because Crabbox does not yet ship that runtime helper.

Open decisions

The design leaves the helper distribution channel, whole-module versus exported-function metering, phase-two CLI surface, cross-host proof matrix, and Wasmtime-upgrade versioning policy for maintainer approval before runtime implementation.

@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. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. labels Jul 28, 2026
@clawsweeper

clawsweeper Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 31, 2026, 11:21 AM ET / 15:21 UTC.

ClawSweeper review

What this changes

This PR adds a detailed design for deterministic Wasmtime-fuel evidence and changes the existing future timing-schema example from an instructions counter to measured with unit: "fuel".

Merge readiness

⚠️ Ready for maintainer review - 4 items remain

Keep this PR open for maintainer product-direction review. The supplied context shows a documentation-only proposal that would make a fuel-specific v1 artifact contract and Wasmtime subprocess boundary more concrete, while leaving several compatibility decisions intentionally unresolved; local repository inspection could not run because the read-only sandbox failed before executing commands.

Priority: P3
Reviewed head: 4905737a0c5e62b7ea5c1b4fcc1a9c41d9f8c908
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The proposal is well-scoped and accompanied by claimed documentation validation, but explicit maintainer direction and local source verification remain necessary before merge.
Proof confidence 🌊 off-meta tidepool Not applicable: Real behavior proof is not required because this PR only changes files under docs/.
Patch quality 🦐 gold shrimp (3/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: Real behavior proof is not required because this PR only changes files under docs/.
Evidence reviewed 3 items Repository inspection infrastructure failed: The required first read-only inspection command failed before the shell ran with bwrap: loopback: Failed RTM_NEWADDR: Operation not permitted; current-main source, exact diff, and git history could therefore not be independently verified in this review.
Prior merged contract: The supplied GitHub context identifies merged #857 as the earlier deterministic-performance-evidence contract; this PR builds on that contract but changes the documented metric representation and fills in implementation direction.
Unresolved design choices: The PR body explicitly leaves helper distribution, metering scope, phase-two CLI surface, cross-host proof matrix, and Wasmtime-upgrade versioning for maintainer approval, so the documentation is not a purely editorial change.
Findings None None.
Security None None.

How this fits together

Crabbox runs checks in disposable provider-backed environments and records artifacts for CI-facing evidence. This proposed documentation defines how a future deterministic performance gate would meter a WASI test workload, validate its artifact, and feed that evidence into existing artifact collection.

flowchart LR
  A[Workflow policy tests] --> B[WASI test binary]
  B --> C[Pinned metering helper]
  C --> D[Wasmtime fuel measurement]
  D --> E[Versioned evidence artifact]
  E --> F[Crabbox artifact validation]
  F --> G[Future CI regression gate]
Loading

Decision needed

Question Recommendation
Should Crabbox document measured plus unit: "fuel" and the Wasmtime subprocess boundary as the intended v1 evidence contract before the runtime implementation and upgrade policy are selected? Confirm and merge the v1 design: Approve the fuel terminology and subprocess boundary as the documented direction, with later implementation PRs preserving the stated compatibility rules.

Why: This PR intentionally chooses user- and machine-facing artifact terminology while deferring several compatibility and product-surface decisions; source review alone cannot choose that permanent direction.

Before merge

  • Resolve merge risk (P1) - Merging would make measured/unit: "fuel" and a Wasmtime-oriented metering boundary the documented v1 direction before maintainers decide its long-term artifact and upgrade-compatibility policy.
  • Resolve merge risk (P1) - The sandbox failure prevents an independent check of whether current main has already changed this contract or whether the proposed documentation is fully consistent with adjacent guidance.
  • Complete next step (P2) - A maintainer must choose whether this documentation should establish a permanent future artifact and upgrade contract; no narrow mechanical repair is appropriate.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Documentation scope 2 files affected; 489 additions, 4 deletions Most of the PR is a new design document, but the smaller existing-contract edit changes the documented future JSON representation.

Merge-risk options

Maintainer options:

  1. Confirm the artifact contract before merge (recommended)
    Have the relevant owner explicitly approve the v1 field names, unit semantics, and upgrade policy so later consumers do not inherit an accidental compatibility commitment.
  2. Defer the prescriptive design
    Pause this PR and keep the previously merged contract abstract until a runtime proving slice supplies cross-host and upgrade evidence.

Technical review

Best possible solution:

Confirm the permanent evidence-schema and compatibility direction, then land one coherent documentation contract that distinguishes stable artifact fields from implementation details deferred to a later runtime PR.

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

Not applicable: this is a documentation-only design proposal, not a report of failing current behavior. The supplied context includes compile and repository-gate claims, but local execution and source inspection were unavailable.

Is this the best way to solve the issue?

Unclear: the design is internally presented as phased, but maintainers must first decide whether the proposed fuel-specific artifact vocabulary and runtime boundary are the right permanent contract.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 5aa1c3983874.

Labels

Label changes:

  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp.
  • remove rating: 🐚 platinum hermit: Current PR rating is rating: 🦐 gold shrimp, so this older rating label is no longer current.

Label justifications:

  • P3: This is a forward-looking documentation and design proposal rather than a current user-facing regression.
  • merge-risk: 🚨 compatibility: The proposed v1 JSON field names and unit semantics may become a compatibility commitment for future artifact consumers and upgrades.
  • 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: Real behavior proof is not required because this PR only changes files under docs/.

Evidence

What I checked:

  • Repository inspection infrastructure failed: The required first read-only inspection command failed before the shell ran with bwrap: loopback: Failed RTM_NEWADDR: Operation not permitted; current-main source, exact diff, and git history could therefore not be independently verified in this review.
  • Prior merged contract: The supplied GitHub context identifies merged docs: define deterministic perf evidence #857 as the earlier deterministic-performance-evidence contract; this PR builds on that contract but changes the documented metric representation and fills in implementation direction. (docs/features/deterministic-perf-evidence.md:65, 974d5c6ca1b8)
  • Unresolved design choices: The PR body explicitly leaves helper distribution, metering scope, phase-two CLI surface, cross-host proof matrix, and Wasmtime-upgrade versioning for maintainer approval, so the documentation is not a purely editorial change. (docs/plan/deterministic-perf-evidence.md:1, 4905737a0c5e)

Likely related people:

  • vincentkoc: The supplied related-item history identifies this person as the author of the merged deterministic-performance-evidence contract that this PR extends. (role: feature contract author; confidence: medium; commits: 974d5c6ca1b8; files: docs/features/deterministic-perf-evidence.md)

Rank-up moves

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

  • Record an explicit maintainer decision on the v1 artifact fields, metering boundary, and upgrade policy.
  • Refresh the review after a working read-only environment can compare the proposal against current main documentation.

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 (6 earlier review cycles)
  • reviewed 2026-07-28T03:54:34.658Z sha 4905737 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-30T01:14:16.245Z sha 4905737 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-30T11:53:04.789Z sha 4905737 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-30T13:38:01.941Z sha 4905737 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-30T18:26:27.401Z sha 4905737 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-31T06:31:40.655Z sha 4905737 :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. 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: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jul 30, 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. 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.

1 participant