diff --git a/agent/ADDITIONAL/RUST.md b/agent/ADDITIONAL/RUST.md index e46bd55..b37fe55 100644 --- a/agent/ADDITIONAL/RUST.md +++ b/agent/ADDITIONAL/RUST.md @@ -17,7 +17,8 @@ Rust-specific extensions to `STYLE.md`. The core principles apply; this addendum - No `Box` — erases information callers may need - No single monolithic error enum spanning multiple unrelated modules - No `unwrap()` or `expect()` in paths reachable at runtime; reserve for invariants that cannot fail by construction -- Prefer `expect("invariant description")` over `unwrap()` — name the invariant, not the symptom +- `unwrap()` is acceptable for hardcoded literal values that are obviously valid (e.g. `NaiveDate::from_ymd_opt(2016, 3, 1).unwrap()`) — the value itself is the documentation +- Prefer `expect("invariant description")` over `unwrap()` in all other cases — name the invariant, not the symptom ## 3. Ownership signals design - Prefer owned types in structs that clearly own their data @@ -25,7 +26,7 @@ Rust-specific extensions to `STYLE.md`. The core principles apply; this addendum - `Arc>` only when shared mutable ownership is genuinely required - Widespread `clone()` calls are a code smell — investigate before accepting - No `Arc>` to resolve borrow conflicts without first understanding why they exist -- No cloning to avoid lifetime annotations — lifetimes communicate real constraints +- Avoid lifetime annotations where a cheap clone or `.into()` removes the need — lifetimes add cognitive overhead and should only appear when the borrow reflects a real, meaningful constraint (e.g. a streaming reader holding a reference across calls). A single clone at a struct boundary or a `.into()` cast for a string literal is preferable to annotating an entire type with `'a` ## 4. Traits define behaviour, not convenience - Define a trait when there are (or will be) multiple concrete implementations @@ -42,12 +43,36 @@ Rust-specific extensions to `STYLE.md`. The core principles apply; this addendum - No test helpers mixed into the public API ## 6. Serde config conventions +- Use JSON as the config file format — not TOML, YAML, or properties files +- Write JSON keys in snake_case to match Rust field names — no `rename_all` or `rename` attributes needed on config structs - Apply `#[serde(deny_unknown_fields)]` to all config structs — makes unrecognised keys an error rather than a silent no-op - Use internally-tagged enums (`#[serde(tag = "...")]`) for config variants with associated parameters - Name the tag field after the domain concept (e.g. `"model"`, `"strategy"`) - No untagged or adjacently-tagged enums for config without specific reason -## 7. Runtime-to-static dispatch +## 7. CSV reader pattern +When reading CSV input files, separate raw deserialisation from domain construction: +- Define a private `*Row` struct (e.g. `AnimalRow`) that derives `Deserialize` and mirrors the CSV columns exactly — column renaming via `#[serde(rename = "...")]` lives here +- Define a separate public domain struct (e.g. `CattleDetails`) that uses proper domain types (`LocationKey`, `TimeStep`, custom enums) — no raw `u32`/`String` primitives where a richer type exists +- Wrap `csv::Reader` in a named `*Reader` struct; expose `new(path: &Path)` for construction +- Write custom serde deserialiser logic in small private submodules (`mod date_format`, `mod dam_flag`) within the same file, wired in via `#[serde(with = "...")]` on the `Row` struct — keeps the Row definition readable and the logic local + +Two loading patterns depending on file size: + +**Eager (`load()`)** — for small lookup tables that are needed throughout the run: +- Expose a consuming `load(...)` method that reads the whole file and returns the fully-typed collection +- Use for holdings, relationships, neighbours, abundance, seeding inputs + +**Streaming (`take_day()`)** — for large chronologically-sorted files (e.g. ~300M row movement files): +- Keep the `csv::Reader` open inside the struct alongside a one-record lookahead buffer and the last-seen `TimeStep` +- Expose `take_day(&TimeStep) -> Result>` — drains all records for that timestep and buffers the next, so only one day is in memory at a time +- Use for cattle and sheep movement files + +## 8. Imports +- Always import types and functions with `use` at the top of the file — do not write fully-qualified paths at call sites (e.g. `std::io::Error`, `std::process::Command`) where a `use` statement would serve +- Exception: `use std::fmt;` followed by `fmt::Formatter` / `fmt::Result` at call sites is idiomatic and preferred — importing `Result` directly would require `Result as FmtResult` to avoid a name clash with `std::result::Result`, which is more verbose and less readable + +## 9. Runtime-to-static dispatch Where a component is selected at runtime but the variant set is closed, bridge with a single startup `match`: ```rust diff --git a/agent/MAP-GUIDANCE.md b/agent/MAP-GUIDANCE.md index 0cad90a..1f10546 100644 --- a/agent/MAP-GUIDANCE.md +++ b/agent/MAP-GUIDANCE.md @@ -28,9 +28,9 @@ Mark unmapped nodes with `(TODO)`. Place the tree at the end of the root node, a ```markdown # Node Name -[Up](#parent) -[Down](#child-one) -[Down](#child-two) +[Parent Name](#parent) +[Child One](#child-one) +[Child Two](#child-two) Terse conceptual description. Lead with the mental picture. @@ -50,10 +50,11 @@ The user can stop reading before this; the agent reads through it. Each node carries markdown links encoding its position in the tree: -- `[Up](#parent)` — one link to the parent. Omitted only for the root node. -- `[Down](#child)` — one link per child. +- `[Parent Name](#parent)` — one link to the parent. Omitted only for the root node. +- `[Child Name](#child)` — one link per child. +- Links use the actual node name as the link text, not a generic `Up`/`Down` label. - Links are standard markdown anchors, navigable with `gd` in editors with a markdown LSP (marksman recommended). -- To see a node's siblings, go Up — the parent's Down list shows them all. +- To see a node's siblings, follow the parent link — the parent's child links show them all. ### Who maintains what @@ -82,7 +83,6 @@ When the project name clashes with a prominent internal concept, pick whichever Prefer a term that carries domain information over a generic "Application". - ## Node sizing Aim for under ~800 characters per node. The real test: if a node feels like it warrants sub-sections, it's too big — split into children. Flag nodes that exceed this to the user. @@ -92,7 +92,6 @@ Aim for under ~800 characters per node. The real test: if a node feels like it w Prefer folding a singleton child into its parent. Keep it as its own node when it is a distinct concept in the user's model, or when its detail would bloat the parent. A useful test: if a sibling were later added, would this still be a node? If yes, keep it now. - ## Content principles - Lead with the mental picture, not the implementation. @@ -127,7 +126,6 @@ Map edits are negotiated per-node per the Engagement rule — never pre-staged a For code changes, Approach and Plan typically don't propose map edits and Build doesn't touch the map. Map catch-up follows the build as a per-node negotiation. The completed change's Conclusion may carry a starter draft. Tightly-bound exceptions where small map work rides along a code change are allowed when it genuinely fits. - ## Maintaining the map Signals that the map needs attention: diff --git a/agent/STYLE.md b/agent/STYLE.md index 6506e84..6679faf 100644 --- a/agent/STYLE.md +++ b/agent/STYLE.md @@ -22,6 +22,7 @@ - Names answer "what does this do?" or "what does this represent?" - Use domain language — the vocabulary stakeholders use - No abbreviations that save typing but cost reading: `usr`, `cfg`, `proc` +- Standard domain shorthand that doesn't cost readability is fine: `loc` for location, `rng` for random number generator, `params` for parameters - No technical names (`manager`, `processor`, `handler`) where a domain name exists ## 5. Comments explain *why*, not *what*