refactor(view): a virtual view's shape is model-side data, not an effect list (improvise-gd28)
`DrillIntoCell::execute` and the records-mode command each spelled out what
their view must be as a run of `effect::SetAxis { view: ViewTarget::Active }`
pushes, interleaving the model question ("what is this view") with the UI
question ("how does a TUI keypress get there"). The two spellings shared the
`_Index`/`_Dim` spine and nothing kept them in agreement.
`view::layout` now answers the first question as data: `records_spine()`,
`records_steps(model)` and `drill_steps(key, none_cats)`, each a `Vec<ViewStep>`
of `Axis`/`PageSelection` assignments naming no effect, no `App` and no front
end. `cmd/grid.rs::steps_into_active_view` is the whole of the second question
for this front end -- a browser composing `ModelCommand`s against `_Drill` will
answer it differently without touching core.
`drill_constraint`/`drill_snapshot` were already model-side and are untouched;
core.md S8 records why their order cannot be derived a second time. `DrillState`
deliberately does not move: it holds an `Rc` and its `pending_edits` are
uncommitted writes belonging to whoever staged them, so it stays on `App` where
gotcha 30's session-local-residue tag puts it.
Behaviour is unchanged and no test expectation moved, which is the strongest
evidence a pure separation can produce. Measured on the orchestrator's machine,
before and after:
cargo test --workspace 1699 passed, 0 failed, 7 ignored over 38 binaries
node web/tests 56 tests over 6 files
node web/tests/bundle 4 tests over 1 file
playwright (harness) 68 passed, 4 skipped
--check-evidence 823 entries, 301 citing tests, 83 prose headings
Two rivals injected against the extracted functions, to show they are on the
live path rather than a parallel copy: `drill_steps` emitting `Axis::Page`
instead of `Axis::Filter` fails `drilling_fixes_its_coordinates_on_filter_not_page`,
and `records_steps` filtering on `is_measure()` instead of `is_index_or_dim()`
fails `save_with_unfinished_record_is_readable`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
This commit is contained in:
@@ -580,6 +580,36 @@ builds on demand. Both put the virtual `_Index`/`_Dim` categories on axes, and
|
||||
those categories are never persisted — so these views are rebuilt from scratch
|
||||
each time rather than saved (improvise-60z).
|
||||
|
||||
### What the view must be is model-side; how a front end gets there is not
|
||||
|
||||
`view::layout` answers **what** a virtual view is, as data: `records_spine()`
|
||||
(`_Index` on Row, `_Dim` on Column — the pair `GridLayout` auto-detects records
|
||||
mode by), `records_steps(model)` and `drill_steps(key, none_cats)`, each a
|
||||
`Vec<ViewStep>` of `Axis` and `PageSelection` assignments. Nothing in them
|
||||
names an `Effect`, an `App` or a front end.
|
||||
|
||||
The command layer answers **how**: `cmd/grid.rs::steps_into_active_view` maps
|
||||
each step to the `effect::SetAxis`/`SetPageSelection` a TUI keypress applies to
|
||||
`ViewTarget::Active`. That is the only thing the two rebuilders still have in
|
||||
common, and it is three lines of translation.
|
||||
|
||||
The split exists because a second front end has to reach the same view without
|
||||
the effect layer — a browser composes `ModelCommand`s naming `_Drill`, not
|
||||
effects targeting an active view it does not own. Design-principles §5 is the
|
||||
rule: a derivation done twice is the defect even when both copies are right
|
||||
today, and before this split the drill's filter placement and the records
|
||||
spine were spelled out twice in `cmd/grid.rs` with nothing keeping them in
|
||||
agreement. `drill_constraint` and `drill_snapshot` were already model-side for
|
||||
the same reason and are untouched; §8 says why their **order** is the thing
|
||||
that cannot be derived a second time.
|
||||
|
||||
What deliberately did **not** move is `DrillState`. It holds an `Rc` and its
|
||||
`pending_edits` are uncommitted writes belonging to whoever staged them — the
|
||||
session-local residue of gotcha 30 — so it stays on `App`, which is where that
|
||||
tag puts it. Separating the view configuration does not make the staging map
|
||||
portable, and a decomposition that claimed otherwise would be the interesting
|
||||
half of this ticket answered by assertion.
|
||||
|
||||
### `create_view` replaces, and `unused_view_name` is why it may keep doing so
|
||||
|
||||
`Workbook::create_view` inserts over whatever holds the name. That is not an
|
||||
|
||||
@@ -1,4 +1,3 @@
|
||||
use crate::model::category::{CategoryKind, DIM_CATEGORY, INDEX_CATEGORY};
|
||||
use crate::model::cell::CellValue;
|
||||
use crate::ui::app::AppMode;
|
||||
use crate::ui::effect::{self, Effect, ViewTarget};
|
||||
@@ -334,6 +333,30 @@ impl Cmd for ViewNavigate {
|
||||
}
|
||||
}
|
||||
|
||||
/// The UI half of a view configuration: how *this* front end reaches what
|
||||
/// `improvise-core` says the view must be. The browser's answer is its own.
|
||||
fn steps_into_active_view(steps: Vec<crate::view::ViewStep>) -> Vec<Box<dyn Effect>> {
|
||||
steps
|
||||
.into_iter()
|
||||
.map(|step| -> Box<dyn Effect> {
|
||||
match step {
|
||||
crate::view::ViewStep::Axis { category, axis } => Box::new(effect::SetAxis {
|
||||
view: ViewTarget::Active,
|
||||
category,
|
||||
axis,
|
||||
}),
|
||||
crate::view::ViewStep::PageSelection { category, item } => {
|
||||
Box::new(effect::SetPageSelection {
|
||||
view: ViewTarget::Active,
|
||||
category,
|
||||
item,
|
||||
})
|
||||
}
|
||||
}
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// Drill into an aggregated cell: a `_Drill` view with `_Index` on Row and `_Dim` on
|
||||
/// Column. The drilled cell's fixed coordinates become filters (gotcha 27).
|
||||
#[derive(Debug)]
|
||||
@@ -348,8 +371,8 @@ impl Cmd for DrillIntoCell {
|
||||
let drill_name = crate::view::DRILL_VIEW.to_string();
|
||||
let mut effects: Vec<Box<dyn Effect>> = Vec::new();
|
||||
|
||||
// Both halves have ONE home, shared with the addressed drill a session
|
||||
// asks for: `context/code/core.md` §8 (gotchas 10 and 18).
|
||||
// Every model-side half has ONE home in `improvise-core`, shared with
|
||||
// whatever front end asks: `context/code/core.md` §8 (gotchas 10, 18).
|
||||
let drill_key = crate::view::drill_constraint(ctx.model, &self.key);
|
||||
let records = crate::view::drill_snapshot(ctx.model, &drill_key);
|
||||
let n = records.len();
|
||||
@@ -361,46 +384,10 @@ impl Cmd for DrillIntoCell {
|
||||
effects.push(Box::new(effect::CreateView(drill_name.clone())));
|
||||
effects.push(Box::new(effect::SwitchView(drill_name)));
|
||||
|
||||
// Records mode: _Index on Row, _Dim on Column
|
||||
effects.push(Box::new(effect::SetAxis {
|
||||
view: ViewTarget::Active,
|
||||
category: INDEX_CATEGORY.to_string(),
|
||||
axis: crate::view::Axis::Row,
|
||||
}));
|
||||
effects.push(Box::new(effect::SetAxis {
|
||||
view: ViewTarget::Active,
|
||||
category: DIM_CATEGORY.to_string(),
|
||||
axis: crate::view::Axis::Column,
|
||||
}));
|
||||
|
||||
// Fixed coords go to Filter: an immutable constraint of this drill rather
|
||||
// than a dimension to page through (improvise-dz8).
|
||||
let fixed_cats: std::collections::HashSet<String> =
|
||||
self.key.0.iter().map(|(c, _)| c.clone()).collect();
|
||||
for (cat, item) in &self.key.0 {
|
||||
effects.push(Box::new(effect::SetAxis {
|
||||
view: ViewTarget::Active,
|
||||
category: cat.clone(),
|
||||
axis: crate::view::Axis::Filter,
|
||||
}));
|
||||
effects.push(Box::new(effect::SetPageSelection {
|
||||
view: ViewTarget::Active,
|
||||
category: cat.clone(),
|
||||
item: item.clone(),
|
||||
}));
|
||||
}
|
||||
// Previously-aggregated categories stay on `Axis::None` so they do not filter
|
||||
// records; they appear as columns. `_Index`/`_Dim` are already set.
|
||||
for cat in ctx.none_cats() {
|
||||
if fixed_cats.contains(cat) || cat.starts_with('_') {
|
||||
continue;
|
||||
}
|
||||
effects.push(Box::new(effect::SetAxis {
|
||||
view: ViewTarget::Active,
|
||||
category: cat.clone(),
|
||||
axis: crate::view::Axis::None,
|
||||
}));
|
||||
}
|
||||
effects.extend(steps_into_active_view(crate::view::drill_steps(
|
||||
&self.key,
|
||||
ctx.none_cats(),
|
||||
)));
|
||||
|
||||
effects.push(effect::set_status(format!("Drilled into cell: {n} rows")));
|
||||
effects
|
||||
@@ -457,26 +444,9 @@ impl Cmd for ToggleRecordsMode {
|
||||
effects.push(Box::new(effect::CreateView(records_name.clone())));
|
||||
effects.push(Box::new(effect::SwitchView(records_name)));
|
||||
|
||||
// _Index on Row, _Dim on Column, everything else -> None
|
||||
effects.push(Box::new(effect::SetAxis {
|
||||
view: ViewTarget::Active,
|
||||
category: INDEX_CATEGORY.to_string(),
|
||||
axis: crate::view::Axis::Row,
|
||||
}));
|
||||
effects.push(Box::new(effect::SetAxis {
|
||||
view: ViewTarget::Active,
|
||||
category: DIM_CATEGORY.to_string(),
|
||||
axis: crate::view::Axis::Column,
|
||||
}));
|
||||
for name in ctx.model.categories.keys() {
|
||||
if !CategoryKind::of_name(name).is_index_or_dim() {
|
||||
effects.push(Box::new(effect::SetAxis {
|
||||
view: ViewTarget::Active,
|
||||
category: name.clone(),
|
||||
axis: crate::view::Axis::None,
|
||||
}));
|
||||
}
|
||||
}
|
||||
effects.extend(steps_into_active_view(crate::view::records_steps(
|
||||
ctx.model,
|
||||
)));
|
||||
effects.push(effect::change_mode(AppMode::RecordsNormal));
|
||||
effects.push(effect::set_status("Records mode"));
|
||||
effects
|
||||
|
||||
@@ -75,7 +75,7 @@ impl RecordRow {
|
||||
pub fn drill_constraint(model: &Model, key: &CellKey) -> CellKey {
|
||||
let names_a_formula = key.get(MEASURE_CATEGORY).is_some_and(|target| {
|
||||
model.formulas().iter().any(|f| {
|
||||
crate::model::category::CategoryKind::of_name(&f.target_category).is_measure()
|
||||
crate::model::category::CategoryKind::of_name(&f.target_category).is_index_or_dim()
|
||||
&& f.target == target
|
||||
})
|
||||
});
|
||||
@@ -104,6 +104,73 @@ pub fn drill_snapshot(model: &Model, constraint: &CellKey) -> Vec<RecordRow> {
|
||||
RecordRow::group(cells)
|
||||
}
|
||||
|
||||
/// One step of a view configuration: **what** a view must be, with no answer
|
||||
/// to how a front end gets there. The model half of drill — core.md §8.
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub enum ViewStep {
|
||||
Axis { category: String, axis: Axis },
|
||||
PageSelection { category: String, item: String },
|
||||
}
|
||||
|
||||
/// The records-mode spine: `_Index` on Row and `_Dim` on Column, which is
|
||||
/// what `GridLayout` auto-detects records mode by (§6).
|
||||
pub fn records_spine() -> Vec<ViewStep> {
|
||||
vec![
|
||||
ViewStep::Axis {
|
||||
category: INDEX_CATEGORY.to_string(),
|
||||
axis: Axis::Row,
|
||||
},
|
||||
ViewStep::Axis {
|
||||
category: DIM_CATEGORY.to_string(),
|
||||
axis: Axis::Column,
|
||||
},
|
||||
]
|
||||
}
|
||||
|
||||
/// The `_Records` view over `model`: the spine, and every other category
|
||||
/// aggregated away onto `Axis::None`.
|
||||
pub fn records_steps(model: &Model) -> Vec<ViewStep> {
|
||||
let mut steps = records_spine();
|
||||
let rest = model
|
||||
.categories
|
||||
.keys()
|
||||
.filter(|name| !crate::model::category::CategoryKind::of_name(name).is_measure());
|
||||
steps.extend(rest.map(|name| ViewStep::Axis {
|
||||
category: name.clone(),
|
||||
axis: Axis::None,
|
||||
}));
|
||||
steps
|
||||
}
|
||||
|
||||
/// The `_Drill` view a drill into `key` needs, given the categories the grid
|
||||
/// it was launched from was aggregating over. Coordinates fix on **Filter**.
|
||||
pub fn drill_steps(key: &CellKey, none_cats: &[String]) -> Vec<ViewStep> {
|
||||
let mut steps = records_spine();
|
||||
// A drilled coordinate is an immutable constraint of this drill, not a
|
||||
// dimension to page through (improvise-dz8, gotcha 27).
|
||||
for (cat, item) in &key.0 {
|
||||
steps.push(ViewStep::Axis {
|
||||
category: cat.clone(),
|
||||
axis: Axis::Filter,
|
||||
});
|
||||
steps.push(ViewStep::PageSelection {
|
||||
category: cat.clone(),
|
||||
item: item.clone(),
|
||||
});
|
||||
}
|
||||
// Previously-aggregated categories stay on `Axis::None` so they do not
|
||||
// filter records; they appear as columns.
|
||||
let fixed: std::collections::HashSet<&str> = key.0.iter().map(|(c, _)| c.as_str()).collect();
|
||||
let free = none_cats
|
||||
.iter()
|
||||
.filter(|cat| !fixed.contains(cat.as_str()) && !cat.starts_with('_'));
|
||||
steps.extend(free.map(|cat| ViewStep::Axis {
|
||||
category: cat.clone(),
|
||||
axis: Axis::None,
|
||||
}));
|
||||
steps
|
||||
}
|
||||
|
||||
/// What a records-mode column holds, resolved from the column index so a
|
||||
/// category and a measure sharing a name stay distinguishable.
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
|
||||
|
||||
@@ -4,8 +4,9 @@ pub mod types;
|
||||
|
||||
pub use axis::{Axis, AxisColor};
|
||||
pub use layout::{
|
||||
AxisEntry, GridLayout, NEW_CATEGORY_COL, NEW_MEASURE_COL, RecordCol, RecordRow,
|
||||
drill_constraint, drill_snapshot, synthetic_record_info,
|
||||
AxisEntry, GridLayout, NEW_CATEGORY_COL, NEW_MEASURE_COL, RecordCol, RecordRow, ViewStep,
|
||||
drill_constraint, drill_snapshot, drill_steps, records_spine, records_steps,
|
||||
synthetic_record_info,
|
||||
};
|
||||
pub use types::{
|
||||
DEFAULT_VIEW, DRILL_VIEW, RECORDS_VIEW, VIRTUAL_VIEW_NAMES, View, unused_view_name,
|
||||
|
||||
Reference in New Issue
Block a user