docs: add repository context files
Add new context files to assist with repository navigation and design consistency: - context/repo-map.md: A roadmap for the repository. - context/design-principles.md: Guidelines for maintaining repository consistency. Update CLAUDE.md to include instructions on using the new context files. Co-Authored-By: fiddlerwoaroof/git-smart-commit (unsloth/gemma-4-26B-A4B-it-GGUF:UD-Q5_K_XL)
This commit is contained in:
Edward Langley
228
context/design-principles.md
Normal file
228
context/design-principles.md
Normal file
@ -0,0 +1,228 @@
|
||||
# Improvise Design Principles
|
||||
|
||||
## 1. Functional-First Architecture
|
||||
|
||||
### Commands Are Pure, Effects Are Side-Effectful
|
||||
|
||||
Every user action flows through a two-phase pipeline:
|
||||
|
||||
1. **Command** (`Cmd` trait) — reads immutable context, returns a list of effects.
|
||||
The `CmdContext` is a read-only snapshot: model, layout, mode, cursor position.
|
||||
Commands never touch `&mut App`. All decision logic is pure.
|
||||
|
||||
2. **Effect** (`Effect` trait) — a small struct with an `apply(&self, app: &mut App)` method.
|
||||
Each effect is one discrete, debuggable state change. The app applies them in order.
|
||||
|
||||
This separation means:
|
||||
- Commands are testable without a terminal or an `App` instance.
|
||||
- Effects can be logged, replayed, or composed.
|
||||
- The only place `App` is mutated is inside `Effect::apply`.
|
||||
|
||||
### Prefer Transformations to Mutation
|
||||
|
||||
Where possible, build new values rather than mutating in place:
|
||||
- `CellKey::with(cat, item)` returns a new key with an added/replaced coordinate.
|
||||
- `CellKey::without(cat)` returns a new key with a coordinate removed.
|
||||
- Viewport positioning is computed as a pure function (`viewport_effects`) that
|
||||
returns a `Vec<Effect>`, not a method that pokes at scroll offsets directly.
|
||||
|
||||
### Compose Small Pieces
|
||||
|
||||
Commands compose via `Binding::Sequence` — a keymap entry can chain multiple
|
||||
commands, each contributing effects independently. The `o` key (add row + begin
|
||||
editing) is two commands composed at the binding level, not a monolithic handler.
|
||||
|
||||
---
|
||||
|
||||
## 2. Polymorphism Over Conditionals
|
||||
|
||||
### Dispatch Through Traits and Registries, Not Match Blocks
|
||||
|
||||
- **Commands**: 40+ types each implement `Cmd`. A `CmdRegistry` maps names to
|
||||
constructor closures. Dispatching a key presses looks up the binding, resolves
|
||||
the command name through the registry, and calls `execute`. No central
|
||||
`match command_name { ... }` block.
|
||||
|
||||
- **Effects**: 50+ types each implement `Effect`. Collected into a `Vec<Box<dyn Effect>>`
|
||||
and applied in order. No `match effect_kind { ... }`.
|
||||
|
||||
- **Keymaps**: Each mode has its own `Keymap` (a `HashMap<KeyPattern, Binding>`).
|
||||
Mode dispatch is one table lookup, not a nested `match (mode, key)`.
|
||||
|
||||
### Use Enums to Make Invalid States Unrepresentable
|
||||
|
||||
- `BinOp` is an enum (`Add | Sub | Mul | ...`), not a string. Invalid operators
|
||||
are caught at parse time, not silently ignored at eval time.
|
||||
|
||||
- `Axis` is `Row | Column | Page | None`. A category is on exactly one axis.
|
||||
Cycling is a four-state rotation — no boolean flags, no "row_or_column" ambiguity.
|
||||
|
||||
- `Binding` is `Cmd | Prefix | Sequence`. The keymap lookup returns one of these
|
||||
three shapes; dispatch pattern-matches exhaustively.
|
||||
|
||||
- `CategoryKind` is `Regular | VirtualIndex | VirtualDim | Label`. Business rules
|
||||
(e.g., the 12-category limit counts only `Regular`) are enforced by matching
|
||||
on the enum, not by checking name prefixes.
|
||||
|
||||
### When You Add a Variant, the Compiler Finds Every Call Site
|
||||
|
||||
Prefer exhaustive `match` over `if let` or `_ =>` wildcards. When a new `Axis`
|
||||
variant or `AppMode` is added, non-exhaustive matches produce compile errors
|
||||
that guide you to every place that needs updating.
|
||||
|
||||
---
|
||||
|
||||
## 3. Correctness by Construction
|
||||
|
||||
### Canonical Forms Prevent Equivalence Bugs
|
||||
|
||||
`CellKey::new()` sorts coordinates by category name. Two keys that name the same
|
||||
intersection but in different order are identical after construction. Equality,
|
||||
hashing, and storage all work correctly without callers needing to remember to
|
||||
sort. Property tests verify this invariant.
|
||||
|
||||
### Smart Constructors Enforce Invariants
|
||||
|
||||
- `CellKey::new()` is the only way to build a key — it always sorts.
|
||||
- `Category::add_item()` deduplicates by name and auto-assigns IDs via a private
|
||||
counter. External code cannot fabricate an `ItemId`.
|
||||
- `Model::add_category()` checks the 12-category limit before insertion.
|
||||
- `Formula::new()` takes all required fields; there is no default/empty formula
|
||||
to accidentally leave half-initialized.
|
||||
|
||||
### Type-Safe Identifiers
|
||||
|
||||
`CategoryId` and `ItemId` are typed aliases. While they are `usize` underneath,
|
||||
using named types signals intent and prevents accidentally passing an item count
|
||||
where an item ID is expected.
|
||||
|
||||
### Symbol Interning for Data Integrity
|
||||
|
||||
`DataStore` interns category and item names into `Symbol` values (small copyable
|
||||
handles). This means:
|
||||
- String comparison is integer comparison — fast and allocation-free.
|
||||
- A secondary index maps `(Symbol, Symbol)` pairs to cell sets, enabling O(1)
|
||||
lookups for aggregation queries.
|
||||
- Symbols can only be created through the `SymbolTable`, so misspelled names
|
||||
produce a distinct symbol rather than silently matching a wrong cell.
|
||||
|
||||
### Parse-Time Validation
|
||||
|
||||
Formulas are parsed into a typed AST (`Expr` enum) at entry time. If the syntax
|
||||
is invalid, the user gets an error immediately. The evaluator only sees
|
||||
well-formed trees — it does not need to handle malformed input.
|
||||
|
||||
---
|
||||
|
||||
## 4. Separation of Concerns
|
||||
|
||||
### Four Layers
|
||||
|
||||
| Layer | Directory | Responsibility |
|
||||
|-------|-----------|----------------|
|
||||
| **Model** | `src/model/` | Categories, items, groups, cell data, formulas. Pure data, no rendering. |
|
||||
| **View** | `src/view/` | Axis assignments, page selection, hidden items, layout computation. Derived from model. |
|
||||
| **Command / Effect** | `src/command/`, `src/ui/effect.rs` | Intent (commands) and state mutation (effects). Bridges user input to model changes. |
|
||||
| **Rendering** | `src/draw.rs`, `src/ui/` | Terminal drawing. Reads model + view, writes pixels. No mutation. |
|
||||
|
||||
### Formulas Are Data, Not Code
|
||||
|
||||
A formula is a serializable struct: raw text, target name, category, AST, optional
|
||||
filter. It is stored in the model alongside regular data. The evaluator walks the
|
||||
AST at read time. Formulas never become closures or runtime-generated code.
|
||||
|
||||
### Display Rounding Is View-Only
|
||||
|
||||
Number formatting (`format_f64`) rounds for display. Formula evaluation always
|
||||
operates on full `f64` precision. The rounding function is only called in
|
||||
rendering paths — never in `eval_formula` or aggregation.
|
||||
|
||||
### Drill State Isolates Edits
|
||||
|
||||
When editing aggregated (drill-down) cells, a `DrillState` snapshot freezes the
|
||||
current cell set. Pending edits accumulate in a staging map. On commit,
|
||||
`ApplyAndClearDrill` writes them all atomically. On cancel, the snapshot is
|
||||
discarded. No partial writes reach the model.
|
||||
|
||||
---
|
||||
|
||||
## 5. Guidelines for New Code
|
||||
|
||||
- **Add a command, not a special case.** If you need new behavior on a keypress,
|
||||
implement `Cmd`, register it, and bind it in the keymap. Do not add an
|
||||
`if key == 'x'` branch inside `handle_key`.
|
||||
|
||||
- **Return effects, do not mutate.** Your command's `execute` receives `&CmdContext`
|
||||
(immutable). Produce a `Vec<Box<dyn Effect>>`. If you need a new kind of state
|
||||
change, create a new `Effect` struct.
|
||||
|
||||
- **Use the type system.** If a value can only be one of N things, make it an enum.
|
||||
If an invariant must hold, enforce it in the constructor. If a field is
|
||||
optional, use `Option` — do not use sentinel values.
|
||||
|
||||
- **Test the logic, not the wiring.** Commands are pure functions of context;
|
||||
test them by building a `CmdContext` and asserting on the returned effects.
|
||||
You do not need a terminal.
|
||||
|
||||
- **Keep `Option`/`Result`/`Box` at the boundaries.** Core logic should work with
|
||||
concrete values. Wrap in `Option` at the edges (parsing, lookup, I/O) and
|
||||
unwrap early. Do not thread `Option` through deep call chains.
|
||||
|
||||
---
|
||||
|
||||
## 6. Testing
|
||||
|
||||
### Coverage and ambition
|
||||
|
||||
Aim for **~80% line and branch coverage** on logic code. This is a quality floor —
|
||||
go higher where the code is tricky or load-bearing, but don't pad coverage by
|
||||
testing trivial getters or chasing 100% on rendering widgets. The test suite
|
||||
should remain fast (under 2 seconds). Slow tests erode the habit of running them.
|
||||
|
||||
### Demonstrate bugs before fixing them
|
||||
|
||||
Write a test that **fails on the current code** before writing the fix. Prefer a
|
||||
small unit test targeting the broken function over an end-to-end test. After the
|
||||
fix, the test stays as a regression guard. Document the bug in the test's
|
||||
doc-comment (see `model/types.rs` → `formula_tests` for examples).
|
||||
|
||||
### Use property tests judiciously
|
||||
|
||||
Property tests (`proptest`) are for **invariants that must hold across all
|
||||
inputs** — not a replacement for example-based tests. Good candidates:
|
||||
|
||||
- Structural invariants: CellKey is always sorted, each category lives on exactly
|
||||
one axis, toggle-collapse is involutive, hide/show roundtrips.
|
||||
- Serialization roundtrips: save/load identity.
|
||||
- Determinism: `evaluate` returns the same result for the same inputs.
|
||||
|
||||
Keep case counts at the default (256). Don't crank them to thousands — if a
|
||||
property needs more cases to feel safe, constrain the input space with better
|
||||
strategies rather than brute-forcing. Property tests that take hundreds of
|
||||
milliseconds each are a sign something is wrong.
|
||||
|
||||
### What to test
|
||||
|
||||
- **Model, formula, view**: the core logic. Unit tests for each operation and
|
||||
edge case. Property tests for invariants. These are the highest-value tests.
|
||||
- **Commands**: build a `CmdContext`, call `execute`, assert on the returned
|
||||
effects. Pure functions — no terminal needed.
|
||||
- **Persistence**: round-trip tests (`save → load → save` produces identical
|
||||
output). Cover groups, formulas, views, hidden items, legacy JSON.
|
||||
- **Format**: boundary cases for comma placement, rounding, negative numbers.
|
||||
- **Import**: field classification heuristics, CSV quoting, multi-file merge.
|
||||
|
||||
### What not to test
|
||||
|
||||
- Ratatui `Widget::render` implementations — pure drawing code that changes
|
||||
often. Test the data they consume (layout, cat_tree, format) instead.
|
||||
- Trivial data definitions (`ast.rs`, `axis.rs`).
|
||||
- Module re-export files.
|
||||
|
||||
### Test the property, not the implementation
|
||||
|
||||
A test like "calling `set_axis(cat, Row)` sets the internal map entry to `Row`"
|
||||
is brittle — it mirrors the implementation and breaks if the storage changes.
|
||||
Instead test the observable contract: "after `set_axis(cat, Row)`,
|
||||
`axis_of(cat)` returns `Row` and `categories_on(Row)` includes `cat`." This
|
||||
style survives refactoring and catches real bugs.
|
||||
Reference in New Issue
Block a user