fiddlerwoaroof/improvise
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge remote-tracking branch 'gh/main' into wave3/protocol-session

Browse Source
This commit is contained in:
Edward Langley
2026-09-20 21:32:02 -07:00
parent 313e02f522 0947fd0c6c
commit ec236da7c5
17 changed files with 319 additions and 49 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/improvise-command/src/command/cmd/cell.rs
+4 -1
View File
@@ -1,3 +1,4 @@
use crate::model::Aggregation;
use crate::ui::effect::{self, Effect};
use super::core::{Cmd, CmdContext};
@@ -140,7 +141,9 @@ impl Cmd for YankCell {
"yank"
}
fn execute(&self, ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
let value = ctx.model.evaluate_aggregated(&self.key, ctx.none_cats());
let value = ctx
.model
.evaluate_aggregated(&self.key, Aggregation::of(ctx.none_cats()));
vec![
Box::new(effect::SetYanked(value)),
effect::set_status("Yanked"),
crates/improvise-command/src/command/cmd/search.rs
+5 -1
View File
@@ -1,3 +1,4 @@
use crate::model::Aggregation;
use crate::model::cell::CellValue;
use crate::ui::app::AppMode;
use crate::ui::effect::{self, Effect, Panel};
@@ -112,7 +113,10 @@ impl Cmd for SearchNavigate {
Some(k) => k,
None => return false,
};
let s = match ctx.model.evaluate_aggregated(&key, ctx.none_cats()) {
let s = match ctx
.model
.evaluate_aggregated(&key, Aggregation::of(ctx.none_cats()))
{
Some(CellValue::Number(n)) => format!("{n}"),
Some(CellValue::Text(t)) => t,
Some(CellValue::Error(e)) => format!("ERR:{e}"),
crates/improvise-command/src/ui/project.rs
+5 -3
View File
@@ -4,10 +4,10 @@
use std::collections::BTreeMap;
use improvise_core::format::{format_f64, format_value, parse_number_format};
use improvise_core::model::Model;
use improvise_core::model::category::MEASURE_CATEGORY;
use improvise_core::model::cell::CellValue;
use improvise_core::model::record::{AggregateHash, RecordHash};
use improvise_core::model::{Aggregation, Model};
use improvise_core::view::layout::{AxisEntry, GridLayout, RecordCol};
use improvise_core::view::{Axis, View};
use improvise_core::workbook::Workbook;
@@ -237,7 +237,9 @@ fn column_totals(
.map(|c| {
let total: f64 = (0..layout.row_count())
.filter_map(|r| layout.cell_key(r, c))
.map(|key| model.evaluate_aggregated_f64(&key, &layout.none_cats))
.map(|key| {
model.evaluate_aggregated_f64(&key, Aggregation::of(&layout.none_cats))
})
.sum();
CellDisplay::number(format_f64(total, comma, decimals))
})
@@ -272,7 +274,7 @@ fn cell_display(
app.model_state
.workbook
.model
.evaluate_aggregated(&key, &layout.none_cats)
.evaluate_aggregated(&key, Aggregation::of(&layout.none_cats))
});
CellDisplay {
text: format_value(value.as_ref(), comma, decimals),
crates/improvise-command/src/ui/viewport.rs
+2 -2
View File
@@ -6,7 +6,7 @@ use unicode_width::UnicodeWidthStr;
use improvise_protocol::header_level_widths;
use crate::format::format_f64;
use crate::model::Model;
use crate::model::{Aggregation, Model};
use crate::view::{AxisEntry, GridLayout};
/// Minimum column width — enough for short numbers/labels + 1 char gap.
@@ -62,7 +62,7 @@ pub fn compute_col_widths(
for (ci, wref) in widths.iter_mut().enumerate().take(n) {
let total: f64 = (0..layout.row_count())
.filter_map(|ri| layout.cell_key(ri, ci))
.map(|key| model.evaluate_aggregated_f64(&key, &layout.none_cats))
.map(|key| model.evaluate_aggregated_f64(&key, Aggregation::of(&layout.none_cats)))
.sum();
let s = format_f64(total, fmt_comma, fmt_decimals);
let w = s.width() as u16;
crates/improvise-core/src/model/mod.rs
+1 -1
View File
@@ -4,4 +4,4 @@ pub mod record;
pub mod symbol;
pub mod types;
pub use types::Model;
pub use types::{Aggregation, Model};
crates/improvise-core/src/model/types.rs
+63 -16
View File
@@ -200,6 +200,25 @@ fn eval_is(context: &CellKey, filter: &Filter) -> f64 {
.is_some_and(|v| v == filter.item.as_str()) as i32 as f64
}
/// Whether a grid hides dimensions a cell key does not name. The emptiness of
/// the view's `Axis::None` list is the whole question — `context/code/core.md`.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum Aggregation {
/// The key names every dimension, so the cell is read as stored.
Exact,
/// Fold every cell the key does not name.
OverHidden,
}
impl Aggregation {
pub fn of(none_cats: &[String]) -> Self {
match none_cats.is_empty() {
true => Aggregation::Exact,
false => Aggregation::OverHidden,
}
}
}
/// Pure-data document model: categories, cells, formulas. It knows nothing
/// about views — see [`crate::workbook::Workbook`] and core.md §10.
#[derive(Debug, Clone, Serialize, Deserialize)]
@@ -476,8 +495,8 @@ impl Model {
})
}
pub fn evaluate_aggregated(&self, key: &CellKey, none_cats: &[String]) -> Option<CellValue> {
if none_cats.is_empty() {
pub fn evaluate_aggregated(&self, key: &CellKey, agg: Aggregation) -> Option<CellValue> {
if agg == Aggregation::Exact {
return self.evaluate(key);
}
@@ -491,8 +510,8 @@ impl Model {
}
/// [`Self::evaluate_aggregated`] as `f64`, with `0.0` for an empty cell.
pub fn evaluate_aggregated_f64(&self, key: &CellKey, none_cats: &[String]) -> f64 {
self.evaluate_aggregated(key, none_cats)
pub fn evaluate_aggregated_f64(&self, key: &CellKey, agg: Aggregation) -> f64 {
self.evaluate_aggregated(key, agg)
.and_then(|v| v.as_f64())
.unwrap_or(0.0)
}
@@ -815,7 +834,7 @@ mod agg_helper_tests {
#[cfg(test)]
mod model_tests {
use super::Model;
use super::{Aggregation, Model};
use crate::model::cell::{CellKey, CellValue};
fn coord(pairs: &[(&str, &str)]) -> CellKey {
@@ -994,7 +1013,7 @@ mod model_tests {
// With Date hidden, it aggregates to SUM.
let none_cats = vec!["Date".to_string()];
let result = m.evaluate_aggregated(&partial_key, &none_cats);
let result = m.evaluate_aggregated(&partial_key, Aggregation::of(&none_cats));
assert_eq!(result, Some(CellValue::Number(150.0)));
}
@@ -1031,6 +1050,19 @@ mod model_tests {
);
}
#[test]
fn a_hidden_dimension_list_reduces_to_one_of_two_states() {
assert_eq!(Aggregation::of(&[]), Aggregation::Exact);
assert_eq!(
Aggregation::of(&["Date".to_string()]),
Aggregation::OverHidden
);
assert_eq!(
Aggregation::of(&["Date".to_string(), "Payee".to_string()]),
Aggregation::OverHidden
);
}
#[test]
fn evaluate_aggregated_no_hidden_delegates_to_evaluate() {
let mut m = Model::new("Test");
@@ -1039,7 +1071,7 @@ mod model_tests {
m.set_cell(coord(&[("Region", "East")]), CellValue::Number(42.0));
let key = coord(&[("Region", "East")]);
assert_eq!(
m.evaluate_aggregated(&key, &[]),
m.evaluate_aggregated(&key, Aggregation::Exact),
Some(CellValue::Number(42.0))
);
}
@@ -1065,7 +1097,7 @@ mod model_tests {
fn status_of(m: &Model) -> Option<CellValue> {
m.evaluate_aggregated(
&coord(&[("Payee", "Acme"), ("_Measure", "Status")]),
&["Date".to_string()],
Aggregation::of(&["Date".to_string()]),
)
}
@@ -1156,14 +1188,14 @@ mod model_tests {
let key = coord(&[("Payee", "Acme"), ("_Measure", "Price")]);
let none_cats = vec!["Date".to_string()];
let result = m.evaluate_aggregated(&key, &none_cats);
let result = m.evaluate_aggregated(&key, Aggregation::of(&none_cats));
assert_eq!(result, Some(CellValue::Number(20.0))); // avg(10, 30) = 20
}
}
#[cfg(test)]
mod formula_tests {
use super::Model;
use super::{Aggregation, Model};
use crate::formula::parse_formula;
use crate::model::cell::{CellKey, CellValue};
@@ -1715,12 +1747,18 @@ mod formula_tests {
m.recompute_formulas(&none_cats);
// Equality says X is zero…
assert_eq!(
m.evaluate_aggregated(&coord(&[("_Measure", "IsZero")]), &none_cats),
m.evaluate_aggregated(
&coord(&[("_Measure", "IsZero")]),
Aggregation::of(&none_cats)
),
Some(CellValue::Number(1.0))
);
// …so dividing by X must be div/0 in the cached path too.
assert_eq!(
m.evaluate_aggregated(&coord(&[("_Measure", "Ratio")]), &none_cats),
m.evaluate_aggregated(
&coord(&[("_Measure", "Ratio")]),
Aggregation::of(&none_cats)
),
Some(CellValue::Error("div/0".into()))
);
}
@@ -2382,7 +2420,7 @@ mod route_then_aggregate_tests {
let uncached = m.evaluate(&north);
m.recompute_formulas(&none_cats);
let cached = m.evaluate_aggregated(&north, &none_cats);
let cached = m.evaluate_aggregated(&north, Aggregation::of(&none_cats));
assert_eq!(uncached, Some(CellValue::Number(11.0)));
assert_eq!(cached, uncached, "the two evaluators must not diverge");
@@ -2397,7 +2435,10 @@ mod route_then_aggregate_tests {
m.recompute_formulas(&none_cats);
for region in ["North", "South"] {
assert_eq!(
m.evaluate_aggregated(&key(&[("Region", region), (MEASURE, "Pred")]), &none_cats),
m.evaluate_aggregated(
&key(&[("Region", region), (MEASURE, "Pred")]),
Aggregation::of(&none_cats)
),
Some(CellValue::Number(11.0)),
"{region}"
);
@@ -2428,7 +2469,10 @@ mod route_then_aggregate_tests {
m.recompute_formulas(&none_cats);
for (region, expected) in [("North", 1.0), ("South", 0.0)] {
assert_eq!(
m.evaluate_aggregated(&key(&[("Region", region), (MEASURE, "Pred")]), &none_cats),
m.evaluate_aggregated(
&key(&[("Region", region), (MEASURE, "Pred")]),
Aggregation::of(&none_cats)
),
Some(CellValue::Number(expected)),
"{region}"
);
@@ -2444,7 +2488,10 @@ mod route_then_aggregate_tests {
m.recompute_formulas(&none_cats);
for (region, expected) in [("North", 14.0), ("South", 200.0)] {
assert_eq!(
m.evaluate_aggregated(&key(&[("Region", region), (MEASURE, "Pred")]), &none_cats),
m.evaluate_aggregated(
&key(&[("Region", region), (MEASURE, "Pred")]),
Aggregation::of(&none_cats)
),
Some(CellValue::Number(expected)),
"{region}"
);
crates/improvise-core/src/view/layout.rs
+3 -3
View File
@@ -1,8 +1,8 @@
use std::rc::Rc;
use crate::model::Model;
use crate::model::category::{DIM_CATEGORY, INDEX_CATEGORY, MEASURE_CATEGORY};
use crate::model::cell::{CellKey, CellValue};
use crate::model::{Aggregation, Model};
use crate::view::{Axis, View};
/// The trailing column that creates a new measure: typing a name into it
@@ -436,7 +436,7 @@ impl GridLayout {
for (ci, cell) in row.iter_mut().enumerate() {
*cell = self
.cell_key(ri, ci)
.and_then(|k| model.evaluate_aggregated(&k, &self.none_cats))
.and_then(|k| model.evaluate_aggregated(&k, Aggregation::of(&self.none_cats)))
.is_some();
}
}
@@ -494,7 +494,7 @@ impl GridLayout {
self.records_display(row, col).unwrap_or_default()
} else {
self.cell_key(row, col)
.and_then(|key| model.evaluate_aggregated(&key, &self.none_cats))
.and_then(|key| model.evaluate_aggregated(&key, Aggregation::of(&self.none_cats)))
.map(|v| crate::format::format_value(Some(&v), fmt_comma, fmt_decimals))
.unwrap_or_default()
}
crates/improvise-formula/src/formula.pest
+4 -2
View File
@@ -20,7 +20,9 @@ formula = { SOI ~ target ~ "=" ~ expr ~ where_clause? ~ EOI }
// to `|Total Revenue|` name the same item.
target = { identifier }
where_clause = { ^"WHERE" ~ identifier ~ "=" ~ filter_value }
// Compound-atomic like `is_expr`, so `sp+` is the real separator: relax it and
// `WHEREAS x = y` reads as a WHERE on a category `AS`.
where_clause = ${ ^"WHERE" ~ sp+ ~ identifier ~ sp* ~ "=" ~ sp* ~ filter_value }
// ---- expressions --------------------------------------------------------
@@ -64,7 +66,7 @@ paren_expr = { "(" ~ expr ~ ")" }
// Aggregates with optional inline WHERE filter inside the parens.
agg_call = { agg_func ~ "(" ~ expr ~ inline_where? ~ ")" }
agg_func = { ^"SUM" | ^"AVG" | ^"MIN" | ^"MAX" | ^"COUNT" }
inline_where = { ^"WHERE" ~ identifier ~ "=" ~ filter_value }
inline_where = ${ ^"WHERE" ~ sp+ ~ identifier ~ sp* ~ "=" ~ sp* ~ filter_value }
// IF(cond, then, else). Comparison is a standalone rule because comparison
// operators are not valid in general expressions — only inside an IF condition.
crates/improvise-formula/src/parser.rs
+29
View File
@@ -463,6 +463,35 @@ mod tests {
assert_eq!(filter.item, "East");
}
#[test]
fn where_does_not_swallow_the_start_of_a_longer_word() {
assert!(parse_formula("X = SUM(Rev WHEREAS x = y)", "Cat").is_err());
assert!(parse_formula("X = Revenue WHEREAS x = y", "Cat").is_err());
assert!(parse_formula("X = Revenue WHEREAS=y", "Cat").is_err());
assert!(parse_formula("X = SUM(Rev WHEREAS=y)", "Cat").is_err());
let inline = parse_formula("X = SUM(Rev WHERE Region = East)", "Cat").unwrap();
assert!(
matches!(&inline.expr, Expr::Agg(_, _, Some(_))),
"{:?}",
inline.expr
);
let trailing = parse_formula("X = Revenue WHERE Region = East", "Cat").unwrap();
assert_eq!(trailing.filter.as_ref().unwrap().category, "Region");
}
/// `sp+` is the boundary, so the keyword needs a space after it exactly as
/// `IS` does — a quoted category no longer abuts it.
#[test]
fn where_requires_a_space_after_the_keyword() {
assert!(parse_formula("X = Revenue WHERE|Region Name|=|East Coast|", "Cat").is_err());
let f = parse_formula("X = Revenue WHERE |Region Name|=|East Coast|", "Cat").unwrap();
let filter = f.filter.as_ref().unwrap();
assert_eq!(
(filter.category.as_str(), filter.item.as_str()),
("Region Name", "East Coast")
);
}
// ── IS, the coordinate predicate (improvise-4zl7) ───────────────────
#[test]
crates/improvise-io/src/persistence/parse.rs
+39 -1
View File
@@ -50,6 +50,29 @@ struct ParsedFile {
views: Vec<PView>,
}
impl ParsedFile {
/// Two `## Category:` sections naming one category are one category. The
/// merge concatenates, so the built order is the order a writer emits.
fn push_category(&mut self, pc: PCategory) {
let PCategory {
name,
items,
group_parents,
} = pc;
match self.categories.iter_mut().find(|e| e.name == name) {
Some(existing) => {
existing.items.extend(items);
existing.group_parents.extend(group_parents);
}
None => self.categories.push(PCategory {
name,
items,
group_parents,
}),
}
}
}
// ── Helpers for walking the pest parse tree ──────────────────────────────────
/// Advance an iterator, returning an error if empty.
@@ -285,7 +308,7 @@ fn walk_file(file: Pair<'_, Rule>) -> Result<ParsedFile> {
Rule::initial_view => {
parsed.initial_view = Some(section_title(&mut pair.into_inner(), "initial_view")?);
}
Rule::category_section => parsed.categories.push(walk_category_section(pair)?),
Rule::category_section => parsed.push_category(walk_category_section(pair)?),
Rule::formulas_section => parsed.formulas.extend(walk_formulas_section(pair)?),
Rule::data_section => parsed.data.extend(walk_data_section(pair)?),
Rule::view_section => parsed.views.push(walk_view_section(pair)?),
@@ -1078,6 +1101,21 @@ mod parser_edge_cases {
assert!(!item_names.is_empty());
}
#[test]
fn two_sections_naming_one_category_round_trip_to_a_stable_group_order() {
let text = "v2025-04-09\n# Test\n## Category: C\n> G1[P1]\n\
## Category: C\n- X[G2]\n> G2[P2]\n";
let fmt = crate::persistence::format_md;
let printed = fmt(&parse_md(text).unwrap());
for owed in ["> G1[P1]", "> G2[P2]", "- X[G2]"] {
assert!(
printed.contains(owed),
"the merge dropped {owed}:\n{printed}"
);
}
assert_eq!(printed, fmt(&parse_md(&printed).unwrap()));
}
#[test]
fn parse_category_with_no_items() {
let text = "v2025-04-09\n# Test\n## Category: Empty\n## Category: Full\n- A\n";
crates/improvise-worker-server/src/worker.rs
+4 -1
View File
@@ -297,7 +297,10 @@ mod tests {
fn value_at(wb: &Workbook, row: usize, col: usize) -> Option<CellValue> {
let layout = improvise_core::view::layout::GridLayout::new(&wb.model, wb.active_view());
let key = layout.cell_key(row, col)?;
wb.model.evaluate_aggregated(&key, &layout.none_cats)
wb.model.evaluate_aggregated(
&key,
improvise_core::model::Aggregation::of(&layout.none_cats),
)
}
/// `Storage::list` is how a browser offers a document picker, and it must
crates/improvise-worker-server/tests/post_message.rs
+3 -1
View File
@@ -5,6 +5,7 @@ use std::path::{Path, PathBuf};
use improvise_command::ui::app::App;
use improvise_command::ui::reduce;
use improvise_core::model::Aggregation;
use improvise_core::model::cell::{CellKey, CellValue};
use improvise_core::view::Axis;
use improvise_core::view::layout::GridLayout;
@@ -190,7 +191,8 @@ fn the_worker_loads_projects_edits_and_persists_over_post_message() {
fn value_at(wb: &Workbook, row: usize, col: usize) -> Option<CellValue> {
let layout = GridLayout::new(&wb.model, wb.active_view());
let key = layout.cell_key(row, col)?;
wb.model.evaluate_aggregated(&key, &layout.none_cats)
wb.model
.evaluate_aggregated(&key, Aggregation::of(&layout.none_cats))
}
/// A second worker over the same backend sees what the first one saved: what a