feat(challenge-sdk-wasm): add term-challenge types, sandbox host functions, and extended trait - #41
Conversation
…ypes and host functions Add term-challenge evaluation model support to the WASM guest SDK, enabling challenge authors to define tasks, execute sandbox commands, and report per-task scoring breakdowns. New types (types.rs): TaskDefinition, SandboxExecRequest, SandboxExecResponse, TaskResult, and TermEvaluationParams provide no_std-compatible structures for task-based evaluation workflows. EvaluationOutput gains a `details` field for serialized per-task result breakdowns. A score_f64_scaled() helper converts f64 scores to i64 via fixed-point scaling (×10000). New host functions (host_functions.rs): Declares sandbox_exec, get_timestamp, and log_message under the platform_sandbox WASM import module, with safe Rust wrappers host_sandbox_exec, host_get_timestamp, and host_log. Extended Challenge trait (lib.rs): Adds optional tasks() and configure() methods with default implementations. The register_challenge! macro now exports get_tasks and configure as additional WASM ABI entry points. Allocator fix (alloc_impl.rs): Adds allocate(size, _align) as a no_mangle alias for alloc() to fix ABI mismatch where the validator calls with two parameters. ARENA_SIZE is now configurable via the large-arena feature flag (4 MiB when enabled, 1 MiB default). Cargo.toml: Adds default and large-arena feature flags.
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThe pull request extends the WASM challenge SDK with sandbox execution infrastructure, task management capabilities, and configurable memory allocation. New types define task structures, sandbox request/response formats, and evaluation parameters. The Challenge trait gains Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
crates/challenge-sdk-wasm/src/alloc_impl.rs (1)
57-60: Silently ignoring_alignmay mask future bugs.The PR notes the validator currently passes
0for alignment, so delegating toalloc(hardcoded align=8) is fine today. However, if a future host ever supplies a real alignment value, this will silently use 8-byte alignment instead, which could lead to subtle memory-alignment issues. Consider at minimum a debug assertion or a comment in the function body documenting why_alignis intentionally ignored.💡 Suggested documentation
#[no_mangle] pub extern "C" fn allocate(size: i32, _align: i32) -> i32 { + // NOTE: _align is intentionally ignored. The validator currently passes 0; + // `alloc` always uses 8-byte alignment which satisfies all WASM types. alloc(size) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/challenge-sdk-wasm/src/alloc_impl.rs` around lines 57 - 60, The allocate function currently ignores the _align parameter and always calls alloc(size) (which uses a hardcoded 8-byte alignment); add a short justification and safety check: either add a comment explaining that hosts currently pass 0 alignment and 8-byte alignment is intentional, or add a debug assertion (e.g., debug_assert!(_align == 0 || _align == 8)) to catch unexpected non-zero align values in debug builds, and keep the existing call to alloc(size) to preserve current behavior; update the allocate function (symbol: allocate) to include this comment/assertion referencing alloc so future changes are explicit.crates/challenge-sdk-wasm/src/lib.rs (1)
30-35:configure()returns()— no way to signal failure to the host.
setup_environment(line 27) returnsboolto indicate success/failure, and the ABI wrapper (line 178) propagates it. In contrast,configurereturns(), and the ABI wrapper (line 207) unconditionally returns1. If a challenge implementation encounters bad config, it cannot communicate this to the host.If configuration can fail, consider aligning with
setup_environment:💡 Suggested change
- fn configure(&self, _config: &[u8]) {} + fn configure(&self, _config: &[u8]) -> bool { true }And in the macro:
#[no_mangle] pub extern "C" fn configure(config_ptr: i32, config_len: i32) -> i32 { let slice = unsafe { core::slice::from_raw_parts(config_ptr as *const u8, config_len as usize) }; - <$ty as $crate::Challenge>::configure(&_CHALLENGE, slice); - 1 + if <$ty as $crate::Challenge>::configure(&_CHALLENGE, slice) { + 1 + } else { + 0 + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/challenge-sdk-wasm/src/lib.rs` around lines 30 - 35, The configure(&self, _config: &[u8]) function currently returns () so callers/host cannot know if configuration failed; change its signature to return a bool (or other simple failure indicator) like setup_environment does, update the challenge implementation(s) to return false on bad config, and update the macro-generated ABI wrapper that currently unconditionally returns 1 to propagate the configure return value (i.e., convert the bool into the ABI return) so failures are visible to the host; ensure you update any places referencing configure (and the macro that emits the ABI wrapper) to match the new signature.crates/challenge-sdk-wasm/src/types.rs (1)
55-57:score_f64_scaledsilently converts NaN to 0 and saturates on infinity.The
as i64cast is well-defined in Rust (saturating semantics since 1.45), but NaN inputs silently become a score of0and±Infinitysaturates toi64::MAX/i64::MIN. In a scoring context, these likely indicate upstream bugs that should surface as errors rather than be silently accepted.💡 Suggested defensive variant
pub fn score_f64_scaled(value: f64) -> i64 { + debug_assert!(value.is_finite(), "score_f64_scaled received non-finite value: {}", value); (value * 10_000.0) as i64 }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/challenge-sdk-wasm/src/types.rs` around lines 55 - 57, The function score_f64_scaled currently casts with "as i64" which silently maps NaN to 0 and infinities to i64::MIN/MAX; change score_f64_scaled to validate the input first (use value.is_nan() and value.is_finite()) and return a Result<i64, E> (or Option<i64>) instead of silently converting, returning an Err with a clear message for NaN or ±Infinity; multiply by 10_000.0 only after validating and then perform the safe cast (or saturating/clamping explicitly if you prefer exact semantics) so callers can handle invalid upstream scores.crates/challenge-sdk-wasm/src/host_functions.rs (1)
187-217: New sandbox host functions follow established patterns — looks good.The implementations of
host_sandbox_exec,host_get_timestamp, andhost_logare consistent with the existing host function wrappers in this file.One point worth noting:
host_get_timestamp()(line 211, viaplatform_sandbox) coexists withhost_get_time()(line 175, viaplatform_terminal). Consider adding a brief doc comment to each clarifying which context they serve, so downstream challenge authors pick the right one.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/challenge-sdk-wasm/src/host_functions.rs` around lines 187 - 217, Add brief doc comments to distinguish the two time-related host functions so users know which context to use: annotate host_get_time() (platform_terminal) and host_get_timestamp() (platform_sandbox) with one-line comments describing their source (platform_terminal vs platform_sandbox) and intended usage (e.g., terminal-restricted vs sandbox host-provided timestamp) placed directly above each function definition to make the difference clear to downstream challenge authors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/challenge-sdk-wasm/src/alloc_impl.rs`:
- Around line 57-60: The allocate function currently ignores the _align
parameter and always calls alloc(size) (which uses a hardcoded 8-byte
alignment); add a short justification and safety check: either add a comment
explaining that hosts currently pass 0 alignment and 8-byte alignment is
intentional, or add a debug assertion (e.g., debug_assert!(_align == 0 || _align
== 8)) to catch unexpected non-zero align values in debug builds, and keep the
existing call to alloc(size) to preserve current behavior; update the allocate
function (symbol: allocate) to include this comment/assertion referencing alloc
so future changes are explicit.
In `@crates/challenge-sdk-wasm/src/host_functions.rs`:
- Around line 187-217: Add brief doc comments to distinguish the two
time-related host functions so users know which context to use: annotate
host_get_time() (platform_terminal) and host_get_timestamp() (platform_sandbox)
with one-line comments describing their source (platform_terminal vs
platform_sandbox) and intended usage (e.g., terminal-restricted vs sandbox
host-provided timestamp) placed directly above each function definition to make
the difference clear to downstream challenge authors.
In `@crates/challenge-sdk-wasm/src/lib.rs`:
- Around line 30-35: The configure(&self, _config: &[u8]) function currently
returns () so callers/host cannot know if configuration failed; change its
signature to return a bool (or other simple failure indicator) like
setup_environment does, update the challenge implementation(s) to return false
on bad config, and update the macro-generated ABI wrapper that currently
unconditionally returns 1 to propagate the configure return value (i.e., convert
the bool into the ABI return) so failures are visible to the host; ensure you
update any places referencing configure (and the macro that emits the ABI
wrapper) to match the new signature.
In `@crates/challenge-sdk-wasm/src/types.rs`:
- Around line 55-57: The function score_f64_scaled currently casts with "as i64"
which silently maps NaN to 0 and infinities to i64::MIN/MAX; change
score_f64_scaled to validate the input first (use value.is_nan() and
value.is_finite()) and return a Result<i64, E> (or Option<i64>) instead of
silently converting, returning an Err with a clear message for NaN or ±Infinity;
multiply by 10_000.0 only after validating and then perform the safe cast (or
saturating/clamping explicitly if you prefer exact semantics) so callers can
handle invalid upstream scores.
1 participant
Summary
Extends the WASM Guest SDK (
challenge-sdk-wasm) with types, host function declarations, and trait methods needed to support terminal-based challenge evaluation workflows.Changes
New types (
types.rs)TaskDefinition— describes a single evaluation task (id, description, command, expected output, timeout, scoring criteria)SandboxExecRequest/SandboxExecResponse— structured request/response for executing commands in the host sandboxTaskResult— per-task evaluation result with pass/fail, score, and optional output/metricsTermEvaluationParams— structured deserialization target forEvaluationInput.paramsdetails: Option<Vec<u8>>field toEvaluationOutputfor carrying per-task result breakdownsscore_f64_scaled()helper to convert f64 scores to i64 with fixed-point scaling (×10000)Host function declarations (
host_functions.rs)platform_sandboximport module withextern "C"declarations:sandbox_exec— execute a command in the host sandboxget_timestamp— retrieve current timestamp for benchmarkinglog_message— structured logging from WASM guest to hosthost_sandbox_exec,host_get_timestamp,host_logExtended
Challengetrait (lib.rs)tasks()method (returns serialized task list, default empty)configure()method (accepts config bytes, default no-op)register_challenge!macro to exportget_tasksandconfigureas additional WASM ABI functionsAllocator fix (
alloc_impl.rs)allocate(size, _align)as a#[no_mangle]alias foralloc()to fix ABI mismatch where the validator callsallocate(size, 0)with two parametersARENA_SIZEconfigurable: default 1 MiB,large-arenafeature flag for 4 MiBCargo.toml
defaultandlarge-arenafeature flagsSummary by CodeRabbit
Release Notes