-
Notifications
You must be signed in to change notification settings - Fork 0
docs: add Rust agent guidance #15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
jan-kubica
wants to merge
3
commits into
main
Choose a base branch
from
feat/rust-agent-guidance
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| ## Coding Conventions | ||
|
|
||
| ### Rust | ||
|
|
||
| - Use Rust 2024 for new crates. Pin the toolchain in `rust-toolchain.toml` and | ||
| keep `rustfmt` and `clippy` installed. | ||
| - In workspaces, put shared lint policy in `[workspace.lints]`; member crates | ||
| should opt in with `[lints] workspace = true`. | ||
| - Treat | ||
| `cargo clippy --workspace --all-targets --all-features -- -D warnings` as the | ||
| baseline quality gate unless a repo documents a narrower command. | ||
| - Use Dylint for shared stella-specific Rust lints that Clippy cannot express. | ||
| Run `cargo dylint --workspace --all` after Clippy when the repo has | ||
| `dylint.toml`. | ||
| - Prefer fixing custom lint rules at the shared source over broad local | ||
| suppressions when a rule is wrong across repos. | ||
| - Forbid unsafe code by default. If `unsafe` is truly required, keep it in a | ||
| tiny module with a `SAFETY:` comment explaining the invariant the caller and | ||
| callee rely on. | ||
| - Do not use `unwrap()`, `expect()`, `panic!()`, `todo!()`, or | ||
| `unimplemented!()` in production code. Return typed errors or make the | ||
| impossible state unrepresentable. | ||
| - Avoid unchecked indexing and string slicing. Prefer iterator methods, | ||
| `.get()`, typed span helpers, and APIs that preserve UTF-8 boundary safety. | ||
| - Avoid `as` casts. Prefer `TryFrom`, `From`, explicit checked conversion | ||
| helpers, or domain newtypes. | ||
| - Prefer narrow domain types over primitive strings/numbers for IDs, byte | ||
| offsets, language codes, entity labels, versions, and artifact formats. | ||
| - Keep struct fields private unless direct construction is part of the public | ||
| contract. Use smart constructors for values that must satisfy invariants. | ||
| - Use enums for real closed domain states and boolean-blind choices where | ||
| variants carry domain meaning. For callsite ergonomics alone, prefer an | ||
| options struct or `bon` builder over an enum that only simulates named | ||
| arguments. | ||
| - For functions, use positional parameters for one or two obvious arguments. | ||
| Use a named `SomethingOptions`, `SomethingArgs`, or `SomethingParams` struct | ||
| for 3+ arguments or same-type arguments that are easy to swap. | ||
| - Use `bon` builders for public APIs, constructors, or setup functions with | ||
| many optional/boolean parameters where named callsites improve readability. Do | ||
| not use it to hide unclear domain modeling. | ||
| - Prefer `Result<T, E>` with a concrete error enum for library code. Use | ||
| `thiserror` for typed errors; use `miette` only where human-facing diagnostics | ||
| are valuable. | ||
| - Add `#[must_use]` to builders, config transforms, computed results, and APIs | ||
| where ignoring the return value is likely a bug. | ||
| - Keep comments concise. Comment invariants, non-obvious algorithms, generated | ||
| data contracts, and safety boundaries; do not narrate straightforward code. | ||
| - Keep data out of code. Domain dictionaries, language rules, fixtures, and | ||
| generated artifacts should live in reproducible data files or build outputs, | ||
| organized by language/concept where relevant. | ||
| - Public docs, logs, diagnostics, and comments should write `stella` lowercase. | ||
|
|
||
| ### Rust Module Side Effects | ||
|
|
||
| - Avoid expensive module-level initialization. Prefer explicit prepare/build | ||
| steps, lazy singletons, or build-time generated artifacts. | ||
| - Do not do filesystem, network, environment, or global logger setup from | ||
| library imports. Applications and CLIs own process-level side effects. | ||
| - Keep binding crates thin. Business logic belongs in the Rust core crate; | ||
| TypeScript, Python, WASM, and NAPI layers should translate types and call the | ||
| same core logic. | ||
| - Keep generated artifacts versioned and validated at load time. Reject stale, | ||
| mismatched, or oversized artifacts with typed errors. | ||
|
|
||
| ### Rust Testing | ||
|
|
||
| - Use `cargo nextest run --workspace --all-features` when available; otherwise | ||
| use the repo's documented `cargo test` command. | ||
| - Add property tests with `proptest` for parsers, span math, redaction, | ||
| normalization, serialization, and logic where examples do not cover the input | ||
| space. | ||
| - Add fuzz targets with `cargo-fuzz` for byte/string parsers, document readers, | ||
| search primitives, artifact decoders, and boundary-sensitive code. | ||
| - Use fixture parity tests when replacing an implementation in another language. | ||
| The Rust core, TypeScript binding, and Python binding should produce the same | ||
| structured result from the same fixtures. | ||
| - Benchmark behavior that is part of the product. Track cold start, warm run, | ||
| artifact load, preparation, and execution separately. | ||
| - Do not snapshot sensitive raw text unless the fixture is intentionally public | ||
| and minimal. Prefer normalized summaries, counts, spans, labels, and redacted | ||
| output. | ||
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Rust workspaces that put stella Dylint checks on tests, examples, or feature-gated code, this narrower command only runs through
cargo checkdefaults, while the baseline Clippy gate above explicitly covers--all-targets --all-features. Dylint accepts Cargo check args after--(its docs showcargo dylint --all -- --all-targets ...), so this shared guidance should mirror the same target/feature coverage; otherwise CI can pass while custom lints never inspect those code paths.Useful? React with 👍 / 👎.