chore: add Claude Code rules, hooks, agents, and skills#16
Merged
Conversation
Add comprehensive coding rules and GPU/CUDA acceleration guidelines to CLAUDE.md. Configure hooks for format checking and sensitive file protection. Add 3 new agents (unsafe-auditor, redis-compat-checker, gpu-arch-reviewer) and 8 new skills (unsafe-audit, redis-compat, add-command, gpu-bench, simd-bench, cuda-scaffold, check-hotpath, dual-runtime-check).
📝 WalkthroughWalkthroughThis PR adds Claude AI agent instructions and skill definitions for the moon project, establishing structured workflows for GPU architecture review, Redis compatibility verification, unsafe code auditing, CUDA scaffolding, hot-path analysis, benchmarking, command implementation, and dual-runtime validation. Additionally, coding rules are defined in CLAUDE.md covering allocations, locks, error handling, GPU/CUDA constraints, and command checklists. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 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.
Actionable comments posted: 14
🧹 Nitpick comments (3)
.claude/skills/add-command/SKILL.md (1)
47-49: Error-string rule is overly strict relative to existing helpers.Line 48 should not blanket-ban formatted construction when helper-based dynamic messages are already part of command error handling. Recommend: require exact Redis-compatible text and prefer shared helpers (
err_wrong_args, etc.), usingBytes::from_static()only for true literals.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/add-command/SKILL.md around lines 47 - 49, Relax the blanket rule: require that error texts match Redis exactly and that command implementations prefer shared helpers like err_wrong_args, err_syntax_error, etc., but allow formatted construction when those helpers produce dynamic content; only mandate Bytes::from_static() for true fixed-literal error strings (not for helper-derived strings), and ensure all error returns use Frame::Error (never panic) — update docs and examples to show using helper functions for canonical messages, Bytes::from_static() for literal bytes, and returning Frame::Error in functions such as handle_command / execute_command where errors are produced..claude/skills/unsafe-audit/SKILL.md (1)
23-26:NEW unsafe vs maincommand is a bit brittle; consider a more robust diff filter.Using
git diff main+grep '+.*unsafe'can include noisy matches. Consider tightening this step to added Rust source lines only (excluding diff headers/context) for cleaner audit output.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/unsafe-audit/SKILL.md around lines 23 - 26, The current "NEW unsafe vs main" step uses the command "git diff main -- '*.rs' | grep '+.*unsafe'" which can match diff headers and context; update it to only scan added Rust source lines by using git diff with a filter for added/modified files (diff-filter A/M) and zero context (unified=0) and then only match lines that begin with a single "+" (i.e. exclude "+++" file header lines) when searching for "unsafe"; replace the brittle pipeline in SKILL.md with this tighter diff-filtering approach so the audit output only shows newly added Rust unsafe lines..claude/skills/check-hotpath/SKILL.md (1)
31-35: Reduce false positives by explicitly targetingstd::synclock usage.Given Line 34’s intent (“flag
std::synclock”), narrowing the pattern tostd::sync::{Mutex,RwLock}usage would keep results focused and actionable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/check-hotpath/SKILL.md around lines 31 - 35, Update the grep pattern in the SKILL.md check that currently searches for "Mutex::new\|RwLock::new\|\.lock()\|\.read()\.\|\.write()\." so it only flags usages of the standard library locks; specifically match "std::sync::Mutex" and "std::sync::RwLock" (and their associated ".lock()", ".read()", ".write()" calls) instead of any Mutex/RwLock symbol to reduce false positives; modify the quoted command string referenced in the diff (the grep line) to include "std::sync::Mutex" and "std::sync::RwLock" tokens (and/or use a regex like "std::sync::Mutex(::new)?|std::sync::RwLock(::new)?|std::sync::Mutex::lock|std::sync::RwLock::read|std::sync::RwLock::write") so the check only targets std::sync lock usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/agents/gpu-arch-reviewer.md:
- Line 28: The criterion text "CPU path must produce bit-identical results
(within f32 epsilon)" is internally inconsistent; pick one standard and update
the sentence accordingly: either require true bit-identical equality (remove any
mention of f32 epsilon) or require numerical equality within a defined float32
tolerance (replace the phrase with e.g. "CPU path must produce results equal
within float32 tolerance: max absolute error <= 1.1920929e-7" and state whether
you use max absolute or relative error). Edit the existing phrase "CPU path must
produce bit-identical results (within f32 epsilon)" to the chosen wording and
include the exact epsilon and error metric so reviewers have a single,
unambiguous correctness rule.
In @.claude/agents/unsafe-auditor.md:
- Around line 26-31: The fenced code block in .claude/agents/unsafe-auditor.md
that currently starts with ``` should be updated to include a language tag (for
example change ``` to ```text) to satisfy markdownlint MD040; locate the block
containing the lines "[SOUND|UNSOUND|NEEDS_REVIEW] file.rs:line" and add an
appropriate language identifier (e.g., text) after the opening backticks so the
linter recognizes the fence language.
In @.claude/settings.json:
- Line 24: The hook command hardcodes an absolute path (the cd
/Users/tindang/... in the "command" string) so cargo fmt --check won't run on
other machines/CI; change the command to compute the repo root dynamically
(e.g., use git rev-parse --show-toplevel or $PWD/GITHUB_WORKSPACE) and run cargo
fmt --check from that root only when FILE_PATH ends with .rs (the FILE_PATH
extraction, grep '\.rs$', and cargo fmt --check invocation should remain but
remove the hardcoded cd path), ensuring the formatter runs portably for all
contributors.
- Line 28: The current hook command that greps TOOL_RESULT for the substring
"unsafe" is too broad; change it so it only flags newly added unsafe usages in
edited Rust files and only when a nearby "// SAFETY:" comment is missing.
Replace the simple grep of TOOL_RESULT with a two-step check: (1) compute the
git diff for edited/added *.rs files and search only added lines (those starting
with "+") for the token "unsafe", and (2) for each added unsafe occurrence
verify there is a nearby "// SAFETY:" comment (e.g., same hunk or preceding
context) before producing the warning; keep references to TOOL_RESULT, the
"unsafe" token, and the "// SAFETY:" marker so the hook logic targets just new
unsafe additions without existing safety comments.
In @.claude/skills/add-command/SKILL.md:
- Line 15: Update the skill documentation to stop instructing edits to a PHF
dispatch table and instead point contributors to the real match-based dispatch
in the command module (e.g., the match statement inside command::mod that routes
commands); change the references on lines 15 and 33 to instruct editing the
appropriate match arms in command::mod (or the function that performs command
dispatch) rather than a PHF table, and clarify how to add a new arm/branch and
accompanying handler function name to keep the skill aligned with the actual
code architecture.
- Around line 22-23: The handler template signature is wrong: update the
function `cmd_command_name` to match the real command handler API by switching
the parameter order and types to `fn cmd_command_name(db: &mut Database, args:
&[Frame]) -> Frame` (use `Frame` for args instead of `Bytes`) so it accepts a
mutable `Database` first and an args slice of `Frame` second; adjust any
internal references to the parameters accordingly.
In @.claude/skills/check-hotpath/SKILL.md:
- Around line 31-34: The grep in SKILL.md currently uses patterns like
`\.read()\.\|\.write()\.` that include trailing literal dots and miss common
callsites; update the command to match read/write/lock calls without requiring a
trailing dot and prefer extended regex. Replace the single-quoted pattern with
an extended-regex invocation (e.g. use grep -En) and patterns such as
`Mutex::new|RwLock::new|\.lock\(|\.read\(|\.write\(` so the search in the file
containing the grep line will correctly flag `.read()`, `.write()`, `.lock()`
and variants held across `.await`. Ensure the updated line references the same
file globs (`src/command/ src/shard/event_loop.rs src/io/`) as before.
- Around line 23-47: The grep commands used to detect hot-path violations
currently omit some hot-path files (e.g., src/shard/spsc_handler.rs and
src/storage/dashtable/*.rs), so expand each search invocation to include all
declared hot-path targets; update the three pattern searches (allocation
patterns:
Box::new|Vec::new()|String::new()|Arc::new|.clone()|format!|.to_string()|.to_owned(),
lock patterns: Mutex::new|RwLock::new|.lock()|.read().|.write()., and
syscall/copy patterns) to run against the full hot-path set including
src/shard/spsc_handler.rs and src/storage/dashtable/*.rs (and any other files
listed in the "Hot Path Files" section) so no files are omitted from the checks.
In @.claude/skills/cuda-scaffold/SKILL.md:
- Around line 41-45: The loop that loads float4 via reinterpret_cast<const
float4*>(va + d) in the vectorized dot-product (variables: dim, va, vb, acc) can
read past the end when dim % 4 != 0; modify the routine to compute a
vectorized_end = dim - (dim % 4) and iterate d from 0 to vectorized_end in steps
of 4 using the existing float4 loads, then handle the remaining tail elements
with a simple scalar loop (e.g., for d = vectorized_end; d < dim; ++d) that
accumulates acc using va[d] * vb[d], ensuring no out-of-bounds accesses.
- Around line 55-60: The GPU wrapper {name}_gpu currently calls
CudaDevice::new(...).expect and module.get_fn(...).unwrap which panics on
failure; change {name}_gpu to return a Result (or Option) instead of panicking,
propagate any errors from CudaDevice::new, dev.load_ptx and module.get_fn, and
ensure the public scaffold documents a call-site pattern that on Err (or None)
calls the CPU implementation {name}_cpu(a,b,dim) as a graceful fallback;
specifically, replace the expect/unwrap usage in {name}_gpu with proper error
handling (map_err/try/? or matching) and update docs to show how callers should
handle Err by invoking {name}_cpu.
In @.claude/skills/dual-runtime-check/SKILL.md:
- Around line 30-33: The doc promises dual-runtime testing but only shows the
"Test tokio" step and its cargo command; either add a matching "Test monoio"
step or update the text to state only tokio is tested. To add the missing step,
create a new entry similar to "Test tokio" named "Test monoio" and run the
corresponding command (e.g., MOON_NO_URING=1 cargo test --no-default-features
--features runtime-monoio,jemalloc) so both runtimes are covered; alternatively,
change the heading/description to explicitly state that only tokio is executed.
Ensure you update the SKILL.md section containing the "Test tokio" command to
reflect whichever option you choose.
In @.claude/skills/gpu-bench/SKILL.md:
- Around line 10-13: Add the missing usage entry for the documented "distance"
mode by inserting a `/gpu-bench distance` bullet into the usage list so it
matches the description; update the list that currently contains `/gpu-bench
vector`, `/gpu-bench batch`, and `/gpu-bench all` to include `/gpu-bench
distance` (use the exact string `/gpu-bench distance`) so the SKILL.md usage
section aligns with the described modes.
In @.claude/skills/redis-compat/SKILL.md:
- Around line 23-31: Update the comparison step that currently uses
presentation-oriented redis-cli output to a binary-safe, byte-accurate method:
run the requests with redis-cli using raw output mode and convert both server
responses to a canonical hex or byte-dump (e.g., via xxd/hexdump) before
comparing so CRLFs, null bytes and other binary data are preserved; replace the
existing "Compare response bytes" instruction in SKILL.md with this protocol
(use redis-cli --raw + canonical hex dump) and require exact hex/byte-for-byte
equality when validating compatibility.
In @.claude/skills/simd-bench/SKILL.md:
- Around line 29-34: Add an explicit scalar benchmark command next to the
existing SIMD example in SKILL.md so the comparison is reproducible: mirror the
shown command "cargo bench --bench get_hotpath -- \"3_dashtable\"
--save-baseline simd" but run with the repository’s scalar-disable/force-scalar
cfg or feature and save the baseline as "scalar"; place this scalar command
immediately after the SIMD line in the same example block so users can run both
and compare SSE2 vs scalar.
---
Nitpick comments:
In @.claude/skills/add-command/SKILL.md:
- Around line 47-49: Relax the blanket rule: require that error texts match
Redis exactly and that command implementations prefer shared helpers like
err_wrong_args, err_syntax_error, etc., but allow formatted construction when
those helpers produce dynamic content; only mandate Bytes::from_static() for
true fixed-literal error strings (not for helper-derived strings), and ensure
all error returns use Frame::Error (never panic) — update docs and examples to
show using helper functions for canonical messages, Bytes::from_static() for
literal bytes, and returning Frame::Error in functions such as handle_command /
execute_command where errors are produced.
In @.claude/skills/check-hotpath/SKILL.md:
- Around line 31-35: Update the grep pattern in the SKILL.md check that
currently searches for
"Mutex::new\|RwLock::new\|\.lock()\|\.read()\.\|\.write()\." so it only flags
usages of the standard library locks; specifically match "std::sync::Mutex" and
"std::sync::RwLock" (and their associated ".lock()", ".read()", ".write()"
calls) instead of any Mutex/RwLock symbol to reduce false positives; modify the
quoted command string referenced in the diff (the grep line) to include
"std::sync::Mutex" and "std::sync::RwLock" tokens (and/or use a regex like
"std::sync::Mutex(::new)?|std::sync::RwLock(::new)?|std::sync::Mutex::lock|std::sync::RwLock::read|std::sync::RwLock::write")
so the check only targets std::sync lock usage.
In @.claude/skills/unsafe-audit/SKILL.md:
- Around line 23-26: The current "NEW unsafe vs main" step uses the command "git
diff main -- '*.rs' | grep '+.*unsafe'" which can match diff headers and
context; update it to only scan added Rust source lines by using git diff with a
filter for added/modified files (diff-filter A/M) and zero context (unified=0)
and then only match lines that begin with a single "+" (i.e. exclude "+++" file
header lines) when searching for "unsafe"; replace the brittle pipeline in
SKILL.md with this tighter diff-filtering approach so the audit output only
shows newly added Rust unsafe lines.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c0e2d86c-66a6-4e8c-820c-64078ef6f092
📒 Files selected for processing (13)
.claude/agents/gpu-arch-reviewer.md.claude/agents/redis-compat-checker.md.claude/agents/unsafe-auditor.md.claude/settings.json.claude/skills/add-command/SKILL.md.claude/skills/check-hotpath/SKILL.md.claude/skills/cuda-scaffold/SKILL.md.claude/skills/dual-runtime-check/SKILL.md.claude/skills/gpu-bench/SKILL.md.claude/skills/redis-compat/SKILL.md.claude/skills/simd-bench/SKILL.md.claude/skills/unsafe-audit/SKILL.mdCLAUDE.md
|
|
||
| ## CPU Fallback Correctness | ||
|
|
||
| - CPU path must produce bit-identical results (within f32 epsilon) |
There was a problem hiding this comment.
Clarify correctness criterion: exact match vs epsilon-based match.
Line 28 mixes two different standards (“bit-identical” and “within f32 epsilon”). Pick one to avoid inconsistent reviews.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/agents/gpu-arch-reviewer.md at line 28, The criterion text "CPU path
must produce bit-identical results (within f32 epsilon)" is internally
inconsistent; pick one standard and update the sentence accordingly: either
require true bit-identical equality (remove any mention of f32 epsilon) or
require numerical equality within a defined float32 tolerance (replace the
phrase with e.g. "CPU path must produce results equal within float32 tolerance:
max absolute error <= 1.1920929e-7" and state whether you use max absolute or
relative error). Edit the existing phrase "CPU path must produce bit-identical
results (within f32 epsilon)" to the chosen wording and include the exact
epsilon and error metric so reviewers have a single, unambiguous correctness
rule.
Comment on lines
+26
to
+31
| ``` | ||
| [SOUND|UNSOUND|NEEDS_REVIEW] file.rs:line | ||
| Context: <what the block does> | ||
| Invariant: <what must be true> | ||
| Verdict: <reasoning> | ||
| ``` |
There was a problem hiding this comment.
Add a language tag to the fenced block to satisfy markdownlint.
The code fence beginning on Line 26 should specify a language (e.g., text) to avoid MD040 warnings.
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 26-26: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/agents/unsafe-auditor.md around lines 26 - 31, The fenced code block
in .claude/agents/unsafe-auditor.md that currently starts with ``` should be
updated to include a language tag (for example change ``` to ```text) to satisfy
markdownlint MD040; locate the block containing the lines
"[SOUND|UNSOUND|NEEDS_REVIEW] file.rs:line" and add an appropriate language
identifier (e.g., text) after the opening backticks so the linter recognizes the
fence language.
| "hooks": [ | ||
| { | ||
| "type": "command", | ||
| "command": "FILE_PATH=$(echo \"$TOOL_INPUT\" | grep -o '\"file_path\":\"[^\"]*\"' | head -1 | cut -d'\"' -f4); if echo \"$FILE_PATH\" | grep -q '\\.rs$'; then cd /Users/tindang/workspaces/tind-repo/moon && cargo fmt --check 2>&1 | head -10; fi" |
There was a problem hiding this comment.
Hardcoded absolute path breaks the formatter hook outside one machine.
Line 24 hardcodes a local path, so cargo fmt --check won’t run for other contributors or CI.
🔧 Proposed fix
- "command": "FILE_PATH=$(echo \"$TOOL_INPUT\" | grep -o '\"file_path\":\"[^\"]*\"' | head -1 | cut -d'\"' -f4); if echo \"$FILE_PATH\" | grep -q '\\.rs$'; then cd /Users/tindang/workspaces/tind-repo/moon && cargo fmt --check 2>&1 | head -10; fi"
+ "command": "FILE_PATH=$(echo \"$TOOL_INPUT\" | grep -o '\"file_path\":\"[^\"]*\"' | head -1 | cut -d'\"' -f4); if echo \"$FILE_PATH\" | grep -q '\\.rs$'; then REPO_ROOT=$(git rev-parse --show-toplevel 2>/dev/null || pwd); cd \"$REPO_ROOT\" && cargo fmt --check 2>&1 | head -10; fi"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "command": "FILE_PATH=$(echo \"$TOOL_INPUT\" | grep -o '\"file_path\":\"[^\"]*\"' | head -1 | cut -d'\"' -f4); if echo \"$FILE_PATH\" | grep -q '\\.rs$'; then cd /Users/tindang/workspaces/tind-repo/moon && cargo fmt --check 2>&1 | head -10; fi" | |
| "command": "FILE_PATH=$(echo \"$TOOL_INPUT\" | grep -o '\"file_path\":\"[^\"]*\"' | head -1 | cut -d'\"' -f4); if echo \"$FILE_PATH\" | grep -q '\\.rs$'; then REPO_ROOT=$(git rev-parse --show-toplevel 2>/dev/null || pwd); cd \"$REPO_ROOT\" && cargo fmt --check 2>&1 | head -10; fi" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/settings.json at line 24, The hook command hardcodes an absolute
path (the cd /Users/tindang/... in the "command" string) so cargo fmt --check
won't run on other machines/CI; change the command to compute the repo root
dynamically (e.g., use git rev-parse --show-toplevel or $PWD/GITHUB_WORKSPACE)
and run cargo fmt --check from that root only when FILE_PATH ends with .rs (the
FILE_PATH extraction, grep '\.rs$', and cargo fmt --check invocation should
remain but remove the hardcoded cd path), ensuring the formatter runs portably
for all contributors.
| }, | ||
| { | ||
| "type": "command", | ||
| "command": "if echo \"$TOOL_RESULT\" | grep -q 'unsafe'; then echo 'WARNING: unsafe block detected — ensure // SAFETY: comment is present'; fi" |
There was a problem hiding this comment.
unsafe warning hook is too broad and will produce noisy false positives.
Line 28 matches any unsafe text in tool output, including legitimate existing code and comments. Consider scoping to newly added unsafe in edited Rust files and/or specifically checking for missing nearby // SAFETY: comments.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/settings.json at line 28, The current hook command that greps
TOOL_RESULT for the substring "unsafe" is too broad; change it so it only flags
newly added unsafe usages in edited Rust files and only when a nearby "//
SAFETY:" comment is missing. Replace the simple grep of TOOL_RESULT with a
two-step check: (1) compute the git diff for edited/added *.rs files and search
only added lines (those starting with "+") for the token "unsafe", and (2) for
each added unsafe occurrence verify there is a nearby "// SAFETY:" comment
(e.g., same hunk or preceding context) before producing the warning; keep
references to TOOL_RESULT, the "unsafe" token, and the "// SAFETY:" marker so
the hook logic targets just new unsafe additions without existing safety
comments.
|
|
||
| ## Steps | ||
|
|
||
| 1. **Read the registry**: Find the PHF dispatch table in `src/command/registry.rs` (or equivalent). |
There was a problem hiding this comment.
Dispatch instructions don’t match the actual codebase architecture.
Lines 15 and 33 instruct PHF-table updates, but current dispatch is match-based in src/command/mod.rs. Please update this skill to direct edits to the real dispatch path.
Also applies to: 33-33
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/skills/add-command/SKILL.md at line 15, Update the skill
documentation to stop instructing edits to a PHF dispatch table and instead
point contributors to the real match-based dispatch in the command module (e.g.,
the match statement inside command::mod that routes commands); change the
references on lines 15 and 33 to instruct editing the appropriate match arms in
command::mod (or the function that performs command dispatch) rather than a PHF
table, and clarify how to add a new arm/branch and accompanying handler function
name to keep the skill aligned with the actual code architecture.
Comment on lines
+55
to
+60
| pub fn {name}_gpu(a: &[f32], b: &[f32], dim: usize) -> Vec<f32> { | ||
| use cudarc::driver::*; | ||
| let dev = CudaDevice::new(0).expect("CUDA device"); | ||
| let module = dev.load_ptx(PTX_SRC.into(), "{name}", &["{name}_kernel"]).unwrap(); | ||
| let f = module.get_fn("{name}_kernel").unwrap(); | ||
| // ... launch kernel, copy results |
There was a problem hiding this comment.
Avoid unwrap/expect in generated GPU wrapper; scaffold graceful CPU fallback.
Lines 57–60 currently encode panic paths, which conflicts with the CPU-fallback requirement and can crash production flows.
Suggested direction
#[cfg(feature = "gpu-cuda")]
-pub fn {name}_gpu(a: &[f32], b: &[f32], dim: usize) -> Vec<f32> {
+pub fn {name}_gpu(a: &[f32], b: &[f32], dim: usize) -> Result<Vec<f32>, GpuError> {
use cudarc::driver::*;
- let dev = CudaDevice::new(0).expect("CUDA device");
- let module = dev.load_ptx(PTX_SRC.into(), "{name}", &["{name}_kernel"]).unwrap();
- let f = module.get_fn("{name}_kernel").unwrap();
+ let dev = CudaDevice::new(0)?;
+ let module = dev.load_ptx(PTX_SRC.into(), "{name}", &["{name}_kernel"])?;
+ let f = module.get_fn("{name}_kernel")?;
// ... launch kernel, copy results
}Then document a call-site pattern that falls back to {name}_cpu(...) on Err.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/skills/cuda-scaffold/SKILL.md around lines 55 - 60, The GPU wrapper
{name}_gpu currently calls CudaDevice::new(...).expect and
module.get_fn(...).unwrap which panics on failure; change {name}_gpu to return a
Result (or Option) instead of panicking, propagate any errors from
CudaDevice::new, dev.load_ptx and module.get_fn, and ensure the public scaffold
documents a call-site pattern that on Err (or None) calls the CPU implementation
{name}_cpu(a,b,dim) as a graceful fallback; specifically, replace the
expect/unwrap usage in {name}_gpu with proper error handling (map_err/try/? or
matching) and update docs to show how callers should handle Err by invoking
{name}_cpu.
Comment on lines
+30
to
+33
| 5. **Test tokio** (CI-compatible): | ||
| ```bash | ||
| MOON_NO_URING=1 cargo test --no-default-features --features runtime-tokio,jemalloc | ||
| ``` |
There was a problem hiding this comment.
The workflow claims dual-runtime test coverage but only tests tokio.
Line 3 promises tests for both runtimes, but Lines 30-33 only execute tokio tests. Please add a monoio test step (or explicitly narrow the stated scope).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/skills/dual-runtime-check/SKILL.md around lines 30 - 33, The doc
promises dual-runtime testing but only shows the "Test tokio" step and its cargo
command; either add a matching "Test monoio" step or update the text to state
only tokio is tested. To add the missing step, create a new entry similar to
"Test tokio" named "Test monoio" and run the corresponding command (e.g.,
MOON_NO_URING=1 cargo test --no-default-features --features
runtime-monoio,jemalloc) so both runtimes are covered; alternatively, change the
heading/description to explicitly state that only tokio is executed. Ensure you
update the SKILL.md section containing the "Test tokio" command to reflect
whichever option you choose.
Comment on lines
+10
to
+13
| - `/gpu-bench vector` — vector distance computation (L2, cosine, dot) | ||
| - `/gpu-bench batch` — batch operation throughput | ||
| - `/gpu-bench all` — full GPU benchmark suite | ||
|
|
There was a problem hiding this comment.
Usage list is missing the distance mode documented in the description.
Line 3 advertises distance, but Lines 10-13 only list vector, batch, and all. Add a distance usage example for consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/skills/gpu-bench/SKILL.md around lines 10 - 13, Add the missing
usage entry for the documented "distance" mode by inserting a `/gpu-bench
distance` bullet into the usage list so it matches the description; update the
list that currently contains `/gpu-bench vector`, `/gpu-bench batch`, and
`/gpu-bench all` to include `/gpu-bench distance` (use the exact string
`/gpu-bench distance`) so the SKILL.md usage section aligns with the described
modes.
Comment on lines
+23
to
+31
| 2. For each command, run identical operations against both servers via `redis-cli`: | ||
| - Normal cases (happy path) | ||
| - Wrong number of arguments | ||
| - Wrong key type | ||
| - Boundary values (empty string, very large, negative) | ||
| - Non-existent keys | ||
|
|
||
| 3. Compare response bytes character-for-character. | ||
|
|
There was a problem hiding this comment.
Current comparison method is not byte-accurate for binary-safe validation.
Line 30 asks for byte-level equivalence, but plain redis-cli output is presentation-oriented. Please define a raw/protocol comparison method (e.g., --raw plus hex dump) for both servers to avoid false compatibility results.
🧰 Tools
🪛 LanguageTool
[style] ~27-~27: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...ype - Boundary values (empty string, very large, negative) - Non-existent keys 3. C...
(EN_WEAK_ADJECTIVE)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/skills/redis-compat/SKILL.md around lines 23 - 31, Update the
comparison step that currently uses presentation-oriented redis-cli output to a
binary-safe, byte-accurate method: run the requests with redis-cli using raw
output mode and convert both server responses to a canonical hex or byte-dump
(e.g., via xxd/hexdump) before comparing so CRLFs, null bytes and other binary
data are preserved; replace the existing "Compare response bytes" instruction in
SKILL.md with this protocol (use redis-cli --raw + canonical hex dump) and
require exact hex/byte-for-byte equality when validating compatibility.
Comment on lines
+29
to
+34
| 3. Compare SSE2 baseline vs scalar (force scalar via cfg): | ||
| ```bash | ||
| # Baseline (SIMD enabled) | ||
| cargo bench --bench get_hotpath -- "3_dashtable" --save-baseline simd | ||
| # Report improvement factor | ||
| ``` |
There was a problem hiding this comment.
Missing scalar run command makes the comparison incomplete.
Line 29 says to compare SIMD vs scalar, but only a SIMD baseline command is provided. Please add the explicit scalar benchmark command (using this repo’s scalar-disable cfg/feature) so the comparison is reproducible.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/skills/simd-bench/SKILL.md around lines 29 - 34, Add an explicit
scalar benchmark command next to the existing SIMD example in SKILL.md so the
comparison is reproducible: mirror the shown command "cargo bench --bench
get_hotpath -- \"3_dashtable\" --save-baseline simd" but run with the
repository’s scalar-disable/force-scalar cfg or feature and save the baseline as
"scalar"; place this scalar command immediately after the SIMD line in the same
example block so users can run both and compare SSE2 vs scalar.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.claude/settings.json(format check on edit, block sensitive files, block Cargo.lock, unsafe warning)unsafe-auditor,redis-compat-checker,gpu-arch-reviewer/unsafe-audit,/redis-compat,/add-command,/gpu-bench,/simd-bench,/cuda-scaffold,/check-hotpath,/dual-runtime-checkTest plan
.claude/settings.jsonis valid JSON/autocompleteSummary by CodeRabbit