0xplaygrounds/.claude/skills/rig-review

<!-- This file is generated by scripts/sync_agent_instruction_files.sh. --> <!-- Do not edit this file directly; update AGENTS.md and re-run the sync script. -->

---

name: rig-review description: > Review Rig changes against mandatory quality gates before opening a PR. Use before submitting code to verify error handling, WASM compatibility, coding style, and required checks pass. allowed-tools:

• Read
• Glob
• Grep
• Bash

---

Rig Pre-PR Review

Perform a Rig-focused pre-PR review.

Required output format:

1. List blocking issues first with file and line references.
2. List medium/low-risk issues second.
3. Confirm required checks and test evidence.
4. Explicitly state whether public API changed.
5. If no issues are found, say so and note residual risk.

rig-prompt-hooks

Prompt hooks allow observing and controlling the agent's execution lifecycle.

The PromptHook Trait

PromptHook<M> provides callbacks for both streaming and non-streaming requests. All methods have default implementations — implement only what you need.

Hook Callbacks

• on_completion_call - Before sending prompt to model (returns HookAction)
• on_completion_response - After receiving response (returns HookAction)
• on_tool_call - Before invoking a tool (returns ToolCallHookAction)
• on_tool_result - After tool returns result (returns HookAction)
• on_text_delta - During streaming, on each text chunk (returns HookAction)
• on_tool_call_delta - During streaming, on each tool call chunk (returns HookAction)

Control Flow

• ToolCallHookAction::Continue (or .cont()) - Allow tool execution
• ToolCallHookAction::Skip { reason } (or .skip("reason")) - Reject tool; reason becomes tool result
• ToolCallHookAction::Terminate { reason } (or .terminate("reason")) - Terminate the agentic loop early
• HookAction::Continue (or .cont()) - Continue normal execution
• HookAction::Terminate { reason } (or .terminate("reason")) - Terminate the agentic loop early

Design Rationale

1. Default implementations - All methods have defaults; implement only what you need
2. Per-request hooks - Attached via .with_hook(), not globally
3. WASM compatible - Uses WasmCompatSend/WasmCompatSync bounds

rig-quality-gates

Trait Bounds

Use full where clause syntax for readability:

// Correct
impl<T> CompletionModel for MyModel<T>
where
    T: HttpClientExt + Clone + WasmCompatSend + Debug + Default + 'static,
{
    // ...
}

// Avoid inline bounds for complex signatures
impl<T: HttpClientExt + Clone + WasmCompatSend + Debug + Default + 'static> CompletionModel for MyModel<T> {
    // ...
}

Error Handling

DO NOT use String as an error type. Define proper error enums:

// Correct
#[derive(Debug, thiserror::Error)]
pub enum MyError {
    #[error("Failed to parse response: {0}")]
    ParseError(#[from] serde_json::Error),

    #[error("Provider returned error: {0}")]
    ProviderError(String),
}

// Absolutely forbidden
fn do_thing() -> Result<(), String>  // NO

DO NOT stub out error handling:

// Forbidden
let result = fallible_operation().unwrap();  // NO
let result = fallible_operation().expect("this should work");  // NO unless truly impossible

// Correct
let result = fallible_operation()?;
let result = fallible_operation().map_err(MyError::from)?;

Comments

Comments explain WHY, not WHAT.

If you need to explain what code is doing, the code itself is unclear. Rename variables, extract functions, or restructure.

// Bad - explains what
// Increment counter by one
counter += 1;

// Bad - explains what
// Check if user is admin
if user.role == Role::Admin {

// Good - explains why
// Rate limiting resets at midnight UTC, so we need the day boundary
let boundary = timestamp.truncate_to_day();

// Good - explains non-obvious business logic
// Providers may return tool calls split across multiple chunks,
// so we accumulate them by index until the stream ends

Documentation

• Add doc comments (///) to all public items
• Add module-level documentation (//!) to modules
• Include usage examples in doc comments where helpful

TODO Items

TODO items are not allowed in submitted code.

If you need a TODO, the implementation is incomplete. Either:

1. Complete the implementation
2. Don't submit it yet
3. Create a separate issue tracking the future work

// Forbidden
fn process() {
    // TODO: handle edge case
}

// Forbidden
fn process() {
    unimplemented!("will add later")
}

---

Before Submitting

Required Checks

Run these commands and fix all issues before submitting:

cargo fmt
cargo clippy --all-targets --all-features
cargo test

If you cannot run these commands (e.g., environment limitations), explicitly ask the user to run them before the PR is submitted.

Architectural Changes

If your change involves:

• New traits or significant trait modifications
• Changes to the client/provider architecture
• New abstractions or patterns
• Breaking changes to public APIs

Discuss with the user first. Do not implement major architectural changes without explicit approval. Open an issue for discussion if needed.

Self-Review Checklist

Before considering code complete:

• [ ] All error cases handled properly (no .unwrap() or .expect() on fallible operations unless truly impossible)
• [ ] No String error types
• [ ] No TODO comments
• [ ] No stubbed implementations
• [ ] Comments explain "why", not "what"
• [ ] Uses WasmCompatSend/WasmCompatSync instead of Send/Sync
• [ ] Follows existing code patterns in the codebase
• [ ] Includes tests or examples that compile
• [ ] cargo fmt passes
• [ ] cargo clippy --all-targets --all-features passes
• [ ] cargo test passes

---

Commits

DO NOT MAKE COMMITS UNLESS THE USER HAS ASKED YOU TO DO SO. Users should be able to manually verify that what you have done has worked before proceeding with a commit, and may also want to write their own commit messages.

If the PR contains breaking changes, the PR message must list the public API items that have broken and the migration path for each.

What Will Be Rejected

PRs will be rejected if they contain:

• Lazy workarounds - String error types, .unwrap() everywhere, incomplete handling
• TODO items - Submit complete work or don't submit
• Stubbed error handling - Every error path must be properly handled
• Provider API divergence - Don't add fields/features that don't exist in real provider APIs
• Missing WASM compatibility - Using Send/Sync instead of WasmCompat* variants
• Unclear code requiring explanatory comments - Refactor instead
• Architectural changes without discussion - Major changes need approval first

Remember: The quality bar exists because Rig is used in production by many projects. Every contribution must maintain that standard.