From a54997ee06a50700f9925b7cd8465c4b03bb6d02 Mon Sep 17 00:00:00 2001 From: ChrisGavinAPHA Date: Wed, 1 Jul 2026 10:52:25 +0000 Subject: [PATCH 1/2] Changes from rvfv --- agent/ADDITIONAL/RUST.md | 31 +++++++++++++++++++++++--- agent/MAP-GUIDANCE.md | 48 ++++++++++++---------------------------- agent/STYLE.md | 1 + 3 files changed, 43 insertions(+), 37 deletions(-) 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..0f03778 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 @@ -69,28 +70,15 @@ All optional. Always in this order when present: - **Detail** — implementation-level precision (parameters, thresholds, algorithms). The user can stop reading before this. Marked with bold `**Detail**`. - **See also** — cross-cutting references that aren't parent/child. Marked with bold `**See also**`. Each entry says why the relationship matters. -### Root node naming - -Default to the project's name at the root. Every file-root H1 must be unambiguous with every other heading in the map — duplicates break navigation (fragile auto-generated anchors, indistinguishable jump-picker entries). - -When the project name clashes with a prominent internal concept, pick whichever form reads best: - -- Domain scoping term for the root ("Audio Player", "Build Tool"). -- Suffix ("Deck Application Map", "Deck Map Subtree"). -- Parenthetical ("Deck (Application)"). -- Rename the internal node more specifically. - -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. -## The only-child preference +## The only-child rule -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. +If a node would have no siblings and no children, it's not a node — it's the bottom of its parent. ## Content principles @@ -113,19 +101,11 @@ The format is agnostic about how nodes are distributed across files: ## When to edit the map -Two rules govern every map edit. - -**Sync rule** — *The map describes what exists, not what is proposed.* Don't edit the map ahead of the code. Edits that describe pending work defer until the code is built. - -**Engagement rule** — *Every map edit is negotiated.* One node at a time, with user engagement. Never silent, never bulk. When an edit touches two or more nodes, enumerate them in chat up front and tick through them as each is settled. Per-node approval prompts are phrased as comprehension checks — "does that fit your mental model?", "is that clear enough?" — not yes/no gates. - -The map is exempt from the active-change requirement: edits that describe existing reality may happen at any time. - -### Map edits and the change lifecycle - -Map edits are negotiated per-node per the Engagement rule — never pre-staged as wholesale node bodies in an Approach. Map-only work happens as per-node negotiation directly, exempt from the change lifecycle alongside the existing active-change exemption. +The map is exempt from the active change requirement — it can be updated at any time. However: -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. +- **Only with user engagement.** Never update the map silently or in bulk. Discuss each structural change with the user, and only update one node at a time. +- **During a change:** read the map during the Approach stage. Include map update tasks in the Plan when the mapped area is affected. If an active change already includes map tasks, mention this to the user before making additional edits. +- **Outside a change:** the user may want to restructure the map spontaneously. Support this — it is the core comprehension-building activity. ## Maintaining the map @@ -136,7 +116,7 @@ Signals that the map needs attention: - **Top-level boxes don't match the user's mental model** — restructure. The map follows the user's model, not the code's architecture. - **A node has grown verbose** — cut aggressively. Move precision to **Detail**. - **New concepts don't have a natural home** — the decomposition may need rethinking, not a misc section. -- **An only-child exists that isn't a distinct concept** — fold it into its parent. +- **An only-child exists** — fold it into its parent. Don't treat the map as append-only. Restructuring is not rework — it's what keeps the map useful. 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* From 73df51dfe46d189c32a986be775397f03942642b Mon Sep 17 00:00:00 2001 From: ChrisGavinAPHA Date: Wed, 1 Jul 2026 11:02:58 +0000 Subject: [PATCH 2/2] fixing some bits in map guidance that changed by accident --- agent/MAP-GUIDANCE.md | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/agent/MAP-GUIDANCE.md b/agent/MAP-GUIDANCE.md index 0f03778..1f10546 100644 --- a/agent/MAP-GUIDANCE.md +++ b/agent/MAP-GUIDANCE.md @@ -70,16 +70,27 @@ All optional. Always in this order when present: - **Detail** — implementation-level precision (parameters, thresholds, algorithms). The user can stop reading before this. Marked with bold `**Detail**`. - **See also** — cross-cutting references that aren't parent/child. Marked with bold `**See also**`. Each entry says why the relationship matters. +### Root node naming + +Default to the project's name at the root. Every file-root H1 must be unambiguous with every other heading in the map — duplicates break navigation (fragile auto-generated anchors, indistinguishable jump-picker entries). + +When the project name clashes with a prominent internal concept, pick whichever form reads best: + +- Domain scoping term for the root ("Audio Player", "Build Tool"). +- Suffix ("Deck Application Map", "Deck Map Subtree"). +- Parenthetical ("Deck (Application)"). +- Rename the internal node more specifically. + +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. -## The only-child rule - -If a node would have no siblings and no children, it's not a node — it's the bottom of its parent. +## The only-child preference +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 @@ -101,12 +112,19 @@ The format is agnostic about how nodes are distributed across files: ## When to edit the map -The map is exempt from the active change requirement — it can be updated at any time. However: +Two rules govern every map edit. + +**Sync rule** — *The map describes what exists, not what is proposed.* Don't edit the map ahead of the code. Edits that describe pending work defer until the code is built. + +**Engagement rule** — *Every map edit is negotiated.* One node at a time, with user engagement. Never silent, never bulk. When an edit touches two or more nodes, enumerate them in chat up front and tick through them as each is settled. Per-node approval prompts are phrased as comprehension checks — "does that fit your mental model?", "is that clear enough?" — not yes/no gates. + +The map is exempt from the active-change requirement: edits that describe existing reality may happen at any time. + +### Map edits and the change lifecycle -- **Only with user engagement.** Never update the map silently or in bulk. Discuss each structural change with the user, and only update one node at a time. -- **During a change:** read the map during the Approach stage. Include map update tasks in the Plan when the mapped area is affected. If an active change already includes map tasks, mention this to the user before making additional edits. -- **Outside a change:** the user may want to restructure the map spontaneously. Support this — it is the core comprehension-building activity. +Map edits are negotiated per-node per the Engagement rule — never pre-staged as wholesale node bodies in an Approach. Map-only work happens as per-node negotiation directly, exempt from the change lifecycle alongside the existing active-change exemption. +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 @@ -116,7 +134,7 @@ Signals that the map needs attention: - **Top-level boxes don't match the user's mental model** — restructure. The map follows the user's model, not the code's architecture. - **A node has grown verbose** — cut aggressively. Move precision to **Detail**. - **New concepts don't have a natural home** — the decomposition may need rethinking, not a misc section. -- **An only-child exists** — fold it into its parent. +- **An only-child exists that isn't a distinct concept** — fold it into its parent. Don't treat the map as append-only. Restructuring is not rework — it's what keeps the map useful.