Captures three insights from 2026-04-16 deep review session:
- review-methodology-scoped-explore-agents
- compiler-exhaustiveness-theme
- agent-issue-drift-pattern
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Update various commands to utilize the new AbortChain and CleanEmptyRecords
effects.
- CommitAndAdvance now pushes a mode change effect when aborting.
- ToggleRecordsMode now cleans up empty records upon exiting.
- EnterAdvance now emits AbortChain when at the bottom-right corner.
Co-Authored-By: fiddlerwoaroof/git-smart-commit (gemma-4-26B-A4B-it-UD-Q5_K_XL.gguf)
Implement AbortChain and CleanEmptyRecords effects to allow
short-circuiting effect batches and purging cells with empty coordinates.
Update the App struct to support aborting effects during the application of
an effect batch.
Co-Authored-By: fiddlerwoaroof/git-smart-commit (gemma-4-26B-A4B-it-UD-Q5_K_XL.gguf)
Reflect improvise-8zh: `persistence/` and `import/` now live in the
`improvise-io` sub-crate under `crates/`.
- Sub-crate list expanded to describe improvise-io's scope, its
dependencies (improvise-core, improvise-formula), and the
standalone-build guarantee.
- File Inventory reorganized: "Import layer" + persistence entries
collapsed into a single "I/O crate layers" block with paths rooted at
`crates/improvise-io/src/`.
- Updated line/test counts to current contents.
- Top-level block trimmed (persistence/format no longer live there) and
lib.rs annotated as a re-export facade.
No consumer-facing path changes; `crate::persistence::*` and
`crate::import::*` still resolve via the main crate's re-exports.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Relocate the two I/O module trees into the improvise-io sub-crate
scaffolded in the previous commit:
git mv src/persistence -> crates/improvise-io/src/persistence
git mv src/import -> crates/improvise-io/src/import
The grammar file `improv.pest` moves alongside `persistence/mod.rs`;
the `#[grammar = "persistence/improv.pest"]` attribute resolves relative
to the new crate root and keeps working unchanged.
No path edits inside the moved code: the `crate::model::*`,
`crate::view::*`, `crate::workbook::*`, `crate::format::*`, and
`crate::formula::*` imports inside persistence and import all continue
to resolve because improvise-io's lib.rs re-exports those modules from
improvise-core and improvise-formula, mirroring the pattern improvise-core
uses for `formula`. Verified no `crate::ui::*`, `crate::command::*`,
`crate::draw::*` imports exist in the moved code (per improvise-8zh
acceptance criterion #3).
Main-crate `src/lib.rs` now re-exports `import` and `persistence` from
improvise-io, keeping every `crate::persistence::*` and `crate::import::*`
path in the 4 consumer files (ui/app.rs, ui/effect.rs,
ui/import_wizard_ui.rs, main.rs) resolving unchanged — no downstream
edits needed.
`examples/gen-grammar.rs` had `include_str!("../src/persistence/improv.pest")`;
updated the relative path to the new location under
`crates/improvise-io/src/persistence/`.
Verification:
- cargo check --workspace --examples: clean
- cargo test --workspace: 616 passing (219 main + 190 core + 65 formula + 142 io)
- cargo clippy --workspace --tests: clean
- cargo build -p improvise-io: standalone build succeeds, confirming no
UI/command leakage into the IO crate (improvise-8zh acceptance #2, #3)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Lay groundwork for Phase 3 of the workspace split (improvise-8zh): a new
`crates/improvise-io/` sub-crate that will house the persistence and
import layers once the files move in the next commit.
Only scaffolding here:
- New `crates/improvise-io/Cargo.toml` declares deps on improvise-core,
improvise-formula, and the external crates persistence+import already
use: anyhow, chrono, csv, flate2, indexmap, pest, pest_derive, serde,
serde_json. Dev-deps: pest_meta, proptest, tempfile.
- New `crates/improvise-io/src/lib.rs` re-exports the core modules under
their conventional names (`format`, `model`, `view`, `workbook`,
`formula`) so in the next commit the moved code's `crate::model::*`,
`crate::view::*`, `crate::workbook::*`, `crate::format::*`, and
`crate::formula::*` paths resolve unchanged.
- Root `Cargo.toml` adds the new crate to workspace members and the main
crate's `[dependencies]`, ready to receive the move.
No source files change yet; cargo check --workspace still compiles as
before.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Update keybindings for normal, records-normal, editing, and records-editing
modes to pass the appropriate mode names as arguments to the parameterized
commands.
This ensures that the correct mode is entered when using commands like
`edit-or-drill` , `enter-edit-at-cursor` , `commit-cell-edit` , and
`commit-and-advance-right` .
Co-Authored-By: fiddlerwoaroof/git-smart-commit (gemma-4-26B-A4B-it-UD-Q5_K_XL.gguf)
Make mode-related commands and effects mode-agnostic by passing the target
mode as an argument instead of inspecting the current application mode.
- `CommitAndAdvance` now accepts `edit_mode` .
- `EditOrDrill` now accepts `edit_mode` .
- `EnterEditAtCursorCmd` now accepts `target_mode` .
- `EnterEditAtCursor` effect now accepts `target_mode` .
Update the command registry to parse mode names from arguments and pass
them to the corresponding commands.
Add tests to verify the new mode-passing behavior.
Co-Authored-By: fiddlerwoaroof/git-smart-commit (gemma-4-26B-A4B-it-UD-Q5_K_XL.gguf)
Reflect Phase B of improvise-36h: `model/`, `view/`, `workbook.rs`, and
`format.rs` now live in the `improvise-core` sub-crate under `crates/`.
- Sub-crate list expanded to describe improvise-core's scope, its
dependency on improvise-formula, and the standalone-build guarantee.
- File Inventory reorganized into a single "Core crate layers" block
covering model/view/workbook/format, with paths rooted at
`crates/improvise-core/src/`.
- Updated line/test counts to match current contents (Phase A + merge
with main brought in records mode, IndexMap-backed DataStore, etc).
No architectural change; the main crate's re-exports keep every
`crate::model`/`crate::view`/`crate::workbook`/`crate::format` path
resolving unchanged, so no "How to Find Things" table edits are needed.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Relocate the four pure-data module trees into the improvise-core
sub-crate scaffolded in the previous commit. Phase A already made
these modules UI/IO-free; this commit is purely mechanical:
git mv src/format.rs -> crates/improvise-core/src/format.rs
git mv src/workbook.rs -> crates/improvise-core/src/workbook.rs
git mv src/model -> crates/improvise-core/src/model
git mv src/view -> crates/improvise-core/src/view
The moved code contains no path edits: the `crate::formula::*`,
`crate::model::*`, `crate::view::*`, `crate::workbook::*`,
`crate::format::*` imports inside the four trees all continue to
resolve because the new crate mirrors the same module layout and
re-exports improvise_formula under `formula` via its lib.rs.
Main-crate `src/lib.rs` flips from declaring these as owned modules
(`pub mod model;` etc.) to re-exporting them from improvise-core
(`pub use improvise_core::model;` etc.). This keeps every
`crate::model::*`, `crate::view::*`, `crate::workbook::*`,
`crate::format::*` path inside the 26 consumer files in src/ (ui,
command, persistence, import, draw, main) resolving unchanged — no
downstream edits needed.
Verification:
- cargo check --workspace: clean
- cargo test --workspace: 612 passing (357 main + 190 core + 65 formula)
- cargo clippy --workspace --tests: clean
- cargo build -p improvise-core: standalone build succeeds, confirming
zero UI/IO leakage into the core crate
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Lay the groundwork for Phase B of improvise-36h: a new
`crates/improvise-core/` sub-crate that will house the pure-data core
(model, view, workbook, format) once the files move in the next commit.
Only scaffolding here:
- New `crates/improvise-core/Cargo.toml` mirroring improvise-formula's
structure; declares deps on improvise-formula, anyhow, indexmap, serde.
- New `crates/improvise-core/src/lib.rs` with the single re-export
`pub use improvise_formula as formula;` so that in the next commit the
moved code's `crate::formula::*` paths resolve unchanged.
- Root `Cargo.toml` adds the new crate to the workspace members and the
main crate's `[dependencies]`, ready to receive the move.
No source files change yet; cargo check --workspace still compiles as
before.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Implement a new "Records" mode for data entry.
- Add `RecordsNormal` and `RecordsEditing` to `AppMode` and `ModeKey` .
- `DataStore` now uses `IndexMap` and supports `sort_by_key()` to ensure
deterministic row order.
- `ToggleRecordsMode` command now sorts data and switches to
`RecordsNormal` .
- `EnterEditMode` command now respects records editing variants.
- `RecordsNormal` mode includes a new `o` keybinding to add a record row.
- `RecordsEditing` mode inherits from `Editing` and adds an `Esc` binding
to return to `RecordsNormal` .
- Added `SortData` effect to trigger data sorting.
- Updated UI to display "RECORDS" and "RECORDS INSERT" mode names and
styles.
- Updated keymaps, command registry, and view navigation to support these
new modes.
- Added comprehensive tests for records mode behavior, including sorting
and boundary conditions for Tab/Enter.
Co-Authored-By: fiddlerwoaroof/git-smart-commit (gemma-4-26B-A4B-it-UD-Q5_K_XL.gguf)
Replace `HashMap` with `IndexMap` in `DataStore::cells` to preserve
insertion order. This allows records mode to display rows in the order they
were added. Update `remove` to use `shift_remove` to maintain order during
deletions.
Co-Authored-By: fiddlerwoaroof/git-smart-commit (gemma-4-26B-A4B-it-UD-Q5_K_XL.gguf)
Introduce `ViewFrame` to store both the view name and the `AppMode` when
pushing to the navigation stack. Update `view_back_stack` and
`view_forward_stack` to use `ViewFrame` instead of `String` . Update
`CmdContext` and `Effect` implementations (SwitchView, ViewBack,
ViewForward) to handle the new `ViewFrame` structure. Add `is_editing()`
helper to `AppMode` .
Co-Authored-By: fiddlerwoaroof/git-smart-commit (gemma-4-26B-A4B-it-UD-Q5_K_XL.gguf)
Refactor formula parser tests to use more concise assert!(matches!(...))
syntax. Simplify the formula generator implementation by removing unused
expression variants and using expect() for mandatory grammar rules. Add a
regression test for hyphenated identifiers in bare names.
Co-Authored-By: fiddlerwoaroof/git-smart-commit (gemma-4-26B-A4B-it-UD-Q5_K_XL.gguf)
Workbook::new was setting _Index=Row and _Dim=Column on the default view,
which forced a fresh workbook into records mode on every startup — records
mode auto-activates when both _Index and _Dim sit on axes. The result felt
like broken keybindings: the grid rendered as records instead of an
aggregated pivot, and every navigation key landed in an unexpected spot.
Main fixed this on the Model layer in 709f2df + 6d4b19a (improvise-kos);
the Workbook refactor accidentally reintroduced the pre-fix behavior.
Restore the post-kos defaults: all virtual categories start on Axis::None
(via on_category_added's "_"-prefix rule), then _Measure is bumped to
Axis::Page so aggregated pivot views show one measure at a time. _Index
and _Dim only move onto axes when the user explicitly enters records mode.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Model is now pure data (categories, cells, formulas, measure_agg) with
no references to view/. The Workbook struct owns the Model together
with views and the active view name, and is responsible for cross-slice
operations (add/remove category → notify views, view management).
- New: src/workbook.rs with Workbook wrapper and cross-slice helpers
(add_category, add_label_category, remove_category, create_view,
switch_view, delete_view, normalize_view_state).
- Model: strip view state and view-touching methods. recompute_formulas
remains on Model as a primitive; the view-derived none_cats list is
gathered at each call site (App::rebuild_layout, persistence::load)
so the view dependency is explicit, not hidden behind a wrapper.
- View: add View::none_cats() helper.
- CmdContext: add workbook and view fields so commands can reach both
slices without threading Model + View through every call.
- App: rename `model` field to `workbook`.
- Persistence (save/load/format_md/parse_md/export_csv): take/return
Workbook so the on-disk format carries model + views together.
- Widgets (GridWidget, TileBar, CategoryContent, ViewContent): take
explicit &Model + &View instead of routing through Model.
Tests updated throughout to reflect the new shape. View-management
tests that previously lived on Model continue to cover the same
behaviour via a build_workbook() helper in model/types.rs. All 573
tests pass; clippy is clean.
This is Phase A of improvise-36h. Phase B will mechanically extract
crates/improvise-core/ containing model/, view/, format.rs, workbook.rs.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Virtual categories _Index and _Dim now default to None on new models
instead of being forced onto Row/Column. _Measure defaults to Page
(the natural home for measure filtering). Fixed page_coords builder
to skip Page categories with no items/selection, preventing empty-string
contamination of cell keys.
Made-with: Cursor
Ensure that virtual categories (_Index, _Dim, _Measure) are registered in
the default view with Axis::None. This prevents potential panics when
calling axis_of on these categories and allows users to move them to a
specific axis manually.
Co-Authored-By: fiddlerwoaroof/git-smart-commit (gemma-4-31B-it-UD-Q4_K_XL.gguf)
Refactor the formula parser to assume grammar invariants, replacing
Result-based error handling in tree walkers with infallible functions and
.expect(GRAMMAR_INVARIANT) calls.
This simplification is based on the guarantee that the Pest grammar already
validates the input structure. To verify these invariants and ensure
correctness, extensive new unit tests and property-based tests using
proptest have been added.
Co-Authored-By: fiddlerwoaroof/git-smart-commit (gemma-4-31B-it-UD-Q4_K_XL.gguf)
Replace the manual tokenizer and recursive descent parser with a PEG
grammar using the pest library.
This migration involves introducing a formal grammar in formula.pest and
updating the parser implementation to utilize the generated Pest parser
with a tree-walking approach to construct the AST.
The change introduces a stricter requirement for identifiers: multi-word
identifiers must now be enclosed in pipe quotes (e.g., |Total Revenue|) and
are no longer accepted as bare words.
Tests have been updated to reflect the new parsing logic, remove
tokenizer-specific tests, and verify the new pipe-quoting and escape
semantics.
BREAKING CHANGE: Multi-word identifiers now require pipe-quoting (e.g. |Total Revenue|) and
are no longer accepted as bare words.
Co-Authored-By: fiddlerwoaroof/git-smart-commit (gemma-4-31B-it-UD-Q4_K_XL.gguf)
The records mode column filter excluded all categories starting with '_',
which hid _Measure. Changed to explicitly exclude only _Index and _Dim,
making _Measure visible as a data column. Updated the blank-model editing
test to reflect the new column order (_Measure first, Value last).
Made-with: Cursor
Clean up formatting of a test assertion in app.rs to improve readability.
Co-Authored-By: fiddlerwoaroof/git-smart-commit (gemma-4-31B-it-UD-Q4_K_XL.gguf)
Simplify the commit_cell_value function by extracting its core logic into
specialized helper functions: commit_regular_cell_value, stage_drill_edit,
and commit_plain_records_edit. This improves readability and reduces
nesting.
Co-Authored-By: fiddlerwoaroof/git-smart-commit (gemma-4-31B-it-UD-Q4_K_XL.gguf)
Extract the hardcoded error message "Record coordinates cannot be empty"
into a public constant in effect.rs to avoid duplication and improve
maintainability.
Co-Authored-By: fiddlerwoaroof/git-smart-commit (gemma-4-31B-it-UD-Q4_K_XL.gguf)
