Add Rust coding standard skill and fix Clippy lints
- Add .agents/skills/rust-coding-standard/SKILL.md with Rust code style rules - Fix Clippy warnings: use &Path instead of &PathBuf, prefer map over filter_map, use unwrap_or_default, simplify closure types in engine.rs
Seto Elkahfi committed
Apr 3, 2026 at 16:27 UTC
d0e384d5d41d094e90e15d9330653bac1f2137ab
3 files changed
+127
-8
.agents/skills/rust-coding-standard/SKILL.md
new
+116
@@ -0,0 +1,116 @@
1
+---
2
+name: rust-coding-standard
3
+description: Rust coding standards for the onde crate. Covers error handling, naming, async patterns, platform gating, clippy compliance, and cross-platform SDK conventions. Apply to all Rust edits.
4
+allowed-tools: Read, Write, Edit, Glob, Grep
5
+user-invocable: false
6
+---
7
+
8
+# Rust Coding Standard
9
+
10
+Rules for writing Rust in the `onde` crate. These are traps to avoid, not maps to follow.
11
+
12
+---
13
+
14
+## Correctness First
15
+
16
+Prioritize correctness and clarity. Performance is secondary unless explicitly stated. Complete, mostly-correct code beats perfect code that doesn't solve the problem.
17
+
18
+## Error Handling
19
+
20
+- Propagate errors with `?`. Never call `.unwrap()` or `.expect()` outside of `#[cfg(test)]` code.
21
+- Never silently discard errors with `let _ =` on fallible operations:
22
+ - Propagate with `?` when the caller should handle it.
23
+ - Use `log::warn!` or `log::error!` when you need to ignore but want visibility.
24
+ - Use `match` or `if let Err(e)` when custom recovery logic is needed.
25
+- Use `anyhow` for internal application errors in `engine.rs`.
26
+- Use `thiserror` for the `InferenceError` enum — it crosses the FFI boundary via UniFFI.
27
+- Be careful with indexing operations that may panic if out of bounds. Prefer `.get()` and safe alternatives.
28
+
29
+## Naming
30
+
31
+- Use full words for all variable and parameter names. No single-letter names, no abbreviations (`environment` not `env`, `client` not `c`, `message` not `msg`).
32
+- `snake_case` for functions and variables, `CamelCase` for types, `SCREAMING_SNAKE_CASE` for constants.
33
+- Model constants live in `src/inference/models.rs`. Prefix HuggingFace repo IDs with the org name: `BARTOWSKI_QWEN25_...`.
34
+
35
+## Comments
36
+
37
+- Comments explain **why**, never **what**. Do not write comments that summarize or re-describe the code.
38
+- Doc comments (`///`) on public items are fine and encouraged — they explain the contract, not the implementation.
39
+- Do not add organizational separators like `// --- Helper functions ---` unless they are already established in the file's style.
40
+
41
+## Module Structure
42
+
43
+- **No `mod.rs` for new modules.** Use `src/some_module.rs` instead of `src/some_module/mod.rs`. Exception: `inference/mod.rs` already exists and is the intentional module root — don't move it.
44
+- Prefer implementing functionality in existing files unless it's a genuinely new logical component. Avoid creating many small single-purpose files.
45
+
46
+## Async
47
+
48
+- Runtime is `tokio`. All async functions use `tokio::sync::Mutex`, not `std::sync::Mutex`.
49
+- Use variable shadowing to scope `.clone()` calls inside async closures, minimizing borrow lifetimes:
50
+
51
+```rust
52
+tokio::spawn({
53
+ let engine = engine.clone();
54
+ async move {
55
+ engine.do_work().await;
56
+ }
57
+});
58
+```
59
+
60
+## Logging
61
+
62
+- Use `log` crate macros: `log::debug!`, `log::info!`, `log::warn!`, `log::error!`.
63
+- No `println!` or `eprintln!` in library code. Ever.
64
+- Log at `debug` for routine operations, `info` for lifecycle events (model loaded/unloaded), `warn` for recoverable issues, `error` for failures.
65
+
66
+## Platform Gating
67
+
68
+- Use `#[cfg(target_os = "...")]` blocks. Match `Cargo.toml`'s target-conditional dependency sections.
69
+- When a function is only used on specific platforms, gate the function itself — not just the call site. This prevents dead-code warnings on other targets.
70
+- When an import is only used on specific platforms, gate the `use` statement separately. Do not put platform-only imports inside a shared `use` block.
71
+- The supported `target_os` values are: `macos`, `ios`, `tvos`, `windows`, `linux`, `android`.
72
+- ISQ model loading is `#[cfg(target_os = "macos")]` only. Gate the `Isq` variant, its match arms, and its imports (`IsqBits`, `TextModelBuilder`) to macOS.
73
+
74
+## Clippy Compliance
75
+
76
+All code must pass `cargo clippy --all-targets -- -D warnings`. Common traps:
77
+
78
+- `&PathBuf` in function parameters → use `&Path` instead.
79
+- `unwrap_or_else(Type::default)` → use `unwrap_or_default()`.
80
+- `filter_map(|x| { ... Some(y) })` where every path returns `Some` → use `map`.
81
+- `.map(|x| f(x))` where `f` is a plain function → use `.map(f)`.
82
+- Redundant type annotations in closures: `.map(|c: &String| c.trim())` → `.map(|c| c.trim())`.
83
+- Unused imports/variants on platform-gated code → gate the import/variant to the same platform.
84
+
85
+## Types for FFI
86
+
87
+- `usize` / `isize` are not FFI-safe. Use `u64` / `i64` in any type that crosses the UniFFI boundary.
88
+- `impl Into<String>` is not FFI-safe. Use concrete `String` in exported signatures.
89
+- All UniFFI-exported types go in `src/inference/types.rs`. The FFI wrapper lives in `src/inference/ffi.rs`.
90
+- `ChatEngine` (engine.rs) owns Rust-idiomatic logic. `OndeChatEngine` (ffi.rs) is the thin FFI wrapper. Never put `#[uniffi::export]` on `ChatEngine`.
91
+
92
+## Re-exports
93
+
94
+`mistralrs`, `hf_hub`, and `mistralrs_core` are re-exported from `lib.rs` for downstream Rust consumers. Keep these in sync with what's actually available per platform.
95
+
96
+## Testing
97
+
98
+- Unit tests go in a `#[cfg(test)] mod tests` block at the bottom of the file they test.
99
+- Use `#[tokio::test]` for async tests.
100
+- Tests may use `.unwrap()` and `.expect()` — the panics are the assertions.
101
+- Test names describe the scenario: `engine_send_without_model_errors`, not `test_send` or `test_1`.
102
+- Run `cargo test --lib` to skip integration tests that require model downloads.
103
+
104
+## Adding a New Model
105
+
106
+This is a common task with a specific checklist:
107
+
108
+1. Add `pub const` entries in `src/inference/models.rs` for repo ID, GGUF filename, and `TOK_MODEL_ID`.
109
+2. Add the repo ID to `SUPPORTED_MODELS`.
110
+3. Add a `SupportedModelInfo` entry to `SUPPORTED_MODEL_INFO` with `expected_size_bytes` from the HF API.
111
+4. Add a constructor to `GgufModelConfig` in `engine.rs` (follow `qwen25_1_5b` pattern).
112
+5. Export a free function in `ffi.rs` (follow `qwen25_1_5b_config` pattern).
113
+
114
+## Build Verification
115
+
116
+Always run `cargo check` before returning work. If code does not compile, fix it — do not return with TODOs. On macOS, also run `cargo clippy --all-targets -- -D warnings` to catch lint issues before CI does.
\ No newline at end of file
src/hf_cache.rs
+8
-5
@@ -13,7 +13,10 @@ use {
13
crate::inference::models::{SUPPORTED_MODELS, SUPPORTED_MODEL_INFO},
14
log::{debug, info, warn},
15
serde::{Deserialize, Serialize},
16
- std::{fs, path::PathBuf},
16
+ std::{
17
+ fs,
18
+ path::{Path, PathBuf},
19
+ },
20
tsync::tsync,
21
};
22
@@ -222,7 +225,7 @@ fn format_size(bytes: u64) -> String {
225
}
226
227
/// List the snapshot revisions available for a cached model directory.
225
-fn list_revisions(model_dir: &PathBuf) -> Vec<String> {
228
+fn list_revisions(model_dir: &Path) -> Vec<String> {
229
let snapshots_dir = model_dir.join("snapshots");
230
if !snapshots_dir.is_dir() {
231
return Vec::new();
@@ -564,9 +567,9 @@ pub fn repair_hf_cache_symlinks(model_id: &str) {
567
Ok(entries) => entries
568
.flatten()
569
.filter(|e| e.path().is_dir())
567
- .filter_map(|e| {
570
+ .map(|e| {
571
let rev = e.file_name().to_string_lossy().to_string();
569
- Some((rev, e.path()))
572
+ (rev, e.path())
573
})
574
.collect(),
575
Err(_) => return,
@@ -836,7 +839,7 @@ pub fn list_supported_hf_models() -> SupportedHfModelsResponse {
839
let local_size = cache_path
840
.as_ref()
841
.filter(|p| p.exists())
839
- .map(|p| dir_size(p))
842
+ .map(dir_size)
843
.unwrap_or(0);
844
845
let has_cache = local_size > 0;
src/inference/engine.rs
+3
-3
@@ -392,7 +392,7 @@ impl ChatEngine {
392
393
let elapsed = start.elapsed();
394
395
- let sampling = sampling.unwrap_or_else(SamplingConfig::default);
395
+ let sampling = sampling.unwrap_or_default();
396
397
info!(
398
"ChatEngine: ISQ model {} loaded in {} (sampling: temp={:?}, max_tokens={:?})",
@@ -569,7 +569,7 @@ impl ChatEngine {
569
.message
570
.content
571
.as_ref()
572
- .map(|c: &String| c.trim().to_string())
572
+ .map(|c| c.trim().to_string())
573
.unwrap_or_else(|| "(empty response)".to_string());
574
575
let finish_reason = response.choices[0].finish_reason.clone();
@@ -646,7 +646,7 @@ impl ChatEngine {
646
.message
647
.content
648
.as_ref()
649
- .map(|c: &String| c.trim().to_string())
649
+ .map(|c| c.trim().to_string())
650
.unwrap_or_else(|| "(empty response)".to_string());
651
652
let finish_reason = response.choices[0].finish_reason.clone();