43 Commits

Author SHA1 Message Date
Ed Langley 50e1314d06 Merge pull request #334 from fiddlerwoaroof/docs/bookkeeping-2026-10-03
CI / Build all bundles (push) Has been cancelled
CI / Test and lint (host) (push) Has been cancelled
CI / Nix build (crate2nix) and the repo-stats pipeline (push) Has been cancelled
CI / wasm32 check (browser bundle crates) (push) Has been cancelled
CI / Browser JS unit tests (node --test) (push) Has been cancelled
CI / Knowledge base consistency (push) Has been cancelled
CI / Lint the harness crate (push) Has been cancelled
CI / Browser harness (OPFS contract + shell) (push) Has been cancelled
CI / Deploy to GitHub Pages (push) Has been cancelled
docs(context): repo-map and kb bookkeeping, 2026-10-03
2026-10-02 23:28:15 -07:00
Claude (bookkeeping) 91d9925494 docs(context): name the tests that guard the registry's name literals
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
2026-10-02 23:21:49 -07:00
Claude (bookkeeping) 942888e0dc docs(context): correct gotchas 15 and 16 and drop the duplicate 8jk6.6 kb fact
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
2026-10-02 23:15:02 -07:00
Claude (bookkeeping) dd1bf96e00 docs(context): fix the inventory rows and stale text the review blocked #334 on
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
2026-10-02 23:07:47 -07:00
Claude (bookkeeping) 1da2695f1f docs(context): repo-map and kb bookkeeping for felo.13, 31oe.2/.3, 8jk6.3/.6 and stale-text fixes
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
2026-10-02 22:57:38 -07:00
Ed Langley d571abbd8a Merge pull request #322 from fiddlerwoaroof/feat/8jk6.3-date-hierarchy
feat(import): a date column imports as one hierarchical category (improvise-8jk6.3)
2026-10-02 22:32:35 -07:00
Ed Langley 6693178530 Merge pull request #316 from fiddlerwoaroof/fix/31oe-cmdlang
fix(command): pipe-quoted arguments and refusals for unknown names (improvise-31oe.2, 31oe.3)
2026-10-02 22:30:46 -07:00
Claude (cmdlang) 8fa504fbdc Merge gh/main into fix/31oe-cmdlang; renumber cli.md §13/§14 to §14/§15
#321 added its own cli.md section 13, so this PR's sections become 14 (pipe
quoting, surplus arguments) and 15 (names the document lacks), with every
citation renumbered. The stale descriptions of the deleted unknown_* helpers in
cli.md, command.md and the test evidence now describe RequireKnown, and section
15 says a refused name stops the rest of a `:` line but not a headless one.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
2026-10-02 22:00:38 -07:00
Claude (8jk63-fix) 581934886d Merge remote-tracking branch 'gh/main' into feat/8jk6.3-date-hierarchy
# Conflicts:
#	context/test-evidence/formula-and-io.md
#	crates/improvise-io/src/import/wizard.rs
#	src/main.rs
2026-10-02 22:00:32 -07:00
Ed Langley 8da5a3101b Merge pull request #333 from fiddlerwoaroof/fix/felo13-layout-quadratic
perf(layout): O(1) axis index, layout and widths cached across keypresses (improvise-felo.13)
2026-10-02 21:55:37 -07:00
Ed Langley 7e87970278 Merge pull request #321 from fiddlerwoaroof/fix/headless-import-refusals
fix(import): headless import refuses instead of silently succeeding (improvise-8jk6.4, 8jk6.5, 8jk6.6)
2026-10-02 21:41:07 -07:00
Claude (cmdlang) c99353777b fix(command): refuse a malformed coordinate on : and let a later segment see an earlier one (improvise-31oe.3)
PR #316 review blockers. A cursor fallback (clear-cell, yank, paste,
drill-into-cell) now answers only for coordinates that were omitted, so
`:clear-cell Type/Clothing Month/Feb junk` is refused instead of clearing the
cursor's cell. The unknown-name checks move from `execute` to a RequireKnown
effect run at apply time, so `:create-view Second . switch-view Second` is
accepted as `improvise cmd` accepts it. Also pins that a quoted `.` is an
argument.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
2026-10-02 21:33:16 -07:00
Claude (8jk63-fix) 0c56a4b9a1 fix(cli): refuse a --split-date or --axis that names nothing it built (improvise-tpm8) 2026-10-02 21:33:01 -07:00
Claude (8jk63-fix) 053699b652 test(cli): split-date and axis naming nothing are refused (improvise-tpm8) 2026-10-02 21:29:56 -07:00
Claude (8jk63-fix) ec9747cc0e fix(import): the date step refuses a spelling clash where the split fix is (improvise-d0n6) 2026-10-02 21:29:30 -07:00
Claude (8jk63-fix) 22d10987bf test(import): the date step refuses a spelling clash (improvise-d0n6) 2026-10-02 21:29:21 -07:00
Claude (imp321) a244553556 docs(import): name the pre-build refusals and the headless test that sees R5 (improvise-8jk6.17)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
2026-10-02 21:28:13 -07:00
Claude (8jk63-fix) 08b7ab0f35 style(import): format and shorten the spelling-clash check (improvise-d0n6) 2026-10-02 20:58:51 -07:00
Claude (8jk63-fix) c3e12842c2 fix(import): refuse a raw date spelled like a generated item or group (improvise-d0n6) 2026-10-02 20:50:01 -07:00
Claude (8jk63-fix) b516567e58 test(import): a raw date spelled like a generated item or group must not merge (improvise-d0n6) 2026-10-02 20:49:26 -07:00
Claude (imp321) 8f93d4e14a Merge commit '8220a065819652bb224d0b25f5724d6e3a5fbbe8' into fix/headless-import-refusals 2026-10-02 20:30:44 -07:00
Claude (cmdlang) 2e90872a40 Merge commit '8220a065819652bb224d0b25f5724d6e3a5fbbe8' into fix/31oe-cmdlang 2026-10-02 20:30:30 -07:00
Claude (cmdlang) ca61bda130 fix(command): refuse a coordinate argument that is not Category/Item instead of dropping it (improvise-31oe.3)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
2026-10-02 20:30:29 -07:00
Edward Langley 0f116ff582 Merge commit '8220a065819652bb224d0b25f5724d6e3a5fbbe8' into feat/8jk6.3-date-hierarchy 2026-10-02 20:30:05 -07:00
Claude (felo13) 49ea84ca5f fix(docs): register the layout evidence pages; test the two gaps the review found (improvise-felo.13)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
2026-10-02 19:18:47 -07:00
Claude (felo13) 14cef311dd docs(evidence): layout cost evidence and measurements; fmt the merged bench (improvise-felo.13)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
2026-10-02 19:01:46 -07:00
Claude (felo13) 3b168a2ec8 perf(grid): keep the whole-grid summary per layout and read a window over it; exact-key probe in DataStore (improvise-felo.13)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
2026-10-02 18:54:47 -07:00
Claude (felo13) b42aae2e08 perf(app): rebuild the layout and remeasure widths only when their inputs moved (improvise-felo.13)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
2026-10-02 18:47:30 -07:00
Claude (felo13) 2f05f6b62d perf(layout): index a data item by position, not by walking the axis (improvise-felo.13)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
2026-10-02 18:46:18 -07:00
Claude (felo13) 311ae96218 test(layout): a lookup and a cursor key do work independent of grid size (improvise-felo.13)
Two of the three fail on purpose: cell_key is linear in the row index and a
cursor key rebuilds the layout. The third guards the invalidation the fix adds.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
2026-10-02 18:44:06 -07:00
Claude (scale-bisect) 13918bb924 bench: scale bisect harness and report (improvise-felo.10)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
2026-10-02 18:19:25 -07:00
Claude (agent-headless-import) fbe83fcc57 feat(import): --strict refuses a skipped record, and the headless suite covers the reviewed gaps (improvise-8jk6.6, improvise-8jk6.17)
- 8jk6.6, the user's ruling on PR #321 ("Add a --strict flag"): the
  default stays exit 0 with the counted warning. With --strict, any
  skipped record is a refusal that exits 1 and writes nothing. The check
  runs before the build and reads ImportPipeline::skip_warning, the same
  function the build's warning now comes from.
- 8jk6.17: the headless suite now sees three faults that only io unit
  tests caught at efaa5050:
  - naming only the first bad formula (two bad --formula flags);
  - dropping CSV decimal measures;
  - import-json reading a CSV through the JSON constructor.
- Docs record the user's three answers.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
2026-09-24 23:27:45 -07:00
Claude (agent-hier) 6b910889c0 Merge commit '44718e8a' into feat/8jk6.3-date-hierarchy
# Conflicts:
#	crates/improvise-io/src/import/wizard.rs
#	tests/headless_cli.rs
2026-09-24 22:47:28 -07:00
Claude (agent-cmdlang) 2c6b751edd Merge commit 'c91cc055c8ad81afd8f055b71c74af6819020637' into fix/31oe-cmdlang
# Conflicts:
#	context/code/cli.md
#	context/test-evidence/cli.md
2026-09-24 22:33:04 -07:00
Claude (agent-hier) 2f5db9ed8e Merge commit '8d3230e68535c78b4e1b56a96255f0cc4a94fae2' into feat/8jk6.3-date-hierarchy
# Conflicts:
#	context/test-evidence/formula-and-io.md
2026-09-24 22:29:50 -07:00
Claude (agent-hier) 195f3b44b8 feat(import): a date column imports as one hierarchical category, the flat split a choice (improvise-8jk6.3)
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
2026-09-24 22:15:14 -07:00
Claude (agent-headless-import) efaa505059 test(import): a folder of only header-only files kept no record and refuses (improvise-8jk6.6)
After merging PR #314, a header-only file's seeded File proposal means the
category check alone no longer refuses an import that kept no record.
nothing_to_import does, and this pins it. The docs now name from_merge
(switched to from_csv in the merge) among the CSV entry points.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
2026-09-24 22:12:11 -07:00
Claude (agent-headless-import) 9b8b061a93 Merge commit 'c91cc055c8ad81afd8f055b71c74af6819020637' into fix/headless-import-refusals
# Conflicts:
#	context/test-evidence/cli.md
#	context/test-evidence/formula-and-io.md
#	crates/improvise-io/src/import/wizard.rs
#	src/main.rs
#	tests/folder_import_views.rs
#	tests/headless_cli.rs
2026-09-24 22:08:36 -07:00
Claude (agent-headless-import) 87f6c96b53 docs(import): CSV text read by role, and the headless import refusals (improvise-8jk6.4, improvise-8jk6.5, improvise-8jk6.6)
context/code/formula-and-io.md gains the argument for text-first CSV and for
the two refusals, and puts the partial-skip exit code to the user.
context/code/cli.md §13 states the headless contract. Evidence entries are
added for the new tests, with notes on the three parser entries whose
premise moved.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
2026-09-24 22:04:49 -07:00
Claude (agent-headless-import) 8bd4b1a77b fix(import): headless import refuses instead of silently succeeding (improvise-8jk6.4, improvise-8jk6.5, improvise-8jk6.6)
- 8jk6.4: the CSV parser yields source text (CsvRecords), and only the
  role reads it as a number. A forced category keeps 00565796 and
  220944e2 verbatim; measures and analysis read the text through
  csv_parser::number_in. CsvRecords reaches a pipeline only through
  ImportPipeline::from_csv, so handing CSV text to the JSON constructor
  no longer compiles.
- 8jk6.5: an import formula that does not parse is refused by name with
  its parse error: the build refuses (headless exits non-zero and writes
  nothing), and the wizard keeps it out of the list with the text still
  in the buffer.
- 8jk6.6: an import that kept no record refuses and says why, from the
  build and from the wizard's review step. A partial skip stays a
  warning with a count and exit 0.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
2026-09-24 21:54:39 -07:00
Claude (agent-cmdlang) 14c13158dd docs(command): record the command-language quoting, arity and unknown-name refusals (improvise-31oe.2, 31oe.3)
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
2026-09-24 21:53:38 -07:00
Claude (agent-cmdlang) c40b6d3215 fix(command): refuse set-axis, set-page and siblings naming what the document lacks (improvise-31oe.2)
set-axis reads its axis at parse time, so an unknown keyword is an argument
error. set-axis, set-page, hide-item, show-item, toggle-group and switch-view
refuse an unknown category, item, group or view by name instead of doing
nothing, so a headless run exits non-zero.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
2026-09-24 21:47:37 -07:00
Claude (agent-cmdlang) 197ce61d65 fix(command): read |pipe quoting| and refuse surplus arguments (improvise-31oe.3)
The command-line lexer reads |…| as one argument through the shared
unescape_pipe_body, anywhere in a word, so a coordinate can quote each half.
Double quotes stay verbatim. Every registration now declares MaxArgs, and the
registry refuses arguments beyond it, naming them, on both arms.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
2026-09-24 21:43:18 -07:00
41 changed files with 4422 additions and 407 deletions
+5
View File
@@ -31,6 +31,11 @@ name = "reshape_observation"
harness = false
test = false
[[bench]]
name = "scale_bisect"
harness = false
test = false
[dependencies]
gp-tree = { path = "crates/gp-tree" }
improvise-core = { path = "crates/improvise-core" }
+2 -2
View File
@@ -40,9 +40,9 @@ improvise import examples/demo.csv \
--no-wizard \
--category Region --category Product --category Customer \
--measure Revenue --measure Cost \
--time Date --extract Date:Month \
--time Date --extract Date:Month --split-date Date \
--axis Region:row --axis Product:row \
--axis Date_Month:column --axis Measure:column \
--axis Date_Month:column --axis _Measure:column \
--axis Customer:page --axis Date:none \
--formula "Profit = Revenue - Cost" \
--name "Acme Sales Demo" \
+8
View File
@@ -0,0 +1,8 @@
import sys; sys.path.insert(0,'bench')
from scale_bisect import run
cfgs=[dict(k=k,prune=0) for k in (16,17,18,26,27)]+[dict(k=k,prune=1) for k in (7,11,12,13,14,21,22)]+[dict(k=1000,n=n,mode='records') for n in (1000,1200,1500,1800,2000,4600,14500)]
for c in cfgs:
c.setdefault('n',100000); c['ops']='open,key,window'
v=[run('/tmp/sb',c,timeout=200) for _ in range(3)]
f=lambda k:'/'.join(f"{r[k][1]:.1f}" if k in r else '-' for r in v)
print({k:c[k] for k in c if k!='ops'},'rows',v[0].get('rows'),'open',f('open'),'key',f('keypress_down'),'win',f('browser_grid_of_top'),'tuiframe',f('tui_frame_grid_of_whole'),flush=True)
+10
View File
@@ -0,0 +1,10 @@
import sys, json; sys.path.insert(0,'bench')
from scale_bisect import run
cfgs=[dict(k=9,prune=0),dict(k=10,prune=0),dict(k=17,prune=0,groups=1),dict(k=26,prune=0,groups=1),
dict(k=8,prune=1),dict(k=13,prune=1,groups=1),
dict(k=1000,n=1300,mode='records'),dict(k=1000,n=4400,mode='records'),dict(k=1000,n=14000,mode='records')]
for c in cfgs:
c.setdefault('n',100000)
for rep in range(3):
r=run('/tmp/sb',c,timeout=300)
print(json.dumps({'cfg':c,'rep':rep,**r}),flush=True)
+38
View File
@@ -0,0 +1,38 @@
"""Drive benches/scale_bisect one configuration per process; parse its lines.
usage: scale_bisect.py BIN k=.. n=.. [cats=..] [prune=..] [mode=..] [ops=..] [--timeout S]
"""
import subprocess, sys, re, json
def run(binary, cfg, timeout=90):
args = [binary] + [f"{k}={v}" for k, v in cfg.items()]
try:
p = subprocess.run(args, capture_output=True, text=True, timeout=timeout)
out, status = p.stdout, "ok"
except subprocess.TimeoutExpired as e:
out, status = (e.stdout or b"").decode() if isinstance(e.stdout, bytes) else (e.stdout or ""), "timeout"
res = {"status": status}
for line in out.splitlines():
if line.startswith("REFUSED"):
res["status"] = "refused"
m = re.match(r"(\w+) min=([\d.]+) med=([\d.]+) max=([\d.]+)", line)
if m:
res[m[1]] = (float(m[2]), float(m[3]), float(m[4]))
for kv in re.findall(r"(\w+)=([\d.]+)", line):
if kv[0] in ("rows", "cols", "maxrss_mb", "improv_mb", "snapshot_kb", "rss_after_open_mb", "cells"):
res[kv[0]] = float(kv[1])
m = re.match(r"json_encode_setcell_updates ms=(.*)", line)
if m:
res["json_setcell"] = m[1]
return res
if __name__ == "__main__":
binary = sys.argv[1]
cfg, timeout = {}, 90
rest = sys.argv[2:]
for a in rest:
if a.startswith("--timeout"):
timeout = int(a.split("=")[1])
else:
k, v = a.split("=", 1)
cfg[k] = v
print(json.dumps(run(binary, cfg, timeout)))
+10
View File
@@ -0,0 +1,10 @@
import sys; sys.path.insert(0,'bench')
from scale_bisect import run
import itertools
OPS=sys.argv[1];
cfgs=eval(sys.argv[2])
cols=sys.argv[3].split(',')
for c in cfgs:
c=dict(c); c['ops']=OPS
r=run('/tmp/sb',c,timeout=int(sys.argv[4]) if len(sys.argv)>4 else 90)
print(c, r['status'], 'rows',r.get('rows'), *[f"{k}={r[k][1]:.1f}" if k in r else f"{k}=-" for k in cols], f"rss={r.get('maxrss_mb')}", flush=True)
+347
View File
@@ -0,0 +1,347 @@
//! One configuration per process: build a sparse document, time the real paths.
//! `scale_bisect k=1000 n=100000 cats=3 prune=1 mode=grid`; see context/test-evidence/scale-bisect.md.
use std::collections::{HashMap, HashSet};
use std::io::Write;
use std::time::Instant;
use improvise::model::cell::{CellKey, CellValue};
use improvise::view::Axis;
use improvise::workbook::Workbook;
use improvise_command::ui::app::App;
use improvise_command::ui::effect::{self, ViewTarget};
use improvise_command::ui::host::Host;
use improvise_command::ui::project::{self, Viewport};
use improvise_command::ui::reduce::caught_up;
use improvise_io::persistence::{format_md, parse_md, save_to};
use improvise_io::storage::MemoryFs;
use improvise_protocol::{
ClientFrame, KeyCode, KeyEvent, KeyModifiers, ModelCommand, RecordHash, render::RecordCell,
};
fn lcg(s: &mut u64, n: usize) -> usize {
*s = s
.wrapping_mul(6_364_136_223_846_793_005)
.wrapping_add(1_442_695_040_888_963_407);
(*s >> 33) as usize % n
}
struct Cfg {
k: usize,
n: usize,
cats: usize,
prune: bool,
mode: String,
groups: bool,
ops: String,
}
fn arg(args: &HashMap<String, String>, k: &str, d: &str) -> String {
args.get(k).cloned().unwrap_or_else(|| d.to_string())
}
fn document(c: &Cfg) -> (Workbook, Vec<CellKey>) {
let mut wb = Workbook::new("bench");
for ci in 0..c.cats {
let cat = format!("C{ci}");
wb.add_category(cat.clone()).expect("under the limit");
for i in 0..c.k {
if c.groups && ci == 0 {
wb.add_item_in_group(&cat, format!("i{i}"), format!("G{}", i / 10))
.expect("added");
} else {
wb.add_item(&cat, format!("i{i}")).expect("added");
}
}
}
for m in ["Cost", "Rev"] {
wb.add_item("_Measure", m).expect("virtual");
}
let space = (c.k as f64).powi(c.cats as i32);
let wanted = c.n.min((space / 2.0).max(1.0) as usize);
let (mut s, mut seen, mut recs) = (0x9E37_79B9_7F4A_7C15u64, HashSet::new(), Vec::new());
while seen.len() < wanted {
let d: Vec<usize> = (0..c.cats).map(|_| lcg(&mut s, c.k)).collect();
if !seen.insert(d.clone()) {
continue;
}
let coords: Vec<(String, String)> = d
.iter()
.enumerate()
.map(|(ci, x)| (format!("C{ci}"), format!("i{x}")))
.collect();
recs.push(CellKey::new(coords.clone()));
for (m, v) in [("Cost", 3.0), ("Rev", 5.0)] {
let mut key = coords.clone();
key.push(("_Measure".to_string(), m.to_string()));
wb.model.set_cell(CellKey::new(key), CellValue::Number(v));
}
}
let view = wb.active_view_mut();
for ci in 0..c.cats {
let axis = if c.mode == "records" || c.mode == "none" {
Axis::None
} else {
Axis::Row
};
view.set_axis(&format!("C{ci}"), axis);
}
if c.mode == "records" {
view.set_axis("_Index", Axis::Row);
view.set_axis("_Dim", Axis::Column);
} else {
view.set_axis("_Measure", Axis::Column);
}
view.prune_empty = c.prune;
(wb, recs)
}
fn maxrss_mb() -> f64 {
let mut u: libc::rusage = unsafe { std::mem::zeroed() };
unsafe { libc::getrusage(libc::RUSAGE_SELF, &mut u) };
u.ru_maxrss as f64 / 1048576.0
}
fn rss_mb() -> f64 {
let out = std::process::Command::new("ps")
.args(["-o", "rss=", "-p", &std::process::id().to_string()])
.output()
.expect("ps");
String::from_utf8_lossy(&out.stdout)
.trim()
.parse::<f64>()
.unwrap_or(0.0)
/ 1024.0
}
/// Sample `f` up to `reps` times, stopping once `budget_ms` is spent (one sample minimum).
fn sample(name: &str, reps: usize, budget_ms: f64, mut f: impl FnMut(usize)) {
let mut v = Vec::new();
let start = Instant::now();
for r in 0..reps {
let t = Instant::now();
f(r);
v.push(t.elapsed().as_secs_f64() * 1000.0);
if start.elapsed().as_secs_f64() * 1000.0 > budget_ms {
break;
}
}
v.sort_by(|a, b| a.partial_cmp(b).expect("finite"));
println!(
"{name} min={:.3} med={:.3} max={:.3} reps={}",
v[0],
v[v.len() / 2],
v[v.len() - 1],
v.len()
);
std::io::stdout().flush().ok();
}
fn once<T>(name: &str, f: impl FnOnce() -> T) -> T {
let t = Instant::now();
let v = f();
println!(
"{name} min={0:.3} med={0:.3} max={0:.3} reps=1",
t.elapsed().as_secs_f64() * 1000.0
);
std::io::stdout().flush().ok();
v
}
fn key(code: KeyCode) -> KeyEvent {
KeyEvent::new(code, KeyModifiers::NONE)
}
fn main() {
let args: HashMap<String, String> = std::env::args()
.skip(1)
.filter_map(|a| {
a.split_once('=')
.map(|(k, v)| (k.to_string(), v.to_string()))
})
.collect();
let c = Cfg {
k: arg(&args, "k", "1000").parse().expect("k"),
n: arg(&args, "n", "100000").parse().expect("n"),
cats: arg(&args, "cats", "3").parse().expect("cats"),
prune: arg(&args, "prune", "1") == "1",
mode: arg(&args, "mode", "grid"),
groups: arg(&args, "groups", "0") == "1",
ops: arg(
&args,
"ops",
"open,key,frame,window,setcell,host,reshape,io",
),
};
let rows_cap: f64 = arg(&args, "rows_cap", "30000000").parse().expect("cap");
let product = (c.k as f64).powi(c.cats as i32);
if c.mode == "grid" && product > rows_cap {
println!(
"REFUSED product_rows={product:.3e} (> {rows_cap:.0}: layout would materialize it)"
);
return;
}
let want = |o: &str| c.ops.split(',').any(|x| x == o);
let (wb, recs) = document(&c);
println!(
"cells={} records={} base_rss_mb={:.0}",
wb.model.data.iter_cells().count(),
recs.len(),
rss_mb()
);
let mut app = once("open", || App::new(wb, None));
println!(
"rows={} cols={} rss_after_open_mb={:.0}",
app.layout.row_count(),
app.layout.col_count(),
rss_mb()
);
app.term_width = 200;
app.term_height = 50;
let name = app.view_name().to_string();
if want("key") {
sample("keypress_down", 8, 6000.0, |_| {
app.dispatch_key(key(KeyCode::Down));
});
}
if want("frame") {
let whole = |app: &App| Viewport {
row_offset: 0,
col_offset: 0,
rows: app.layout.row_count(),
cols: app.layout.col_count(),
};
sample("tui_frame_grid_of_whole", 3, 8000.0, |_| {
std::hint::black_box(project::grid_of(&app, whole(&app)));
});
let cache = project::grid_of(&app, whole(&app));
let area = ratatui::layout::Rect::new(0, 0, 200, 50);
sample("tui_render_gridwidget_buffer", 5, 4000.0, |_| {
let mut buf = ratatui::buffer::Buffer::empty(area);
let w = improvise::ui::grid::GridWidget::new(
&cache,
app.cursor(),
&app.view_state.active_view,
&app.layout,
&app.view_state.mode,
&app.view_state.search_query,
&app.view_state.buffers,
app.drill_state.as_ref(),
);
ratatui::widgets::Widget::render(w, area, &mut buf);
std::hint::black_box(buf);
});
}
if want("window") {
let mid = app.layout.row_count() / 2;
for (label, off) in [("top", 0), ("mid", mid)] {
let vp = Viewport {
row_offset: off,
col_offset: 0,
rows: 45,
cols: 12,
};
sample(&format!("browser_grid_of_{label}"), 5, 6000.0, |_| {
std::hint::black_box(project::grid_of(&app, vp));
});
}
}
if want("setcell") && !recs.is_empty() {
let existing = recs[recs.len() / 2].clone().with("_Measure", "Rev");
sample("setcell_existing", 5, 6000.0, |r| {
app.apply_effects(vec![Box::new(effect::SetCell(
existing.clone(),
CellValue::Number(10.0 + r as f64),
))]);
});
let mut s = 99u64;
sample("setcell_new_record", 5, 6000.0, |_| {
let mut coords: Vec<(String, String)> = (0..c.cats)
.map(|ci| (format!("C{ci}"), format!("i{}", lcg(&mut s, c.k))))
.collect();
coords.push(("_Measure".into(), "Rev".into()));
app.apply_effects(vec![Box::new(effect::SetCell(
CellKey::new(coords),
CellValue::Number(1.0),
))]);
});
}
if want("reshape") && c.mode == "grid" {
let v = || ViewTarget::Active;
sample("reshape_hide_item", 3, 6000.0, |r| {
app.apply_effects(vec![Box::new(effect::HideItem {
view: v(),
category: "C1".into(),
item: format!("i{r}"),
})]);
});
if c.groups {
sample("reshape_toggle_group", 3, 6000.0, |_| {
app.apply_effects(vec![Box::new(effect::ToggleGroup {
view: v(),
category: "C0".into(),
group: "G0".into(),
})]);
});
}
let last = format!("C{}", c.cats - 1);
sample("reshape_set_axis", 4, 6000.0, |r| {
let axis = if r % 2 == 0 { Axis::Page } else { Axis::Row };
app.apply_effects(vec![Box::new(effect::SetAxis {
view: v(),
category: last.clone(),
axis,
})]);
});
sample("reshape_toggle_prune", 2, 6000.0, |_| {
app.apply_effects(vec![Box::new(effect::TogglePruneEmpty(v()))]);
});
}
if want("io") {
let wbr = &app.model_state.workbook;
let text = once("format_md", || format_md(wbr));
println!("improv_mb={:.2}", text.len() as f64 / 1048576.0);
once("parse_md", || parse_md(&text).expect("parses"));
let fs = MemoryFs::new();
sample("persist_per_command_save_to", 3, 6000.0, |_| {
save_to(&fs, wbr, std::path::Path::new("d.improv")).expect("write");
});
}
if want("host") && !recs.is_empty() {
let mut host = Host::new(app);
let (id, snap) = once("host_connect_snapshot", || host.connect(45, 12));
sample("json_encode_snapshot", 3, 4000.0, |_| {
std::hint::black_box(serde_json::to_vec(&snap.msg).expect("json"));
});
println!(
"snapshot_kb={:.1}",
serde_json::to_vec(&snap.msg).expect("json").len() as f64 / 1024.0
);
let rec = recs[recs.len() / 2].clone();
let cell = RecordCell {
record: RecordHash::of(&rec),
measure: Some("Rev".into()),
};
let mut enc: Vec<(f64, usize)> = Vec::new();
sample("host_handle_setcell_and_project", 4, 8000.0, |r| {
let wire = caught_up(
host.app(),
ModelCommand::SetCell {
view: name.clone(),
cell: cell.clone(),
value: CellValue::Number(50.0 + r as f64),
},
);
let out = host.handle(id, ClientFrame::command(wire));
let t = Instant::now();
let bytes: usize = out
.iter()
.map(|a| serde_json::to_vec(&a.msg).expect("json").len())
.sum();
enc.push((t.elapsed().as_secs_f64() * 1000.0, bytes));
});
println!("json_encode_setcell_updates ms={:?}", enc);
}
println!("maxrss_mb={:.0}", maxrss_mb());
}
+146
View File
@@ -392,3 +392,149 @@ The headless path matters most, because the code-maat pipeline
(improvise-1xfb.5.1) runs there. `a_headless_import_of_a_folder_merges_its_csv_files_and_repeats_byte_for_byte`
pins the views, the sparse Data lines and the byte-identical second run, with
the first run's output sitting in the folder it reads.
## 13 — a headless import refuses rather than saving less than was asked (improvise-8jk6.4, 8jk6.5, 8jk6.6)
The user treats the headless command and model layer as a product of its own,
and the scripted `import --no-wizard` is where it meets a pipeline. The
repo-stats pipeline (improvise-1xfb.5.1) found three ways that import saved
something other than what the flags asked for, and still exited 0. The
reasoning for each is in `context/code/formula-and-io.md`, "A CSV field is
text, and the field's role reads it" and "An import refuses what it cannot
keep". The CLI's contract:
- **`--category` keeps the CSV text.** A forced category's items are the
file's text, verbatim: `00565796` stays `00565796`, and `220944e2` stays
`220944e2`. Measures still read as numbers. A JSON number has no source
text, so it is named as JSON writes it (`1e3` is `1000.0`).
- **An unparseable `--formula` is a refusal.** It exits non-zero and writes
nothing. Every bad formula is named on stderr, each with its parse error.
This is §10's rule: the request was not met, so the exit code says so. The
wizard refuses the same text at the formula step, with the same wording.
- **An import that kept no record is a refusal.** It exits non-zero, writes
nothing, and says why: every record was skipped as not a JSON object, or
the source had none.
- **An import that skipped *some* records exits 0 by default.** It writes the
model and warns with a count. With `--strict` (the user's ruling on PR #321)
any skipped record is a refusal instead: exit 1, the same count named, and
nothing written. The argument is in `formula-and-io.md`.
Two refusals bail in `run_headless_import` before the build: the `--strict`
skip check (`src/main.rs:305`) and the `--on-duplicate refuse` collision
check. The rest come from `build_model_with_warnings` as an `Err`. All of
them reach `main` as exit 1 before `persistence::save`. So "writes nothing"
holds by statement order, not by where the refusal is raised. The headless
tests check it by asserting that the output path does not exist.
## 14 — the command language reads `|pipe quoting|`, and a surplus argument is refused (improvise-31oe.3)
`create-view |By area|` used to make a view named `|By` and drop `area|` without
a word. Two separate defects, fixed separately.
**The quoting.** `parse.rs` now has one lexer, `lex`, and everything else is
built on it: `parse_line_with`, `segments` and `parse_segment_interactive`. The
rules are:
- A `|…|` piece is read by `improvise_formula::quote::unescape_pipe_body`, the
one home of pipe quoting (gotcha 34). So `\|`, `\\` and `\n` mean exactly what
they mean in `.improv` and in formulas. The lexer only finds the closing pipe:
a backslash takes the next character with it, so `\|` does not close.
- A piece may sit **anywhere in a word**, and the pieces and bare text are
joined. So a coordinate quotes each half, `|Sales Region|/|East Coast|`, the
way a `.improv` data line quotes each name. The alternative, a pipe only at
the start of a word, would have left no way to spell a coordinate with a
space in it.
- `"…"` is **kept**, verbatim, with no escapes, and a `|` inside it stays a
`|`. It is not a workaround to retire. Formula text carries its own pipes:
`add-formula "Net = |Gross Rev| - Cost"` must reach `parse_formula` with the
pipes on. If the line had pipes only, every pipe in a formula would need
escaping. Its old leniency is kept too: a `"` that is never closed runs to
the end of the line. A `|` that is never closed is refused, naming the text
from the open pipe on, because a name is never meant to run to the end.
- The separator is a **bare** `.` word. A quoted `|.|` or `"."` is an argument,
and a `.` inside `|a . b|` does not split the line. The old `split_on_dot`
was a second scanner that knew double quotes and not pipes. With one lexer
there is no second scanner to disagree.
- A comment line (`#`, `//`) is not lexed, so an unmatched pipe in a comment is
not an error.
`segments` now returns `Result`, because an unterminated pipe is a fact about
the whole line. `main.rs` refuses the line before running any of it, as §7 does
for a parse error, and `ExecuteCommand` refuses it on the status line.
What is still not spellable: an item whose name contains `/`. The coordinate is
split on the first `/` **after** unquoting, so `|a/b|/x` is still read as
category `a` and item `b/x`. Closing that needs the lexer to hand structured
coordinates to the commands, which is a larger change than this ticket.
**The arity.** Every registration now declares `MaxArgs::AtMost(n)` or
`MaxArgs::Any` beside its prototype. `CmdRegistry::parse` and
`CmdRegistry::interactive` apply it after construction succeeds. So the rule is
written once and every command gets it, and a new registration cannot compile
without a declaration. The refusal names the command, the limit and every
surplus argument (`create-view: takes at most 1 argument(s), got 2; unexpected
"area"`). A headless run exits non-zero through the §7 parse gate, and the `:`
line refuses on the status line.
It runs **after** the command's own parse, deliberately. A command whose own
refusal states its shape keeps it: `add-formula`'s "quote the formula"
(improvise-z05h) is more use than an arity, and it is what `formula Product
Total = Widget + Gadget` still says.
`MaxArgs::Any` is a claim, and the list of commands making it is short:
`add-items`, `set-cell`, `set-format` (it joins its words), `clear-cell`,
`yank`, `paste`, `drill-into-cell` and `commit-cell-edit`. Each takes a list of
items or coordinates. `no_command_silently_drops_an_argument_it_was_given` names
that list, so widening another command to `Any` has to add it there.
## 15 — a command that names something the document lacks is refused (improvise-31oe.2)
`set-axis Regoin row`, `set-page Region Nowhere` and `set-axis Region sideways`
all exited 0 and said nothing. §10's rule applies: nothing changed, so the
request was not met.
- **An unreadable axis is an argument error.** `set-axis` now checks its axis
with `parse_axis` when it is parsed, so a script is refused before any of it
runs. `execute` still refuses an axis it cannot read rather than returning
nothing. That arm cannot be reached through the registry.
- **An unknown category, item, group or view is a refusal**, from `RequireKnown`
(below), because only the document knows what it holds. `effect::unknown_name`
asks the active view (`try_axis_of`) for a category. That is exactly the
condition under which `View::set_axis` was a silent no-op. It asks
`effective_item_names` for an item, so a formula target is a known `_Measure`
item. It asks both an item's own group and the category's `groups` for a
group. `add_item_in_group` registers no `Group`, and a parent group has no
items, so either one alone misses a real group. A view is asked of
`workbook.views`.
The siblings fixed with it: `hide-item` and `show-item` (hiding an unknown item
wrote it into `hidden_items`), `toggle-group`, and `switch-view` (the effect
ignores a view it lacks, and says nothing). `set-page` with an unknown item was
worse than a no-op: it wrote the name into `page_selections`.
The check runs at **apply** time, as an effect of its own. `execute` runs
against one context built before the line ran, and a `:` line builds every
segment up front, so a check in `execute` refused
`create-view Second . switch-view Second` and
`add-item Type Tools . set-page Type Tools` after their first halves had landed,
while `improvise cmd` (which rebuilds the context per segment) accepted them.
Each of these commands therefore returns `RequireKnown(needs)` followed by the
real effect. `RequireKnown` refuses and aborts the batch (gotcha 21), so the
real effect never applies. It is a `ViewOnly` effect that writes nothing and is
off the wire on purpose: the wire's own effects (`model_effects`, gotcha 38)
stay non-refusing, since a refusing effect there becomes a frame rejection
(`MutationRefused`). That is arguably right, but it changes the protocol, and
a browser composes these only from names it was sent. It is left for a ticket
of its own.
A refused name stops the rest of a `:` line, because the abort ends the one batch
the whole line runs as. Headless runs each segment as its own batch, so an abort
never crosses a `.` there: `:set-page Type Nowhere . add-item Type X` adds
nothing, while `improvise cmd` on the same line exits 1 and still adds `X`.
`load`/`Save` already behaved this way. The two front ends' chain semantics are
left for a ticket of their own.
The interactive cursor fallbacks of `clear-cell`, `yank`, `paste` and
`drill-into-cell` answer only for coordinates that were OMITTED
(`only_when_omitted`). Coordinates that were given and are malformed are
refused, never replaced by the cursor's cell.
+18 -2
View File
@@ -2996,8 +2996,10 @@ ride in `ViewState::transient_prefix`, so they must live where the wire types do
and the re-export keeps `crate::command::key::…` paths working.
`parse.rs` is the quasi-lisp prefix parser: `word arg1 arg2 …`, several commands per
line separated by `.`, coordinate pairs written `Category/Item`, and quoted strings
(`"Profit = Revenue - Cost"`).
line separated by `.`, coordinate pairs written `Category/Item`, verbatim quoted
strings (`"Profit = Revenue - Cost"`) and `|pipe-quoted names|` read by
`improvise_formula::quote`. One lexer serves every entry point, so the separator
and the quoting cannot disagree (improvise-31oe.3, `context/code/cli.md` §14).
### `cmd/core.rs`
@@ -3014,6 +3016,12 @@ writes any other field, so an accepted argument list is never overridden by
context (improvise-2bac, improvise-yf7p; the argument is `context/code/cli.md`
§8). `register_pure` is the case with no fallback: both arms are `parse`.
Every registration also declares its `MaxArgs`, and the registry refuses surplus
arguments on both arms after the command is built. `register_nullary` declares
`AtMost(0)` for itself, and an `effect_cmd!` declares its limit in the macro
call as `MAX_ARGS`. So the arity rule is written once and is total over the
registry (improvise-31oe.3, `context/code/cli.md` §14).
`write_buffer` is the effect that writes a named buffer — the mirror of
`read_buffer`, and the only place that knows "search" is not an entry in the buffers
map but its own `ViewState` field (improvise-gdl, gotcha 11). That knowledge used to
@@ -3372,6 +3380,14 @@ The parseable model-mutation commands are thin `Cmd` wrappers around effects,
constructible from string args, sharing the same execution path as
keymap-dispatched commands (gotcha 16).
A wrapper that names a category, item, group or view returns
`effect::RequireKnown(Needs)` followed by its real effect (improvise-31oe.2).
`RequireKnown` asks `unknown_name` against the live app when the batch applies
and refuses and aborts if the document lacks the name, so a `:` line sees what
its earlier segments made. The effects themselves stay silent about unknown
names, because they are also the wire's mutation path. Why the check is a
separate effect: `context/code/cli.md` §15.
### `cmd/wizard.rs` (improvise-2wm, improvise-907)
Import-wizard key handling. The wizard is a small step machine, and which key means
+165
View File
@@ -613,6 +613,59 @@ different row order on screen, and a different detected date format, since
varied. The `HashSet` in `analyze_records` is there to deduplicate and **never
to iterate**.
### A date column imports as ONE hierarchical category (improvise-8jk6.3)
The user: "I think dates should usually be one category and we can use
formulas to crosstab." So the default `DateShape::Hierarchy` builds one
category named after the column, where the old import built up to three flat
ones beside it (`Date_Year`, `Date_Quarter`, `Date_Month`):
- **Its items are the finest chosen component**, spelled so they stay unique
across years: `2024-01`, `2024-Q1`, `2024`, which are
`extract_date_component`'s existing spellings. A month is never `Jan`.
- **The coarser chosen components are its groups**, coarsest first
(`DateComponent::COARSE_TO_FINE`): a month sits in its quarter, and the
quarter's `>` line names its year. A level left unchosen is skipped rather
than drawn empty, so Year + Month groups months straight into years.
- **It opens folded**, one row per year, with nothing to set, because every
group starts closed (`context/code/core.md` §19).
- **It costs one of the category slots**, not up to four, and
`proposed_category_count` counts it that way.
- **The raw dates are gone from that category.** The day is not a component, so
two records of one month land on one cell and the collision step asks, as it
does for any other collision. `Split` keeps the day.
- **A value the detected format cannot read keeps its raw text** as an
ungrouped item, with a warning naming the column and the row count, rather
than being dropped. The warning is separate from the split shape's, because
that one reports an absent coordinate and this one a coordinate that is there
and ungrouped. **Unless its spelling is one the hierarchy generates**
(improvise-d0n6): a raw `2024-01` or `2024` is the same name as the generated
month or year, so it would join the grouped item and the warning would be
false. The build **refuses**, naming the column and the spellings and pointing
at the split shape. That follows the collision rule below: where two records
would silently be one thing, the user is asked and nothing is defaulted. A
fallback to the split shape was rejected as a silent change of the user's
choice, and a renamed raw item as altering the data. The check is
`Plan::spelling_clash`.
- **Item order is first-seen, not chronological.** Order is a correctness
property here (gotcha 18), and a group need not be contiguous, so a CSV that
is not in date order still nests correctly. It is just drawn in the order
it was read.
`DateShape::Split` is the old behaviour, unchanged, and stays a choice. `s` on
the wizard's date step toggles the field under the cursor, and the header line
for each field says which it will get ("one category: Year > Quarter > Month"
or "split: Date, Date_Year, …"). It is worded once, in
`project.rs::date_shape_sentence`. Headless, `--split-date <field>` does the
same, and `--extract` keeps its meaning: which components to build.
**Crosstabs by formula are not available yet.** A formula can name a month
(`SUM(Amount WHERE Date = "2024-01")`, `Date IS "2024-01"`), but a `WHERE` or
`IS` naming a GROUP matches no record: both compare the coordinate to the name
exactly. `SUM(Amount WHERE Date = "2024")` is an empty sum, and `Date IS "2024"`
is 0 on every month of 2024. That was measured on this branch and reported for
filing, not built here.
### The ambiguous-date heuristic reads American-first, deliberately
`01/02/2025` parses under both `%m/%d/%Y` and `%d/%m/%Y` and nothing in the data
@@ -658,6 +711,118 @@ one proves nothing.
backend rather than only a file (improvise-fuj); the path-taking wrappers remain
for the native CLI.
### A CSV field is text, and the field's role reads it (improvise-8jk6.4)
The parser used to type every field as it read it: text that parsed as an
`i64` or a finite `f64` became a `Value::Number`. That is a lossy step, and it
ran before anyone knew what the column was for. A column the user then forced
to Category (`--category`, or the wizard's `c`) took its item names from the
number's display, not from the file. So commit `00565796` became item `565796`,
and `220944e2` was read as scientific notation and became `22094400.0`. The
repo-stats pipeline (improvise-1xfb.5.1) found it.
CSV has no types. Whether `00565796` is a number depends on whether its column
is a measure, and the parser cannot know that. So the parser now yields every
non-empty field as a `Value::String` of its text (blank is still `Null`), and
two readers apply the old numeric rule, `csv_parser::number_in`, only where a
number is wanted:
- **Analysis** proposes a kind from a typed copy of the records
(`Scalars::typed_record`), so a numeric CSV column is still proposed as a
Measure, exactly as before.
- **A measure** takes its value through `Scalars::number`.
A category or a label is named from the text itself, so it is verbatim.
`ImportPipeline` carries the reading as a private `scalars: Scalars`:
`Typed` for JSON and `Text` for CSV. JSON cannot share the text rule, because
a JSON string is text by the file's own say. `"02139"` in JSON is a zip code,
and reading it as a number would propose it as a Measure. A JSON number
forced to Category has no source text to keep, because serde_json does not
keep one. It is named by `Value`'s own display, which is what writing it back
as JSON would say: `42`, `1.5`, and `1000.0` for `1e3`.
**The parser returns `CsvRecords`, not `Vec<Value>`.** It derefs to
`[Value]`, so a caller can read it. It converts to a `Vec` only inside the
crate, and the only public way into a pipeline is `ImportPipeline::from_csv`.
This is not caution for its own sake. When the parser first changed, eleven
tests in four files failed. Every one handed parser output to
`ImportPipeline::new`, the JSON constructor, which would read every column as
text and propose `Revenue` as a Category. Production had the same shape at all
three CSV entry points: `main.rs`'s single file, `ImportPipeline::from_merge`
(the merge path, from PR #314), and `ImportJsonHeadless`. `from_merge` merged
in with `Self::new` and now calls `Self::from_csv`. Now that mistake does not
compile. Two holes remain, and
both take a deliberate step. `ImportPipeline::records` is still a public
`Vec<Value>`, and a test helper once copied it out of a CSV pipeline into a
JSON one. And `CsvRecords` derefs to a slice that `.to_vec()` will copy.
### An import refuses what it cannot keep (improvise-8jk6.5, improvise-8jk6.6)
Two failures used to end in an import that saved and reported success.
**A formula that does not parse.** `build_model_with_warnings` ran
`if let Ok(formula) = parse_formula(..)`, so a bad `--formula` vanished. The
headless run then saved and exited 0, and the wizard kept the bad text in its
list and dropped it the same way at build. Both now go through one
function, `import_formula`, whose error names the formula and carries pest's
parse error: `Formula error in 'Y = x +': …`. The prefix matches the TUI
formula minibuffer's `Formula error:`.
- `ImportPipeline::parsed_formulas` parses them all. If any fails, it refuses
and names **every** formula that failed, one per line (a message the user
needs is never truncated). The build calls it before creating anything, so a
refusal writes nothing.
- The wizard checks at `confirm_formula`, the one place the user can still fix
the text (gotcha 22). A formula that does not parse stays in the buffer with
the error as the step's message, and never enters the list.
**Nothing kept.** The one skip left after improvise-8jk6.2 is a record that
is not a JSON object. When every record is skipped, there are no fields, so
there are no proposals and no categories. The build did refuse that case, but
by accident, with "At least one category must be accepted". That tells the
user to change a field's kind when there are no fields. The ticket's own repro
(`[1, 2, "three"]`) already exited 1 on the lpzu base, so the headline did not
reproduce there. What did reproduce was the reason. `nothing_to_import` now
states the rule and runs before the category check: `Nothing to import: all 3
records were skipped as not JSON objects`, or `the source has no records`. The
wizard asks it first on `ReviewProposals`, where another array path can still
be chosen.
One case meets PR #314's "a header-only file is still a file". A folder whose
CSVs are **all** header-only has no records, only sources. Built from
`from_merge`'s seeded proposals alone, it would save `File` items, empty
columns and one empty records view per file. It is refused here, because it
kept no record. A header-only file *beside* files with rows is unaffected,
since some record was kept. The user confirmed this on PR #321: "Refuse (as
#321 does)".
**A partial skip stays a warning.** Some records kept and some skipped still
builds, exits 0, and warns with a count: `Skipped 1 of 3 records that were not
JSON objects`. The argument: a non-object in an array of records is not a
record that failed to import. It is structurally not a record, such as a
trailing `null` or a stray scalar in a hand-edited file. Every record that
*is* a record was kept, and the count is on stderr for a script to read.
Making it fatal would fail every import that carries one stray element.
Making it a distinct non-zero exit that still writes gives a contract where
`import … && next` stops even though the file was written.
The user ruled on PR #321: "Add a --strict flag". The default stays exit 0
with the counted warning, pinned by
`an_import_that_skips_some_records_warns_with_a_count_and_still_writes`.
`import --no-wizard --strict` turns any skipped record into a refusal:
`--strict: Skipped 1 of 3 records that were not JSON objects`, exit 1.
**A strict refusal writes nothing.** A refusal that still wrote the file would
be the half-met contract argued against above: `&&` stops while the output
sits there, and a script that checks only the file gets a model that `--strict`
existed to prevent. So the check runs in `run_headless_import` before the
build. It reads `ImportPipeline::skip_warning`, the same function the build's
warning comes from, so the two cannot disagree about what was skipped.
"Skipped" here means a record that is not an object, the only skip left after
8jk6.2. An unparsed date or a short CSV row is a warning about a record that
was kept, and `--strict` does not refuse it.
### A folder of CSVs is one sparse model, with a view per file (improvise-lpzu)
`improvise import <folder>` exists for improvise-1xfb.5.1. code-maat writes one
+37 -24
View File
File diff suppressed because one or more lines are too long
+200
View File
@@ -448,6 +448,57 @@ Rivals R2, R7 and R9 each fail it. Since the review of `4e68372c` the fixture
also holds a header-only `soc.csv`. Its view must carry `File` at `soc`, be a
records view, and draw zero rows. Rivals F7 and F8 fail that half.
### a_csv_column_forced_to_category_keeps_its_text_verbatim
improvise-8jk6.4. Before the fix, `commit,lines` with `00565796` and
`220944e2` forced to Category imported as items `565796` and `22094400.0`.
The CSV parser had typed both as numbers, and the category was named from the
number. On the base this failed with the items line `- |565796|, |22094400.0|`.
The test also asserts both Data lines, which proves the measure `lines` still
reads 3 and 5 through the same records.
- **R1**, a category named from the typed value, fails it.
- **R2**, a CSV measure read as a JSON value (`as_f64` on the text), fails it
on the Data lines.
### a_json_number_forced_to_category_is_named_as_json_writes_it
improvise-8jk6.4, the JSON half. This is not a bug demonstration: it passed
on the base, and it pins that the fix left JSON alone. JSON keeps no source
text for a number, so `42`, `1.5` and `1e3` name items `42`, `1.5` and
`1000.0`. `1e3` is the discriminating value, because its source text, its f64
display (`1000`) and its JSON display (`1000.0`) all differ. **R10**, naming a
number by its `f64` display, fails it on `1000.0`.
### an_import_formula_that_does_not_parse_refuses_names_it_and_writes_nothing
improvise-8jk6.5. Before the fix, `--formula 'Y = x +'` was dropped: the run
printed `Saved to …` and exited 0. That is the base failure (`a dropped
formula exited 0`). The test passes a good formula too (`Z = x`) and asserts
it is not named, so a refusal that lists every formula cannot pass.
`expected` comes from pest's error, which proves the parse error is carried
and not only the formula text. **R4**, a build that drops what does not
parse, fails it. Since improvise-8jk6.17 it passes two bad formulas, so
**R5**, naming only the first, fails it here too. At `efaa5050` only the io
unit test saw R5.
### an_import_that_keeps_no_record_refuses_and_says_why
improvise-8jk6.6. On the lpzu base `[1, 2, "three"]` already exited 1, so the
exit-code half was not the bug. The reason was: `At least one category must
be accepted`, which is true only because no fields exist, and which points
the user at a fix that cannot help. The base failure was on the message.
**R7**, removing `nothing_to_import` from the build, fails it the same way.
### an_import_that_skips_some_records_warns_with_a_count_and_still_writes
improvise-8jk6.6, the partial case. It passed on the base. It pins the
default the user kept on PR #321 ("Add a --strict flag"), so that changing
the default is a decision and not a drift. It asserts the warning's count (`1 of 3`), exit 0,
and a Data line from the record *after* the skipped one. That last check
shows the skip did not end the import early. **R9**, making any skip fatal,
fails it.
### a_folder_of_one_csv_still_gets_a_file_category_and_its_view_but_a_lone_file_does_not
improvise-lpzu, after the user's ruling on PR #314 ("we'd want file identity").
@@ -458,3 +509,152 @@ other half pins the direction the ruling did *not* change: the same CSV named
directly imports without any `File`. The rival that merges every all-CSV
import, lone files included, fails that half ("a lone file keeps today's
shape"). So the test separates "a folder merges" from "everything merges".
### a_pipe_quoted_argument_is_one_argument_without_its_pipes
improvise-31oe.3. Red before the fix: the old tokenizer split on whitespace
only, so `create-view |By area|` built `CreateViewCmd(["|By", "area|"])`. It
asserts the whole built command, so a lexer that joined the words but kept the
pipes fails as well as one that split them. The second case has two quoted
arguments side by side, so a lexer that reads one pipe piece per line fails.
Rival: `bare_word` pushing `|` as a plain character. Fails here and in
`a_coordinate_may_pipe_quote_either_half`.
### a_pipe_quoted_argument_reads_the_escapes_improv_does
The three escapes `pipe_quote` writes, `\|`, `\\` and `\n`, each in its own
case. Red before the fix. Rival: `pipe_body` returning the raw body without
`unescape_pipe_body`. Fails here, at the first case, and nowhere else in the
crate, which is why the case exists. A lexer that treated `\|` as a closing
pipe would also fail the first case, because the pipe after `b` then opens a
piece that never closes.
### a_command_given_more_arguments_than_it_takes_is_refused
improvise-31oe.3's second half. Red before the fix: `create-view By area`
parsed and dropped `area`. It asserts the refusal opens with the command's name
and quotes the surplus word, and that the pipe-quoted spelling of the same line
is the control that parses. Rival: `MaxArgs::admit` answering `Ok` always.
Fails here, in `no_command_silently_drops_an_argument_it_was_given`, and in
`a_colon_line_naming_something_missing_refuses_on_the_status_line`.
### no_command_silently_drops_an_argument_it_was_given
The rule is one rule only if every registration obeys it, so this enumerates
every registered name against every candidate argument list, on both arms. For
each list a command accepts, it appends **four** surplus words. One would be
absorbed by an optional argument (`add-formula ["1"]` plus one more is a valid
two-argument call), and four is more than any optional span in the registry.
The last word must be named in the refusal. The exceptions are stated, not
inferred: the `Any` list, and three commands whose own shape refusal runs
first. Rivals, each run: declaring `add-item-in-group` as `Any` fails here,
naming the dropped list. Removing the check from the interactive arm alone
fails here and in the `:` line test. A test of the pure arm alone would have
missed that.
### a_command_naming_something_the_document_lacks_is_a_refusal
improvise-31oe.2 and its siblings: `set-axis`, `set-page`, `hide-item`,
`show-item`, `toggle-group` and `switch-view`, each naming a missing category,
item, group or view. Red before the fix at the first row, `set-axis Regoin
column`, with an empty status. Each row asserts the refusal flag, the missing
name in the message, and that the document's `.improv` text and the active view
are unchanged. The last two catch a refusal written after the write. The
control is `a_command_naming_what_the_document_has_is_not_refused`, whose group
row is the one that caught a group check reading only `Category::groups`:
`add_item_in_group` registers no `Group`. The checks are `effect::unknown_name`,
run by `RequireKnown` when the batch applies. Rivals, each run: the
`Needs::Item` arm answering `None` fails at `set-page ["Type", "Nowhere"]`. The
category test (`try_axis_of(..).is_none()`) never firing fails at the first
row. The `Needs::View` arm answering `None` fails at the `switch-view` row.
`RequireKnown` placed after the write, or refusing without `refuse_and_stop`'s
abort, fails the document-unchanged assertion.
### a_colon_line_naming_something_missing_refuses_on_the_status_line
The TUI path for both tickets, through `ExecuteCommand` and `App::apply_effects`,
not through the command alone. It asserts the refusal flag, the name on the
status line, and the mode back at Normal. Rivals: an unknown category accepted
fails at `:set-axis Regoin row`, and the arity check removed from the
interactive arm fails at `:create-view By area`.
### an_unknown_category_axis_or_item_exits_non_zero_and_names_it
The three repros from improvise-31oe.2, at the process boundary. Red against
the `gh/main` source with this test file: `set-axis Regoin row` exited 0 with an
empty stderr. The control, `set-page Region East`, must exit 0. Without it, a
fix that refused every `set-page` would pass.
### a_pipe_quoted_name_reaches_the_document_whole
improvise-31oe.3's repro at the process boundary: the saved document must carry
`## View: By area` and no `|By`. The second half, `create-view By area`, must
exit non-zero naming `"area"`. Red against the `gh/main` source: the document
carried the view `|By`.
### command::cmd::registry::tests::a_malformed_coordinate_is_refused_not_dropped
Codex's P2 on PR #316: `set-cell` and `parse_cell_key_from_args` (`clear-cell`,
`yank`, `paste`, `drill-into-cell`, `commit-cell-edit`) built coordinates with
`filter_map(split_once('/'))`, so `set-cell 1 Type/Food unexpected` dropped
`unexpected` without a word. The test names every caller, a half-empty
coordinate, and a control line that must still parse. Rival: the `filter_map`
restored in `parse_cell_key_from_args` fails at the first line,
`set-cell 1 Type/Food unexpected dropped "unexpected" without a word`.
### command::cmd::text_buffer::tests::colon_refuses_a_malformed_coordinate_and_leaves_the_cursor_cell_alone
The PR #316 review's blocker: on `:`, `parse_cell_key_from_args` failing sent
`clear-cell`, `yank`, `paste` and `drill-into-cell` into the cursor fallback,
which ignores its arguments. `:clear-cell Type/Clothing Month/Feb junk` cleared
the cursor's Food/Jan without a word. The test runs each line through the full
`:` path and asserts a refusal naming `"junk"`, both cells intact, nothing
yanked and no drill opened. Rival: `only_when_omitted` made unconditionally
`Ok` fails it at the first line.
### command::cmd::text_buffer::tests::colon_lets_a_later_segment_use_what_an_earlier_one_created
The review's second blocker: a `:` line is built before any segment runs, so the
name checks in `execute` refused `create-view Second . switch-view Second` and
`add-item Type Tools . set-page Type Tools` after the first half landed. The
checks now run when the effect applies. Rival: the checks back in `execute`
fails both lines with `No view named "Second"` and `Unknown item 'Tools'`.
### a_strict_import_that_skips_a_record_refuses_and_writes_nothing
improvise-8jk6.6, the user's ruling on PR #321. Written before `--strict`
existed, it failed on clap's unknown-argument error. It asserts the refusal
names the flag and the same count as the default warning, and that the output
path does not exist.
- **S1**, the flag parsed but ignored, fails it on the exit status.
- **S3**, refusing only after `persistence::save`, fails it on the output
path. That is the "a strict refusal still writes" contract this PR decided
against.
### a_strict_import_that_skips_nothing_still_writes
The control for the entry above. Without it, a `--strict` that refuses
**every** import passes that test. **S2** is that rival, and it fails here.
It failed before the flag existed too, but only on clap's error, which is not
the discriminating failure. The discriminating one is S2.
### a_csv_decimal_measure_imports_as_its_number
improvise-8jk6.17 (F3), from the adversarial review of #321. The fixture has
decimal CSV measures (`1000.50`, `-2.25`). With `number_in`'s `f64` branch
disabled (integers only), no headless test failed at `efaa5050`, and a decimal
measure was silently dropped. **F3** now fails this test and the next one.
This is coverage, not a bug: it passed when written.
### import_json_on_a_csv_reads_its_numeric_column_as_a_measure
improvise-8jk6.17 (F1). `improvise cmd 'import-json f.csv'` reaches
`ImportJsonHeadless`, a third CSV entry point besides `import`. **F1**
restores its CSV branch to the JSON constructor via
`ImportPipeline::new(Value::Array(recs.to_vec()))`. That still compiles,
because `CsvRecords` derefs to a slice. `Revenue` then becomes a category of
text items, and this test fails on its Data lines. At `efaa5050` only
`import_headless_refuses_records_that_collide` failed under F1, and only by
accident. The `## Category: Revenue` assertion names the symptom directly.
This is coverage: it passed when written.
+181 -1
View File
@@ -882,6 +882,11 @@ The control: a **lone** `\r` is itself a record terminator to this reader, so
it is not part of a CRLF pair, and it is why "two records, clean values" is a
real observation rather than a coincidence.
Since improvise-8jk6.4 the parser yields text, so the assertion is that
`Revenue` is the text `1`. A kept `\r` would make it `1\r`, which fails that
assertion, and which `number_in` would not read as a number where a measure
wanted one.
### an_empty_file_is_no_records_and_no_warnings
An empty file is no records and no complaint. The wizard's own preconditions
@@ -919,12 +924,17 @@ The consequence downstream is deliberate and worth stating: a column carrying
Label rather than a Measure. That is the honest answer — the column is not
numeric — and it is visible in the wizard, where a fabricated zero was not.
Since improvise-8jk6.4 the numeric rule is `csv_parser::number_in`, applied
through `typed` only where a number is wanted, and the parser itself yields
text. So this test asserts that `typed` leaves each token as text. The
`unwrap_or` fault would be injected into `number_in` now.
### an_ordinary_float_is_still_a_number
The control for the entry above. Guarding non-finiteness by deleting the float
branch entirely would also pass that test, so this one pins that `1000.50` is
still a `Number` and that `0` — the value the bug invented — is still reachable
when the file really says so.
when the file really says so. Since improvise-8jk6.4 both go through `typed`.
### a_repeated_header_is_renamed_rather_than_overwriting_its_twin
@@ -1675,6 +1685,44 @@ closed, the one its `collapsed:` line names and the three it does not, and the
line is written back. Rival: the old open default; this test fails with the
other eleven in that rival's list.
### a_date_column_imports_as_one_category_of_months_in_quarters_in_years
improvise-8jk6.3, the user's "dates should usually be one category". The
fixture has two days of one month, so that month is one summed cell, and
January of the next year, so a month must stay unique across years. Rivals, each
run against the `date_hierarchy` module, and each caught here:
- The split as the default shape.
- Levels taken finest first, so the year hangs under the month.
- Groups registered without their parents.
- The raw dates still added beside the months.
- A hierarchy counted as `1 + components` slots.
### a_level_left_unchosen_is_skipped_not_emptied
Year + Month nests months straight into years, and Quarter alone is a flat
category of quarters. Caught the finest-first rival and the raw-dates rival.
### an_imported_date_hierarchy_opens_folded_to_its_years
The import sets nothing: the hierarchy opens folded, one row per year, because
every group starts closed (improvise-felo.6). The 2024 row is the fold of its
months, 111. Caught three of the rivals above.
### an_unparsed_date_keeps_its_value_and_is_warned_about
A format that reads none of the values keeps each raw value as an ungrouped
item, and the build says so. Its first version passed on the base commit: the
shape was ignored there and the SPLIT's warning happened to contain the same
words. It now also asserts there is no `Date_Year`. Rival: the unparsed value
not counted; this test fails.
### a_headless_date_import_splits_when_told_to
`--split-date` is the wizard's `s`, headless. It failed first as an unknown
argument. Rival: the flag parsed and ignored; this test fails, and
`a_headless_date_import_is_one_hierarchical_category` does not, which is the
point of having both.
### every_fold_the_api_can_build_reads_back_from_the_file_it_writes
ChatGPT Codex, P2 on #317 at `e09ec752`: `GroupFold::Expanded(usize)` and
@@ -1818,6 +1866,68 @@ document still has the three views in order, `File: filter` at the file's item,
and the same drawn values. This is the first persisted, non-virtual use of
`Axis::Filter` (gotcha 27). R7 and R9 fail it.
### a_csv_column_read_as_a_category_keeps_its_text_and_as_a_measure_its_number
improvise-8jk6.4, at the pipeline. It checks three things in one fixture,
one per reader of a CSV value:
- analysis still proposes the numeric-looking `commit` as a Measure (typed);
- forced to Category, its items are `00565796` and `220944e2` (text);
- `lines` is stored as the number 5 (typed).
The rivals, each run:
- **R1**, a category named from the typed value, fails it on the items.
- **R2**, a CSV measure read with `as_f64` on its text, fails it on the cell.
- **R3**, CSV analysed without typing, fails it on the proposal.
R2 and R3 also fail the five lpzu merge tests in this module and five
headless imports. That is how much of the suite leans on CSV measures reading
as numbers.
This test uses `from_csv` and so cannot compile on the base. The base failure
for 8jk6.4 is the headless test
`a_csv_column_forced_to_category_keeps_its_text_verbatim`.
### a_formula_that_does_not_parse_stops_the_build_and_is_named
improvise-8jk6.5. On the base the build returned `Ok`, keeping `Good` and
silently dropping `Bad`. Two formulas fail to parse here, so the test proves
every failure is named. **R5**, naming only the first, fails it on `Worse`,
and since improvise-8jk6.17 the headless
`an_import_formula_that_does_not_parse_refuses_names_it_and_writes_nothing`
sees R5 too (it passes two bad formulas). It also asserts `Good` is not named, so a
message that lists every formula cannot pass. **R4** (drop, as the base did)
fails it.
### confirming_a_formula_that_does_not_parse_keeps_it_out_and_the_text_in
improvise-8jk6.5, the wizard half. On the base `confirm_formula` pushed
`Bad = revenue +` into the list, which failed on `pipeline.formulas`. The test
asserts the text is still in the buffer and the editor still open. A fix that
refused but cleared the buffer would throw away what the user typed, the same
hazard improvise-v4hg fixed in the TUI formula minibuffer. **R6**, confirming
regardless, fails it. The headless suite cannot see R6.
### a_build_that_keeps_no_record_refuses_and_says_why
improvise-8jk6.6. On the base both halves got `At least one category must be
accepted`, a refusal right only by accident. It asserts the exact message for
the two cases `nothing_to_import` distinguishes: all skipped, and no records
at all.
- **R7**, the check removed from the build, fails it.
- **R9**, `any` swapped for `all`, also fails it, on the empty source: `all`
is vacuously true over no records.
### the_review_step_says_when_no_record_can_be_imported
improvise-8jk6.6, the wizard half (gotcha 22: the check lives on the step
where the user can still choose another array path). On the base the step
refused with `No categories: … Press c to cycle a field's kind`, which offers
a key that has no field to act on. **R8**, the step without the check, fails
it. The build test does not see R8.
### a_header_only_file_keeps_its_file_item_its_columns_and_its_view
The ChatGPT Codex P2 on PR #314 at `4e68372c`, confirmed by the adversarial
@@ -1866,3 +1976,73 @@ improvise-8jk6.12 (4). A `region` of `" "` is kept as a record with no
predates this PR (improvise-ych), but the sparse-record rule now rests on it:
without it the value becomes an item named `" "`. Rival F4 (the reviewer's
fault) fails it.
### a_raw_date_spelled_like_a_generated_item_is_refused
improvise-d0n6 (Codex P2 on PR #322). A `%Y-%m-%d` column imported Year > Month
with `2024-01-15` and a raw `2024-01`. Before the refusal the build succeeded,
put both records on the grouped item `2024-01` (summing them) and warned that
the raw value was "kept ungrouped", which was false. Fault injected: removing
the `spelling_clash` call makes this test fail with `unwrap_err()` on an `Ok`.
### a_raw_date_spelled_like_a_generated_group_is_refused
improvise-d0n6. The same with a raw `2024`, a generated group. Removing the
`spelling_clash` call fails it the same way. The control,
`a_raw_date_spelled_like_nothing_generated_is_kept_ungrouped`, passes either
way, so a blanket refusal of every unparsed value fails it.
### the_date_step_will_not_advance_over_a_spelling_clash
improvise-d0n6 (Copilot on PR #322, and the adversarial review's B1). The
spelling-clash refusal sat only in the build, so a wizard with a raw `2024` in
a `%Y-%m-%d` column passed ConfigureDates and failed on NameModel, three steps
from the `s` that fixes it. Before the check joined `validation_error` the
test failed with the step already `DefineFormulas`; removing that call again
reproduces it. It also toggles to the split shape and requires the step to
advance, so a check that blocks forever fails.
### split_date_naming_an_unknown_or_non_date_field_is_refused
improvise-tpm8 class. `--split-date Nope` and `--split-date Revenue` (a
measure) exited 0 and did nothing. Both now exit 1 naming the flag and the
field; removing the check in `run_headless_import` fails both rows.
### an_axis_naming_a_category_the_import_did_not_build_is_refused
improvise-8jk6.3 review. `--extract Date:Month --axis Date_Month:column` used
to build one `Date` category and drop the `--axis` for the vanished
`Date_Month`, exit 0. It now exits 1 naming the category and `--split-date`.
This also refuses any `--axis` naming an absent category, which was silently
ignored before this PR as well.
### a_folder_of_only_header_only_files_keeps_no_record_and_refuses
improvise-8jk6.6 meeting PR #314's rule that a header-only file is still a
file. Written after merging `main` at `c91cc055`, where the two rules first
met. The file seeds give the pipeline a `File` Category proposal even with no
records. So without `nothing_to_import` the build succeeds and saves `File`
items and empty views: the category check alone cannot refuse. The `setup`
assertion proves that seeded proposal exists, so the test is not passing only
because there was no category. **R7**, the check removed from the build, fails
it. The user confirmed the refusal on PR #321: "Refuse (as #321 does)".
### an_absent_axis_category_that_is_no_date_part_names_what_was_built
improvise-8jk6.3 review (B2). The README's own recipe passed
`--axis Measure:column`, a silent no-op because the category is `_Measure`; the
first refusal blamed dates for it. The test uses a non-date absent name and
asserts the message lists the built categories and carries no date hint.
Fault injected: making the hint unconditional fails it.
### the_readmes_import_recipe_runs
improvise-8jk6.3 review (B2). Runs the README's recipe as printed. Fault
injected: restoring `--axis Measure:column` in the README fails it with the
refusal.
### split_date_on_a_time_field_with_no_date_format_is_refused
improvise-8jk6.3 review. `--time Revenue --split-date Revenue` made the field a
time category with no date format and the split was a silent no-op. Without
the `date_format.is_some()` condition the import exits 0 and this fails.
+125
View File
@@ -0,0 +1,125 @@
# Layout and keypress cost (improvise-felo.13)
Measured on an **Apple M4 Max, 64 GB**, macOS, release build, with
`benches/scale_bisect.rs` (merged from `bench/scale-bisect`, `13918bb9`), one
configuration per process, 3 categories of K items on Row, N = 100,000 capped at
K^3/2 records, prune off unless stated. Baseline is this branch with only the
tests added; "after" is the branch head. `key` is `App::dispatch_key(Down)`;
"cold" is the first key after open or after an edit, which pays the one
whole-grid measurement; "warm" is every key after it.
## Before and after
| rows | before key | after key warm | after key cold | before browser window | after browser window | SetCell before / after |
|---|---|---|---|---|---|---|
| 729 (K=9) | 5.5 | 0.00 | 1.4 | 5.0 | 0.13 | 0.9 / 0.9 |
| 4,913 (K=17) | 108 | 0.00 | 6.0 | 105 | 0.12 | 5.8 / 5.9 |
| 5,832 (K=18) | | 0.00 | 7.7 | | 0.12 | 6.8 |
| 6,859 (K=19) | | 0.00 | 8.8 | | 0.12 | 8.6 |
| 19,683 (K=27) | 1,271 | 0.00 | 24.5 | 1,259 | 0.14 | 23.0 / 22.7 |
| 64,000 (K=40) | 10,771 | 0.00 | 84 | 10,746 | 0.20 | 79 / 80 |
Prune on: K=17 (2,456 rows) key 264 ms -> 0.00 warm, 3.6 cold; prune toggle
172 -> 8.8 ms; K=27 prune toggle 2,189 -> 41.5 ms. Records mode (3 x 1000, 14,500
records): key 1,004 ms -> 0.00 warm, 2.8 cold; **SetCell 1,511 ms -> 1,512 ms,
unchanged** (not this ticket's cost).
Boundaries, warm key and browser window: inside 8 ms at every size measured (to
216,000 rows, 303 ms cold). Cold key: inside 8 ms to K=18 (5,832 rows), over 8 ms at
K=19. Before: inside 8 ms to 729 rows, over 100 ms at 4,913, over 1 s at 19,683.
The remaining cold cost is linear (1.2-1.4 us per row) and is the whole-grid width
and total pass; the warm path does not repeat it.
## What the ticket's reading got right and what it did not predict
The O(rows^2) in `cell_key` was real and is one of three causes. Two more sat
under it. First, `compute_col_widths` ran on **every** keypress, through
`cmd_context`, and `dispatch_key` and `apply_effects` each rebuilt the layout
whether or not anything had moved. Second, once the index was O(1) the width pass
was still O(K^2) per cell: with `_Index`/`_Dim` on `Axis::None` the aggregation is
not `Exact`, so each cell went through `DataStore::matching_values`, which
narrows by the smallest index set (about N/K candidates) and probes the rest. A
partial as wide as the widest stored cell can match only one cell, so
`exact_positions` probes it directly. That second cause is why K=40's cold key
stayed at 8 s after the first two fixes.
### a_cursor_key_rebuilds_neither_the_layout_nor_the_widths
`ui::app::tests`. Two keys after a warm-up leave `layout_builds` and
`width_passes` where they were. Failed before the fix on the layout count (a
cursor key rebuilt it). Rival 1 (`refresh_layout` always rebuilding): fails on
`layout rebuilt on a cursor key`. The test seats the cursor at row 2, so "the
keys moved the cursor" is asserted and a key that does nothing cannot pass.
### an_edit_rebuilds_and_remeasures_so_the_widths_follow_the_value
The control for the test above, and the guard on invalidation. A `SetCell` of a
long text must rebuild the layout and remeasure, and the cached widths must equal
a fresh `compute_col_widths`. Rival 2 (`rebuild_layout` not dropping the cached
geometry): fails on `an edit must remeasure`. It also asserts the widths changed,
so a fixture where the edit does not widen a column cannot pass.
### switching_view_rebuilds_though_the_effect_is_view_only
`SwitchView` is `ViewOnly` (gotcha 3), so skipping the rebuild by `EffectKind`
alone would leave the old view's layout. The layout records the view it drew and
the drill snapshot it froze, and rebuilds when either differs.
### an_axis_lookup_does_work_independent_of_the_row_index
`view::layout::lookup_cost_tests`. Counts the items `data_items` yields during
`cell_key`, `row_tuple`, `row_label` and `column_key`; row 399 of 400 must cost no
more than row 0 plus two. Failed before the fix: `cell_key: row 399 cost 402 steps
against 3 at row 0`. Rival (`cell_key` going back to `data_items(..).nth(row)`):
`cost 400 steps against 1 at row 0`. Limit: the counter lives in `data_items`, so
a different linear walk written without it is not seen; the next test is what
holds the index to the walk's answers.
### the_position_index_answers_what_the_walk_answered
Compares `row_tuple`, `data_row_to_visual` and `row_folds_a_group` against the
walk over a grouped, collapsed and pruned axis, including two indices past the
end. Rival (`data_index_to_visual` reading `data_idx + 1`): fails at `visual 0`.
### a_full_width_partial_probes_one_cell_instead_of_narrowing
`model::cell::exact_path_tests`. A full-width partial must not reach set
narrowing, and an absent cell and an unseen item must match nothing. Rival
(`exact_positions` returning `None`): the narrowing count is 1, not 0. Rival
(`widest <= partial.len() + 1`): fails `a_wider_cell_turns_the_shortcut_off_until_it_is_removed`,
which stores a cell one coordinate wider and expects it matched.
`a_narrower_partial_still_narrows_and_matches_every_superset` is the control: the
shortcut is not a blanket exact lookup. `an_unsorted_full_width_partial_is_still_answered`
holds the canonical-order condition: a partial not in a stored key's order falls
back.
### a_window_over_a_held_summary_equals_the_one_walk_grid_body
`ui::viewport::tests`. The host keeps one `GridSummary` (widths, total formats,
totals) per layout and reads a window over it with `grid_window`; it must equal
what `grid_body` builds for the same window, over the whole grid, an interior
window, the last row and the empty window. Rival (`grid_window` reading column
`ci + 1`): fails at `window 0..2 x 0..10`. Widths stay the **whole-grid** maximum,
because the summary is the same walk `grid_body` did; nothing the user sees moves.
### a_frozen_records_snapshot_replaces_the_live_row_index
`view::layout::lookup_cost_tests`. Three records are frozen, two are then cleared,
and `with_frozen_records` over the snapshot must draw 3 rows (the control: with no
snapshot it draws 1). A drill held open while another client removes records is the
production analogue. Rival (deleting `layout.reindex()` after the snapshot swap):
fails `left: 1, right: 3`; the whole workspace passed it before this test.
### a_drill_snapshot_replaced_alone_rebuilds_the_layout
`ui::app::tests`. `StartDrill` is `ViewOnly`, so a batch of it alone has no `Model`
effect to mark the layout stale; the drill pointer in `LayoutBasis` is the only
thing that rebuilds it. The test starts one snapshot, then a second: the first
step alone passes even with a constant pointer (drill absent to present changes the
basis), which is why the second step is the test. Rival (pointer replaced by a
constant): fails `no rebuild for a second drill snapshot`.
### Observation: the TUI frame
`tui_frame_grid_of_whole` is still 47-53 ms at 19,683 rows after this change: the
draw path projects the whole grid, linear and out of scope for this ticket.
+218
View File
@@ -0,0 +1,218 @@
# Scale bisect: how big a document does improvise handle today (improvise-felo.10)
Measured on `gh/main` @ 8220a065, **Apple M4 Max, 64 GB** (`sysctl`: `hw.memsize 68719476736`), macOS,
release build (`lto`, `opt-level 3`), native only (no wasm, no real OPFS, no terminal).
Harness: `benches/scale_bisect.rs` (one configuration per process) driven by `bench/scale_bisect.py`,
`bench/sweep.py`, `bench/edge.py`, `bench/full.py`. Reproduce:
`nix develop --command cargo build --release --bench scale_bisect`, copy the binary from
`target/release/deps/scale_bisect-*` to `/tmp/sb`, then `python3 bench/edge.py`.
Budgets (user ruling 2026-10-03): **8 ms is the headline** (120 FPS, hot path or defer to background);
16 ms, 100 ms (noticeable) and 1 s (broken) are reference crossings.
## Method, and what is real
Every number goes through the production path: `App::new` (open: formula recompute +
`GridLayout::with_frozen_records`), `App::dispatch_key(Down)` (**keypress/scroll**: it calls
`rebuild_layout`, then `cmd_context`, which runs `compute_col_widths` over the whole grid, then applies effects),
`project::grid_of` (TUI frame: whole grid; browser: a 45x12 window), `App::apply_effects` with `effect::SetCell`,
`SetAxis`/`HideItem`/`ToggleGroup`/`TogglePruneEmpty` (reshape), `Host::connect` and `Host::handle` with a
`caught_up` `ModelCommand::SetCell` (browser path incl. projection), `format_md`/`parse_md`/`save_to(MemoryFs)`.
Document: C0..C{cats-1} with K items, N distinct random tuples ("records", capped at K^cats/2), two measures
each (2N cells), `_Measure` on Column. Timings are the **median** of up to 3-8 in-process repeats; boundary
rows are the median of **3 separate processes** run sequentially (spread shown). Earlier exploratory sweeps
ran 2-3 processes in parallel and agree with the sequential runs within ~5%.
**`View::prune_empty` defaults to false** (`view/types.rs`), so "prune off" is what a user gets until they
toggle it; "prune on" is reported next to it. Rows below are the layout's `row_count`.
**Dominant cost (reasoned, not profiled):** the release profile strips symbols, so `sample` could not
symbolicate. The reason is in the code and the timings agree with it: `GridLayout::cell_key`/`row_item` do
`data_items(&self.row_items).nth(row)` (`view/layout.rs` ~253/292/807/870), O(row) per lookup, and
`compute_col_widths` (every keypress, via `cmd_context`) and `grid_of` visit every cell, so both are
O(rows^2). Evidence: with prune off, rows 729 -> 5.4 ms, 4096 -> 74.6 ms, 19683 -> 1248 ms (x6.7 rows ->
x14 time -> x17 time). A 45x12 browser window costs **the same** as the whole-grid TUI frame (K=30:
2274 vs 2298 ms) because the widths pass is whole-grid whatever the viewport says, so viewport narrowing buys
nothing today. `GridLayout` build itself is cheap (~90 ns/row, ADR 008).
## Attribution: state+projection vs render/serialize (K=10, 1000 rows, prune off; 3 runs)
| step | ms |
|---|---|
| keypress (state: rebuild + widths + effect) | 9.5-9.7 |
| `grid_of` whole grid (TUI projection) | 11.7-12.0 |
| `GridWidget` render into a 200x50 ratatui `Buffer` | 0.12 |
| `grid_of` 45x12 window (browser projection) | 9.1-9.3 |
| JSON encode of one SetCell's projected updates | 0.001-0.005 (0.6-1.1 KB) |
| JSON encode of the connect Snapshot | 0.02 (21 KB) |
Render and serialize are **<=0.5 ms in every row measured** (GridWidget 0.08-0.53 ms up to 14,500 records;
snapshot encode <=0.07 ms; records-mode SetCell updates 21 KB in 0.02 ms). The 8 ms is entirely state and
projection. Not measured: the rest of `draw()` (panels, status; `draw` is private to the binary), terminal I/O,
browser JSON *decode* and DOM paint.
## Axis B: items per category K, 3 categories on Row, N = 100k (capped at K^3/2), prune OFF
| K | rows | open | keypress | browser window | SetCell | maxrss MB |
|---|---|---|---|---|---|---|
| 5 | 125 | 0.1 | 0.5 | 0.5 | 0.2 | 8 |
| 9 | 729 | 0.2 | **5.4** (5.3-5.5) | 5.0 | 0.9 | 13 |
| 10 | 1,000 | 0.2 | **9.6** (9.5-9.7) | 9.2 | 1.2 | 14 |
| 11 | 1,331 | 0.3 | 12.7 | 12.7 | 1.6 | 10 |
| 12 | 1,728 | 0.3 | 20.0 | 19.3 | 2.1 | 11 |
| 15 | 3,375 | 0.5 | 65 | 64 | 4.0 | 14 |
| 16 | 4,096 | 0.6 | 74.6 (74.3-75.2) | 73.4 | - | - |
| 17 | 4,913 | 0.7 | **105** (105.0-105.5) | 103 | - | - |
| 20 | 8,000 | 1.1 | 302 | 301 | 9.4 | 21 |
| 25 | 15,625 | 1.9 | 773 | 771 | 18.8 | 33 |
| 26 | 17,576 | 2.2 | 967-977 | 962 | - | - |
| 27 | 19,683 | 2.4 | **1,248** (1,244-1,251) | 1,244 | - | - |
| 30 | 27,000 | 3.1 | 2,262 | 2,274 | 31 | 49 |
| 40 | 64,000 | 6.7 | 10,858 | 10,833 | 73 | 235 |
| 60 | 216,000 | 21.6 | timeout (>60 s) | - | - | - |
| 1000 | 10^9 | **refused** (see below) | | | | |
Hot-path keypress/scroll and browser window: **largest inside 8 ms: K=9 (729 rows)**; inside 16 ms: K=11;
first over 100 ms: **K=17 (4,913 rows)**; first over 1 s: **K=27 (19,683 rows)**. SetCell alone
(no widths pass; layout rebuild only, ~linear in rows): inside 8 ms to K~19 (K=15 4.0, K=20 9.4), 16 ms ~K=24,
100 ms ~K=45 (K=40 = 73 ms), 1 s extrapolated ~K=95 (**extrapolated, not measured**). Open stays <25 ms to K=60.
**3 x 1,000 on Row cannot be opened at any N**: the layout materializes the cross product (10^9 rows, ~150 B
each = ~150 GB, ADR 008). The harness refuses above 30M rows (`rows_cap`) rather than swap the machine, so
the 10^9 number is the ADR's estimate, not a measurement here.
### Prune ON (same documents)
| K | rows | open | keypress | browser window | SetCell |
|---|---|---|---|---|---|
| 7 | 171 | 2.0 | 4.5 (4.4-4.6) | 1.1 | 5.4 |
| 8 | 256 | 3.0 | **7.0** (7.0-7.1) | 1.6 | 8.5 |
| 9 | 364 | 5.3 | 12.8 | 2.7 | 15.3 |
| 11 | 665 | 12.2 | 29.6 | 6.2 | ~28 |
| 13 | 1,098 | 29.0 | 71 | 14.5 | ~70 |
| 14 | 1,372 | 48 | **119** | 24 | - |
| 20 | 4,000 | 300 | 747 | 145 | 903 |
| 21 | 4,630 | 401 | 968-992 | 190 | - |
| 25 | 7,812 | 767 | 1,899 | 366 | 2,328 |
| 30 | 13,500 | 2,255 | 5,555 | 1,047 | 6,804 |
Prune on: keypress inside 8 ms to **K=8**, first over 100 ms **K=14**, ~1 s at **K~21-22** (K=22 measured
0.93-0.95 s, a different N cap; first clearly over: K=25). Pruning shrinks the grid, so the browser window is
cheaper than prune off at the same K (K=20: 145 ms against 302 ms), but **every keypress re-runs
the prune** (`dispatch_key` calls `rebuild_layout`), so keypress and SetCell are *worse* with prune on
(K=10: 23 vs 9.9 ms). Prune is also quadratic (`nth`). Conclusion: prune on/off moves the boundary by about
one K at 8 ms and makes 100 ms/1 s *earlier* (K=14/21 vs 17/27); it never rescues the 3x1000 case.
## Axis A: records N at fixed K (K=30, 27,000 rows)
| N | prune off keypress | prune on rows | prune on keypress | prune on window |
|---|---|---|---|---|
| 100 | 451 | 100 | 861 | 0.3 |
| 1,000 | 527 | 1,000 | 1,059 | 6.4 |
| 5,000 | 1,013 | 5,000 | 2,178 | 140 |
| 13,500 (cap) | 2,267 | 13,500 | 5,555 | 1,047 |
Prune off: ~450 ms of widths over 27k empty rows + ~1.3 us per stored cell; **N never gets the keypress
under 100 ms at K=30**, and at K=1000 N is irrelevant because the layout cannot be built (rows = K^3).
The N-axis has no bisect point in the 3x1000 regime; N only matters once K is small enough that rows fit, and
there it is a second-order term. Open is flat (3.1-3.6 ms) in N with prune off.
## Axis C: categories on Row (prune off)
| K | cats | rows | open | keypress | note |
|---|---|---|---|---|---|
| 10 | 2 | 100 | 0.1 | 0.2 | |
| 10 | 3 | 1,000 | 0.2 | 9.9 | over 8 ms |
| 10 | 4 | 10,000 | 1.4 | **880** | ~1 s |
| 10 | 5 | 100,000 | 14 | timeout >100 s | |
| 3 | 6 | 729 | 0.3 | 27.9 | 5x worse than K=9 x 3 cats at the same rows |
| 3 | 8 | 6,561 | 2.0 | 2,798 | |
| 3 | 10-12 | 59k-531k | 18-210 | timeout | open alone passes 100 ms at 11 cats |
| 2 | 10 | 1,024 | 0.6 | 172 | |
| 2 | 12 | 4,096 | 2.2 | 3,190 | |
| 2 | 14 | 16,384 | 9 | 63,100 | |
The cap of 24 is unreachable: rows = product of items, so a view with c Row categories of K items dies by
K^c ~ 10^4 rows (1 s) regardless of N, and with two items per category by 12-14 categories. Per-row cost
also grows with category count (each row_item is a Vec of c items). Not measured: Row categories with prune
on, other axes absorbing categories (the realistic mitigation: only 1-2 on Row).
## Axis D: records mode over N records (`with_frozen_records`, the improvise-sk32 quadratic)
| N | open | keypress | browser window | SetCell (apply) | host SetCell+project | maxrss MB |
|---|---|---|---|---|---|---|
| 300 | 1.5 | 0.7 | 0.1 | 1.6 | - | 10 |
| 1,000 | 2.9 | 5.6 (5.5-5.7) | 0.3 | 10 | - | 12 |
| 1,200 | 3.9 | **7.9** (7.7-8.0) | 0.3 | - | - | - |
| 1,300 | 4.5 | 9.3 | 0.3 | 15.5 | 17 | 26 |
| 1,800 | 8.4 | 16.5 | 0.4 | - | - | - |
| 3,000 | 22.9 | 47 | 0.7 | 74 | - | 20 |
| 4,400-4,600 | 47-51 | **95-102** | 0.9 | 149 | 158 | 55 |
| 10,000 | 248 | 492 | 1.8 | 734 | - | 46 |
| 14,000-14,500 | 472-500 | **940-1,002** | 2.6-2.8 | 1,435 | 1,459 | 135-148 |
| 30,000 | 2,198 | 4,314 | 5.6 | 6,467 | - | 125 |
| 100,000 | **25,420** | **51,234** | 19.3 | not reached (killed at 100 s) | - | not reached |
Records mode, keypress/scroll: inside 8 ms **to N~1,200**; 16 ms N~1,800; 100 ms N~4,500 (first over: 4,600 =
102 ms); 1 s N~14,500. Open crosses 8 ms ~1,750, 16 ms ~2,300, 100 ms ~6,500, 1 s ~20,000. Time ~ N^1.95.
SetCell is ~1.5x keypress (it rebuilds and re-groups). The *browser window* is cheap (<20 ms even at 100k)
because records mode has two columns and the width walk is short, so the 51 s is `rebuild_layout`, not
projection. Prune on/off does not change records mode (prune applies to pivoted axes; the sweep ran off).
Records per KB: ~4 KB/record process-wide at 14k (maxrss includes the document generator, so read as an
upper bound); the 100k memory was not obtained.
## Axis E: save / load and per-command persistence
Mode `none` (all categories off-axis, so layout is trivial and only the store scales), 3 x 1,000, N records
= 2N cells:
| cells | format_md | text MB | parse_md | persist per accepted command (`save_to` MemoryFs) | maxrss MB |
|---|---|---|---|---|---|
| 2,000 | 13.8 | 0.1 | 4.5 | 12.4 | 18 |
| 20,000 | 156 | 0.86 | 37 | 161 | 80 |
| 60,000 | 527 | 2.5 | 119 | 539 | 236 |
| **200,000 (the target)** | **1,974** | 8.4 | **401** | **1,984** | 663 |
| 600,000 | 6,470 | 25 | 1,320 | 6,460 | 1,894 |
| 2,000,000 | 23,120 | 84 | 4,668 | 23,088 | 5,611 |
Linear: ~10 us/cell to format, ~2 us/cell to parse. Crossings, format_md: 8 ms ~700 cells, 16 ms ~1.5k,
100 ms ~11k, 1 s ~100k cells. parse_md: 8 ms ~4k cells, 100 ms ~50k, 1 s ~450k. **At the target (200k cells)
every accepted command in the browser worker costs ~2 s of format alone** before OPFS write time (not
measured: MemoryFs only). Persist per command also appears in smaller documents inside the grid axes
(K=10 grid: 5.5 ms; records N=4,400: 63 ms).
## Other operations (3 runs each; "grouped" fixtures start with C0 groups collapsed, so rows are not K^3)
Reshape (prune off, K=10, 1000 rows): hide item 0.25 ms, set axis 0.19 ms, **toggle prune 12.7 ms**.
K=26 grouped, 2,028 rows: hide 0.5, toggle group 1.8, set axis 1.8, **toggle prune 727-750 ms**.
Prune on K=13 grouped, 321 rows: hide 9.4, toggle group 37, set axis 37, toggle prune 37-39 ms.
`Host::connect` snapshot: K=10 6.5 ms, K=26 375 ms, records N=14,000 2.9 ms. Host SetCell+project: K=10 7.7 ms,
K=26 ~400 ms, records N=4,400 158 ms (equal to apply + a whole-grid width walk).
## Classification and feasibility of yielding
| operation | class | today's first size over 8 ms | dominated by | chunked/yielding form |
|---|---|---|---|---|
| keypress / scroll | **hot (<=8 ms)** | 1,000 rows (K=10) / N=1,300 records | `rebuild_layout` + whole-grid `compute_col_widths` (O(rows^2) `nth`), recomputed though nothing changed | not a slicing problem: cache widths/layout across keys, O(1) row lookup (ADR 008/009). Slicing would only hide it |
| browser window projection | **hot** | 1,000 rows | same width walk, whole grid regardless of window | needs the same fix; then window cost is O(window) |
| SetCell apply + project | **hot** | K~19 (prune off), 1,300 records | layout rebuild O(rows) + widths + projection | incremental update of one row/total, not a rebuild |
| column totals | hot | inside `grid_of` (not separable here) | whole-grid sum in the projection | maintained total (ADR 009) |
| open / layout build | deferrable | 40k rows (prune off); N~1,750 (records) | build is ~90 ns/row (sliceable by outer index); the prune/records grouping is the quadratic part | build is feasible in slices once the quadratic is removed; the quadratic itself is not sliceable |
| reshape (hide/collapse/axis) | deferrable | cheap prune-off (<2 ms to ~2k rows); 37 ms prune-on at 321 rows | layout rebuild + prune | one sort/remap at 100k occupied tuples was 2.3 ms in ADR 008's prototype, so a reshape can fit inline; prune toggle needs the quadratic fixed first |
| save (format_md) | deferrable | ~700 cells | per-cell formatting, linear | the Data section is a flat per-cell line stream, so chunking by cell range with a yield every ~400 cells looks feasible, but needs a consistent snapshot (clone or journal); not profiled by function |
| load (parse_md) | deferrable | ~4k cells | pest parse + tree walk, linear | tree walk is chunkable by line; the pest parse of one text is not, unless the Data section is parsed line by line (grammar is line oriented); not profiled by function |
| persistence per command | deferrable (debounce) | ~700 cells | the whole-document `format_md` | coalesce/debounce or append-journal: formatting 2 s per command at target is unusable inline at any slicing |
## What I could not measure
* wasm: all numbers are native; the browser worker is typically slower. Real OPFS I/O, JSON *decode* and
DOM paint, the full `draw()` frame, and Safari are not covered.
* Records mode at 100k: SetCell, host and RSS (killed after open and keypress, ~76 s).
* Per-function profile: symbols are stripped in `[profile.release]`; attribution is reasoned from the
code and the timing exponents, not a flame graph.
* SetCell 100 ms/1 s for prune off past K=40 are extrapolations.
* Memory (`maxrss`) includes the generator's hash set and `recs` vector; it is an upper bound on the
document. Layout cost ~150 B/row per ADR 008, consistent with K=40 (64k rows, 235 MB peak with prune off
including the generator and a second layout during rebuild).
* Noise: boundary rows are 3 sequential processes, spread <=3% (e.g. K=17: 105.0-105.5 ms). Non-boundary
rows are single runs, some taken with 2-3 concurrent processes on a 16+ core machine; treat them as +-5-10%.
@@ -250,12 +250,37 @@ pub trait AtCursor: Cmd {
fn cursor_mut(&mut self) -> &mut CursorState;
}
/// How many arguments a command takes. The registry refuses any beyond it, for
/// every command in one place — `context/code/cli.md` §14.
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum MaxArgs {
AtMost(usize),
Any,
}
impl MaxArgs {
fn admit(self, args: &[String]) -> Result<(), String> {
match self {
MaxArgs::AtMost(n) if args.len() > n => {
let extra: Vec<String> = args[n..].iter().map(|a| format!("{a:?}")).collect();
Err(format!(
"takes at most {n} argument(s), got {}; unexpected {}",
args.len(),
extra.join(" ")
))
}
MaxArgs::AtMost(_) | MaxArgs::Any => Ok(()),
}
}
}
type BoxParseFn = Box<dyn Fn(&[String]) -> Result<Box<dyn Cmd>, String>>;
type BoxInteractiveFn = Box<dyn Fn(&[String], &CmdContext) -> Result<Box<dyn Cmd>, String>>;
/// A registered command entry with both text and interactive constructors.
struct CmdEntry {
name: &'static str,
max: MaxArgs,
parse: BoxParseFn,
interactive: BoxInteractiveFn,
}
@@ -295,10 +320,11 @@ impl CmdRegistry {
pub fn register<C: Cmd + 'static>(
&mut self,
prototype: &C,
max: MaxArgs,
parse: fn(&[String]) -> Result<C, String>,
fallback: Option<FallbackFn<C>>,
) {
self.push(prototype.name(), parse, fallback, |_, _| {});
self.push(prototype.name(), max, parse, fallback, |_, _| {});
}
/// As [`register`](Self::register), then the interactive arm moves the
@@ -306,10 +332,11 @@ impl CmdRegistry {
pub fn register_at_cursor<C: AtCursor + 'static>(
&mut self,
prototype: &C,
max: MaxArgs,
parse: fn(&[String]) -> Result<C, String>,
fallback: Option<FallbackFn<C>>,
) {
self.push(prototype.name(), parse, fallback, |cmd, ctx| {
self.push(prototype.name(), max, parse, fallback, |cmd, ctx| {
*cmd.cursor_mut() = CursorState::from_ctx(ctx);
});
}
@@ -317,12 +344,14 @@ impl CmdRegistry {
fn push<C: Cmd + 'static>(
&mut self,
name: &'static str,
max: MaxArgs,
parse: fn(&[String]) -> Result<C, String>,
fallback: Option<FallbackFn<C>>,
at_cursor: fn(&mut C, &CmdContext),
) {
self.entries.push(CmdEntry {
name,
max,
parse: Box::new(move |args| parse(args).map(|c| Box::new(c) as Box<dyn Cmd>)),
interactive: Box::new(move |args, ctx| {
let mut built = match (parse(args), fallback) {
@@ -338,9 +367,10 @@ impl CmdRegistry {
/// Register a command that needs no interactive context: BOTH arms are
/// `parse`, so neither can demote the other — `context/code/command.md`.
pub fn register_pure(&mut self, prototype: &dyn Cmd, parse: ParseFn) {
pub fn register_pure(&mut self, prototype: &dyn Cmd, max: MaxArgs, parse: ParseFn) {
self.entries.push(CmdEntry {
name: prototype.name(),
max,
parse: Box::new(parse),
interactive: Box::new(move |args, _ctx| parse(args)),
});
@@ -352,6 +382,7 @@ impl CmdRegistry {
let name = f().name();
self.entries.push(CmdEntry {
name,
max: MaxArgs::AtMost(0),
parse: Box::new(move |_| Ok(f())),
interactive: Box::new(move |_, _| Ok(f())),
});
@@ -364,7 +395,9 @@ impl CmdRegistry {
let canonical = self.resolve(name);
for e in &self.entries {
if e.name == canonical {
return (e.parse)(args).map_err(|err| format!("{name}: {err}"));
return (e.parse)(args)
.and_then(|cmd| e.max.admit(args).map(|()| cmd))
.map_err(|err| format!("{name}: {err}"));
}
}
Err(format!("Unknown command: {name}"))
@@ -381,7 +414,7 @@ impl CmdRegistry {
let name = self.resolve(name);
for e in &self.entries {
if e.name == name {
return (e.interactive)(args, ctx);
return (e.interactive)(args, ctx).and_then(|cmd| e.max.admit(args).map(|()| cmd));
}
}
Err(format!("Unknown command: {name}"))
@@ -401,16 +434,31 @@ pub(super) fn require_args(args: &[String], n: usize) -> Result<(), String> {
}
}
/// Parse Cat/Item coordinate args into a CellKey.
pub(super) fn parse_cell_key_from_args(args: &[String]) -> crate::model::cell::CellKey {
let coords: Vec<(String, String)> = args
/// Parse Cat/Item coordinate args into a CellKey, refusing any token that is not
/// one rather than dropping it — `context/code/cli.md` §14.
pub(super) fn parse_cell_key_from_args(
args: &[String],
) -> Result<crate::model::cell::CellKey, String> {
let coords = args
.iter()
.filter_map(|s| {
let (cat, item) = s.split_once('/')?;
Some((cat.to_string(), item.to_string()))
.map(|s| match s.split_once('/') {
Some((cat, item)) if !cat.is_empty() && !item.is_empty() => {
Ok((cat.to_string(), item.to_string()))
}
_ => Err(format!("{s:?} is not a Category/Item coordinate")),
})
.collect();
crate::model::cell::CellKey::new(coords)
.collect::<Result<Vec<_>, _>>()?;
Ok(crate::model::cell::CellKey::new(coords))
}
/// A cursor fallback answers only for coordinates that were OMITTED; given and
/// wrong, they are refused, never the cursor's cell — `context/code/cli.md` §14.
pub(super) fn only_when_omitted(args: &[String]) -> Result<(), String> {
if args.is_empty() {
return Ok(());
}
parse_cell_key_from_args(args)?;
Err("takes Category/Item coordinates, or none to use the cursor".to_string())
}
/// Read the current value of a named buffer from context.
@@ -1,13 +1,13 @@
use crate::model::cell::CellValue;
use crate::ui::effect::{self, Effect, ViewTarget};
use crate::view::Axis;
use super::core::{Cmd, CmdContext, require_args};
use super::core::{Cmd, CmdContext, MaxArgs, parse_axis, parse_cell_key_from_args, require_args};
#[cfg(test)]
mod tests {
use super::*;
use crate::command::cmd::test_helpers::*;
use crate::view::Axis;
#[test]
fn add_category_cmd_produces_add_category_effect() {
@@ -59,7 +59,7 @@ mod tests {
let ctx = make_ctx(&m, &layout, &reg);
let cmd = SetAxisCmd(vec!["Type".to_string(), "col".to_string()]);
let effects = cmd.execute(&ctx);
assert_eq!(effects.len(), 1);
assert_eq!(effects.len(), 2, "the name check, then the write");
let mut app = crate::ui::app::App::new(two_cat_model(), None);
app.apply_effects(effects);
let view = app.model_state.workbook.active_view();
@@ -95,6 +95,104 @@ mod tests {
assert!(present.model_state.workbook.model.formulas().is_empty());
}
/// Run `name args` built by the pure arm against a fresh `App` on `wb`.
fn run_on(wb: &crate::workbook::Workbook, name: &str, args: &[&str]) -> crate::ui::app::App {
let args: Vec<String> = args.iter().map(|s| s.to_string()).collect();
let mut app = crate::ui::app::App::new(wb.clone(), None);
let cmd = crate::command::cmd::registry::default_registry()
.parse(name, &args)
.unwrap_or_else(|e| panic!("`{name} {args:?}` must build: {e}"));
let effects = {
let ctx = app.cmd_context(
crate::command::key::KeyCode::Null,
crate::command::key::KeyModifiers::NONE,
);
cmd.execute(&ctx)
};
app.apply_effects(effects);
app
}
/// A document where every sibling below has something real to name.
fn named_things() -> crate::workbook::Workbook {
let mut wb = two_cat_model();
wb.add_item_in_group("Type", "Tools", "Hardware").unwrap();
wb.create_view("Second");
wb
}
/// improvise-31oe.2 and its siblings: each names something missing, is
/// refused naming it, and changes nothing. Evidence: `context/test-evidence/cli.md`.
#[test]
fn a_command_naming_something_the_document_lacks_is_a_refusal() {
let wb = named_things();
let cases: [(&str, &[&str], &str); 10] = [
("set-axis", &["Regoin", "column"], "Regoin"),
("set-page", &["Regoin", "Food"], "Regoin"),
("set-page", &["Type", "Nowhere"], "Nowhere"),
("hide-item", &["Regoin", "Food"], "Regoin"),
("hide-item", &["Type", "Nowhere"], "Nowhere"),
("show-item", &["Regoin", "Food"], "Regoin"),
("show-item", &["Type", "Nowhere"], "Nowhere"),
("toggle-group", &["Regoin", "Hardware"], "Regoin"),
("toggle-group", &["Type", "Nowhere"], "Nowhere"),
("switch-view", &["Nowhere"], "Nowhere"),
];
let untouched = crate::persistence::format_md(&wb);
for (name, args, missing) in cases {
let app = run_on(&wb, name, args);
let said = &app.view_state.status_msg;
assert!(
app.last_batch_refused(),
"`{name} {args:?}` was not refused: {said:?}"
);
assert!(
said.contains(missing),
"`{name} {args:?}` did not name {missing}: {said:?}"
);
assert_eq!(
crate::persistence::format_md(&app.model_state.workbook),
untouched,
"`{name} {args:?}` changed the document"
);
assert_eq!(
app.view_state.active_view, wb.active_view,
"`{name}` moved the view"
);
}
}
/// The controls: the same commands naming what IS there are not refused.
#[test]
fn a_command_naming_what_the_document_has_is_not_refused() {
let wb = named_things();
let cases: [(&str, &[&str]); 6] = [
("set-axis", &["Type", "column"]),
("set-page", &["Type", "Clothing"]),
("hide-item", &["Type", "Clothing"]),
("show-item", &["Type", "Clothing"]),
("toggle-group", &["Type", "Hardware"]),
("switch-view", &["Second"]),
];
for (name, args) in cases {
let app = run_on(&wb, name, args);
assert!(
!app.last_batch_refused(),
"`{name} {args:?}` was refused: {:?}",
app.view_state.status_msg
);
}
}
/// An axis keyword is an argument error, caught before anything runs.
#[test]
fn set_axis_with_an_unknown_axis_is_an_argument_error() {
let err = crate::command::parse_line("set-axis Type sideways")
.expect_err("sideways is not an axis");
assert!(err.starts_with("set-axis:"), "names the command: {err}");
assert!(err.contains("sideways"), "names the axis: {err}");
}
#[test]
fn write_cmd_without_args_saves() {
let m = two_cat_model();
@@ -129,7 +227,7 @@ mod tests {
// Thin Cmd wrappers around effects, on the same path as keymap dispatch.
macro_rules! effect_cmd {
($name:ident, $cmd_name:expr, $parse:expr, $exec:expr) => {
($name:ident, $cmd_name:expr, $max:expr, $parse:expr, $exec:expr) => {
#[derive(Debug)]
pub struct $name(pub Vec<String>);
impl Cmd for $name {
@@ -143,6 +241,7 @@ macro_rules! effect_cmd {
}
}
impl $name {
pub const MAX_ARGS: MaxArgs = $max;
pub fn parse(args: &[String]) -> Result<Box<dyn Cmd>, String> {
#[allow(clippy::redundant_closure_call)]
($parse)(args)?;
@@ -152,9 +251,16 @@ macro_rules! effect_cmd {
};
}
/// `effect`, preceded by the check that what it names exists, run at APPLY time:
/// a `:` line is built before any of it runs — `context/code/cli.md` §15.
fn unless_unknown(needs: effect::Needs, effect: impl Effect + 'static) -> Vec<Box<dyn Effect>> {
vec![Box::new(effect::RequireKnown(needs)), Box::new(effect)]
}
effect_cmd!(
AddCategoryCmd,
"add-category",
MaxArgs::AtMost(1),
|args: &[String]| require_args(args, 1),
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![Box::new(effect::AddCategory(args[0].clone()))]
@@ -164,6 +270,7 @@ effect_cmd!(
effect_cmd!(
AddItemCmd,
"add-item",
MaxArgs::AtMost(2),
|args: &[String]| require_args(args, 2),
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![Box::new(effect::AddItem {
@@ -176,6 +283,7 @@ effect_cmd!(
effect_cmd!(
AddItemsCmd,
"add-items",
MaxArgs::Any,
|args: &[String]| {
if args.len() < 2 {
Err("add-items requires a category and at least one item".to_string())
@@ -200,6 +308,7 @@ effect_cmd!(
effect_cmd!(
AddItemInGroupCmd,
"add-item-in-group",
MaxArgs::AtMost(3),
|args: &[String]| require_args(args, 3),
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![Box::new(effect::AddItemInGroup {
@@ -213,11 +322,12 @@ effect_cmd!(
effect_cmd!(
SetCellCmd,
"set-cell",
MaxArgs::Any,
|args: &[String]| {
if args.len() < 2 {
Err("set-cell requires a value and at least one Cat/Item coordinate".to_string())
} else {
Ok(())
parse_cell_key_from_args(&args[1..]).map(|_| ())
}
},
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
@@ -226,21 +336,17 @@ effect_cmd!(
} else {
CellValue::Text(args[0].clone())
};
let coords: Vec<(String, String)> = args[1..]
.iter()
.filter_map(|s| {
let (cat, item) = s.split_once('/')?;
Some((cat.to_string(), item.to_string()))
})
.collect();
let key = crate::model::cell::CellKey::new(coords);
vec![Box::new(effect::SetCell(key, value))]
match parse_cell_key_from_args(&args[1..]) {
Ok(key) => vec![Box::new(effect::SetCell(key, value))],
Err(why) => vec![effect::refuse(why)],
}
}
);
effect_cmd!(
AddFormulaCmd,
"add-formula",
MaxArgs::AtMost(2),
|args: &[String]| {
if args.is_empty() || args.len() > 2 {
// The arity alone does not tell anyone to quote — improvise-z05h.
@@ -269,6 +375,7 @@ effect_cmd!(
effect_cmd!(
ClearBufferCmd,
"clear-buffer",
MaxArgs::AtMost(1),
|args: &[String]| require_args(args, 1),
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![super::core::write_buffer(&args[0], String::new())]
@@ -278,6 +385,7 @@ effect_cmd!(
effect_cmd!(
RemoveFormulaCmd,
"remove-formula",
MaxArgs::AtMost(2),
|args: &[String]| require_args(args, 2),
|args: &Vec<String>, ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
let (category, target) = (&args[0], &args[1]);
@@ -298,6 +406,7 @@ effect_cmd!(
effect_cmd!(
CreateViewCmd,
"create-view",
MaxArgs::AtMost(1),
|args: &[String]| require_args(args, 1),
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![Box::new(effect::CreateView(args[0].clone()))]
@@ -307,6 +416,7 @@ effect_cmd!(
effect_cmd!(
DeleteViewCmd,
"delete-view",
MaxArgs::AtMost(1),
|args: &[String]| require_args(args, 1),
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![Box::new(effect::DeleteView(args[0].clone()))]
@@ -316,87 +426,111 @@ effect_cmd!(
effect_cmd!(
SwitchViewCmd,
"switch-view",
MaxArgs::AtMost(1),
|args: &[String]| require_args(args, 1),
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![Box::new(effect::SwitchView(args[0].clone()))]
unless_unknown(
effect::Needs::View(args[0].clone()),
effect::SwitchView(args[0].clone()),
)
}
);
effect_cmd!(
SetAxisCmd,
"set-axis",
|args: &[String]| require_args(args, 2),
MaxArgs::AtMost(2),
|args: &[String]| {
require_args(args, 2)?;
parse_axis(&args[1]).map(|_| ())
},
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
let axis = match args[1].to_lowercase().as_str() {
"row" => Axis::Row,
"column" | "col" => Axis::Column,
"page" => Axis::Page,
"none" => Axis::None,
_ => return vec![], // parse step already validated
};
vec![Box::new(effect::SetAxis {
view: ViewTarget::Active,
category: args[0].clone(),
axis,
})]
match parse_axis(&args[1]) {
Ok(axis) => unless_unknown(
effect::Needs::Category(args[0].clone()),
effect::SetAxis {
view: ViewTarget::Active,
category: args[0].clone(),
axis,
},
),
Err(why) => vec![effect::refuse(why)],
}
}
);
effect_cmd!(
SetPageCmd,
"set-page",
MaxArgs::AtMost(2),
|args: &[String]| require_args(args, 2),
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![Box::new(effect::SetPageSelection {
view: ViewTarget::Active,
category: args[0].clone(),
item: args[1].clone(),
})]
unless_unknown(
effect::Needs::Item(args[0].clone(), args[1].clone()),
effect::SetPageSelection {
view: ViewTarget::Active,
category: args[0].clone(),
item: args[1].clone(),
},
)
}
);
effect_cmd!(
ToggleGroupCmd,
"toggle-group",
MaxArgs::AtMost(2),
|args: &[String]| require_args(args, 2),
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![Box::new(effect::ToggleGroup {
view: ViewTarget::Active,
category: args[0].clone(),
group: args[1].clone(),
})]
unless_unknown(
effect::Needs::Group(args[0].clone(), args[1].clone()),
effect::ToggleGroup {
view: ViewTarget::Active,
category: args[0].clone(),
group: args[1].clone(),
},
)
}
);
effect_cmd!(
HideItemCmd,
"hide-item",
MaxArgs::AtMost(2),
|args: &[String]| require_args(args, 2),
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![Box::new(effect::HideItem {
view: ViewTarget::Active,
category: args[0].clone(),
item: args[1].clone(),
})]
unless_unknown(
effect::Needs::Item(args[0].clone(), args[1].clone()),
effect::HideItem {
view: ViewTarget::Active,
category: args[0].clone(),
item: args[1].clone(),
},
)
}
);
effect_cmd!(
ShowItemCmd,
"show-item",
MaxArgs::AtMost(2),
|args: &[String]| require_args(args, 2),
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![Box::new(effect::ShowItem {
view: ViewTarget::Active,
category: args[0].clone(),
item: args[1].clone(),
})]
unless_unknown(
effect::Needs::Item(args[0].clone(), args[1].clone()),
effect::ShowItem {
view: ViewTarget::Active,
category: args[0].clone(),
item: args[1].clone(),
},
)
}
);
effect_cmd!(
SaveAsCmd,
"save-as",
MaxArgs::AtMost(1),
|args: &[String]| require_args(args, 1),
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![Box::new(effect::SaveAs(std::path::PathBuf::from(&args[0])))]
@@ -406,6 +540,7 @@ effect_cmd!(
effect_cmd!(
SetFormatCmd,
"set-format",
MaxArgs::Any,
|args: &[String]| require_args(args, 1),
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![Box::new(effect::SetNumberFormat(
@@ -418,6 +553,7 @@ effect_cmd!(
effect_cmd!(
ImportCmd,
"import",
MaxArgs::AtMost(1),
|args: &[String]| require_args(args, 1),
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![Box::new(effect::StartImportWizard(args[0].clone()))]
@@ -427,6 +563,7 @@ effect_cmd!(
effect_cmd!(
ExportCmd,
"export",
MaxArgs::AtMost(1),
|_args: &[String]| -> Result<(), String> { Ok(()) },
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
let path = args.first().map(|s| s.as_str()).unwrap_or("export.csv");
@@ -437,6 +574,7 @@ effect_cmd!(
effect_cmd!(
WriteCmd,
"w",
MaxArgs::AtMost(1),
|_args: &[String]| -> Result<(), String> { Ok(()) },
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
if args.is_empty() {
@@ -450,6 +588,7 @@ effect_cmd!(
effect_cmd!(
HelpCmd,
"help",
MaxArgs::AtMost(0),
|_args: &[String]| -> Result<(), String> { Ok(()) },
|_args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![
@@ -462,6 +601,7 @@ effect_cmd!(
effect_cmd!(
HelpPageNextCmd,
"help-page-next",
MaxArgs::AtMost(0),
|_args: &[String]| -> Result<(), String> { Ok(()) },
|_args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![effect::help_page_next()]
@@ -471,6 +611,7 @@ effect_cmd!(
effect_cmd!(
HelpPagePrevCmd,
"help-page-prev",
MaxArgs::AtMost(0),
|_args: &[String]| -> Result<(), String> { Ok(()) },
|_args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![effect::help_page_prev()]
@@ -480,6 +621,7 @@ effect_cmd!(
effect_cmd!(
LoadModelCmd,
"load",
MaxArgs::AtMost(1),
|args: &[String]| require_args(args, 1),
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![Box::new(effect::LoadModel(std::path::PathBuf::from(
@@ -491,6 +633,7 @@ effect_cmd!(
effect_cmd!(
ExportCsvCmd,
"export-csv",
MaxArgs::AtMost(1),
|args: &[String]| require_args(args, 1),
|args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![Box::new(effect::ExportCsv(std::path::PathBuf::from(
@@ -502,6 +645,7 @@ effect_cmd!(
effect_cmd!(
ImportJsonCmd,
"import-json",
MaxArgs::AtMost(3),
|args: &[String]| {
if args.is_empty() {
Err("import-json requires a path".to_string())
@@ -4,7 +4,7 @@
use improvise_gp::fit::one_line;
use improvise_gp::{FitRequest, fit_measure, fitted_measure_name};
use crate::command::cmd::core::{Cmd, CmdContext};
use crate::command::cmd::core::{Cmd, CmdContext, MaxArgs};
use crate::ui::effect::{self, Effect};
/// A seed as the status line spells it (`0x…`, either case) or in decimal. The hex
@@ -27,6 +27,8 @@ pub struct FitMeasure {
}
impl FitMeasure {
pub const MAX_ARGS: MaxArgs = MaxArgs::AtMost(3);
/// A nameless instance for `register_pure`, which reads a command's name
/// off a prototype rather than off a string literal (gotcha 15).
pub fn prototype() -> Self {
@@ -81,52 +81,130 @@ pub fn default_registry() -> CmdRegistry {
let mut r = CmdRegistry::new();
// ── Model mutations (effect_cmd! wrappers) ───────────────────────────
r.register_pure(&AddCategoryCmd(vec![]), AddCategoryCmd::parse);
r.register_pure(&AddItemCmd(vec![]), AddItemCmd::parse);
r.register_pure(&AddItemsCmd(vec![]), AddItemsCmd::parse);
r.register_pure(&AddItemInGroupCmd(vec![]), AddItemInGroupCmd::parse);
r.register_pure(&SetCellCmd(vec![]), SetCellCmd::parse);
r.register_pure(&ClearBufferCmd(vec![]), ClearBufferCmd::parse);
r.register_pure(
&AddCategoryCmd(vec![]),
AddCategoryCmd::MAX_ARGS,
AddCategoryCmd::parse,
);
r.register_pure(&AddItemCmd(vec![]), AddItemCmd::MAX_ARGS, AddItemCmd::parse);
r.register_pure(
&AddItemsCmd(vec![]),
AddItemsCmd::MAX_ARGS,
AddItemsCmd::parse,
);
r.register_pure(
&AddItemInGroupCmd(vec![]),
AddItemInGroupCmd::MAX_ARGS,
AddItemInGroupCmd::parse,
);
r.register_pure(&SetCellCmd(vec![]), SetCellCmd::MAX_ARGS, SetCellCmd::parse);
r.register_pure(
&ClearBufferCmd(vec![]),
ClearBufferCmd::MAX_ARGS,
ClearBufferCmd::parse,
);
r.register(
&ClearCellCommand {
key: CellKey::new(vec![]),
},
MaxArgs::Any,
|args| {
if args.is_empty() {
return Err("clear-cell requires at least one Cat/Item coordinate".into());
}
Ok(ClearCellCommand {
key: parse_cell_key_from_args(args),
key: parse_cell_key_from_args(args)?,
})
},
Some(|_, ctx| {
Some(|args, ctx| {
only_when_omitted(args)?;
Ok(ClearCellCommand {
key: cursor_value_key(ctx)?,
})
}),
);
r.register_pure(&AddFormulaCmd(vec![]), AddFormulaCmd::parse);
r.register_pure(&FitMeasure::prototype(), FitMeasure::parse);
r.register_pure(&RemoveFormulaCmd(vec![]), RemoveFormulaCmd::parse);
r.register_pure(&CreateViewCmd(vec![]), CreateViewCmd::parse);
r.register_pure(&DeleteViewCmd(vec![]), DeleteViewCmd::parse);
r.register_pure(&SwitchViewCmd(vec![]), SwitchViewCmd::parse);
r.register_pure(&SetAxisCmd(vec![]), SetAxisCmd::parse);
r.register_pure(&SetPageCmd(vec![]), SetPageCmd::parse);
r.register_pure(&ToggleGroupCmd(vec![]), ToggleGroupCmd::parse);
r.register_pure(&HideItemCmd(vec![]), HideItemCmd::parse);
r.register_pure(&ShowItemCmd(vec![]), ShowItemCmd::parse);
r.register_pure(&SaveAsCmd(vec![]), SaveAsCmd::parse);
r.register_pure(&LoadModelCmd(vec![]), LoadModelCmd::parse);
r.register_pure(&ExportCsvCmd(vec![]), ExportCsvCmd::parse);
r.register_pure(&ImportJsonCmd(vec![]), ImportJsonCmd::parse);
r.register_pure(&SetFormatCmd(vec![]), SetFormatCmd::parse);
r.register_pure(&ImportCmd(vec![]), ImportCmd::parse);
r.register_pure(&ExportCmd(vec![]), ExportCmd::parse);
r.register_pure(&WriteCmd(vec![]), WriteCmd::parse);
r.register_pure(&HelpCmd(vec![]), HelpCmd::parse);
r.register_pure(&HelpPageNextCmd(vec![]), HelpPageNextCmd::parse);
r.register_pure(&HelpPagePrevCmd(vec![]), HelpPagePrevCmd::parse);
r.register_pure(
&AddFormulaCmd(vec![]),
AddFormulaCmd::MAX_ARGS,
AddFormulaCmd::parse,
);
r.register_pure(
&FitMeasure::prototype(),
FitMeasure::MAX_ARGS,
FitMeasure::parse,
);
r.register_pure(
&RemoveFormulaCmd(vec![]),
RemoveFormulaCmd::MAX_ARGS,
RemoveFormulaCmd::parse,
);
r.register_pure(
&CreateViewCmd(vec![]),
CreateViewCmd::MAX_ARGS,
CreateViewCmd::parse,
);
r.register_pure(
&DeleteViewCmd(vec![]),
DeleteViewCmd::MAX_ARGS,
DeleteViewCmd::parse,
);
r.register_pure(
&SwitchViewCmd(vec![]),
SwitchViewCmd::MAX_ARGS,
SwitchViewCmd::parse,
);
r.register_pure(&SetAxisCmd(vec![]), SetAxisCmd::MAX_ARGS, SetAxisCmd::parse);
r.register_pure(&SetPageCmd(vec![]), SetPageCmd::MAX_ARGS, SetPageCmd::parse);
r.register_pure(
&ToggleGroupCmd(vec![]),
ToggleGroupCmd::MAX_ARGS,
ToggleGroupCmd::parse,
);
r.register_pure(
&HideItemCmd(vec![]),
HideItemCmd::MAX_ARGS,
HideItemCmd::parse,
);
r.register_pure(
&ShowItemCmd(vec![]),
ShowItemCmd::MAX_ARGS,
ShowItemCmd::parse,
);
r.register_pure(&SaveAsCmd(vec![]), SaveAsCmd::MAX_ARGS, SaveAsCmd::parse);
r.register_pure(
&LoadModelCmd(vec![]),
LoadModelCmd::MAX_ARGS,
LoadModelCmd::parse,
);
r.register_pure(
&ExportCsvCmd(vec![]),
ExportCsvCmd::MAX_ARGS,
ExportCsvCmd::parse,
);
r.register_pure(
&ImportJsonCmd(vec![]),
ImportJsonCmd::MAX_ARGS,
ImportJsonCmd::parse,
);
r.register_pure(
&SetFormatCmd(vec![]),
SetFormatCmd::MAX_ARGS,
SetFormatCmd::parse,
);
r.register_pure(&ImportCmd(vec![]), ImportCmd::MAX_ARGS, ImportCmd::parse);
r.register_pure(&ExportCmd(vec![]), ExportCmd::MAX_ARGS, ExportCmd::parse);
r.register_pure(&WriteCmd(vec![]), WriteCmd::MAX_ARGS, WriteCmd::parse);
r.register_pure(&HelpCmd(vec![]), HelpCmd::MAX_ARGS, HelpCmd::parse);
r.register_pure(
&HelpPageNextCmd(vec![]),
HelpPageNextCmd::MAX_ARGS,
HelpPageNextCmd::parse,
);
r.register_pure(
&HelpPagePrevCmd(vec![]),
HelpPagePrevCmd::MAX_ARGS,
HelpPagePrevCmd::parse,
);
// ── Navigation (unified Move) ──────────────────────────────────────
r.register_at_cursor(
@@ -135,6 +213,7 @@ pub fn default_registry() -> CmdRegistry {
cursor: CursorState::default(),
cmd_name: "move-selection",
},
MaxArgs::AtMost(2),
|args| {
require_args(args, 2)?;
let dr = args[0].parse::<i32>().map_err(|e| e.to_string())?;
@@ -160,6 +239,7 @@ pub fn default_registry() -> CmdRegistry {
cursor: CursorState::default(),
cmd_name: $name,
},
MaxArgs::AtMost(0),
|_| {
Ok(Move {
kind: if $to_end {
@@ -185,6 +265,7 @@ pub fn default_registry() -> CmdRegistry {
cursor: CursorState::default(),
cmd_name: "scroll-rows",
},
MaxArgs::AtMost(1),
|args| {
require_args(args, 1)?;
let n = args[0].parse::<i32>().map_err(|e| e.to_string())?;
@@ -202,6 +283,7 @@ pub fn default_registry() -> CmdRegistry {
cursor: CursorState::default(),
cmd_name: "page-scroll",
},
MaxArgs::AtMost(1),
|args| {
require_args(args, 1)?;
let dir = args[0].parse::<i32>().map_err(|e| e.to_string())?;
@@ -217,6 +299,7 @@ pub fn default_registry() -> CmdRegistry {
&EnterAdvance {
cursor: CursorState::default(),
},
MaxArgs::AtMost(0),
|_| {
Ok(EnterAdvance {
cursor: CursorState {
@@ -239,15 +322,17 @@ pub fn default_registry() -> CmdRegistry {
&YankCell {
key: CellKey::new(vec![]),
},
MaxArgs::Any,
|args| {
if args.is_empty() {
return Err("yank requires at least one Cat/Item coordinate".into());
}
Ok(YankCell {
key: parse_cell_key_from_args(args),
key: parse_cell_key_from_args(args)?,
})
},
Some(|_, ctx| {
Some(|args, ctx| {
only_when_omitted(args)?;
Ok(YankCell {
key: cursor_key_to_read(ctx)?,
})
@@ -257,15 +342,17 @@ pub fn default_registry() -> CmdRegistry {
&PasteCell {
at: PasteAt::Spelled(CellKey::new(vec![])),
},
MaxArgs::Any,
|args| {
if args.is_empty() {
return Err("paste requires at least one Cat/Item coordinate".into());
}
Ok(PasteCell {
at: PasteAt::Spelled(parse_cell_key_from_args(args)),
at: PasteAt::Spelled(parse_cell_key_from_args(args)?),
})
},
Some(|_, ctx| {
Some(|args, ctx| {
only_when_omitted(args)?;
Ok(PasteCell {
at: PasteAt::Resolved(cursor_value_key(ctx)?),
})
@@ -288,6 +375,7 @@ pub fn default_registry() -> CmdRegistry {
&EditOrDrill {
edit_mode: AppMode::Normal,
},
MaxArgs::AtMost(1),
|args| {
require_args(args, 1)?;
let edit_mode = parse_mode_name(&args[0])?;
@@ -298,6 +386,7 @@ pub fn default_registry() -> CmdRegistry {
&EnterEditAtCursorCmd {
target_mode: AppMode::Normal,
},
MaxArgs::AtMost(1),
|args| {
require_args(args, 1)?;
let target_mode = parse_mode_name(&args[0])?;
@@ -309,15 +398,17 @@ pub fn default_registry() -> CmdRegistry {
&DrillIntoCell {
key: crate::model::cell::CellKey::new(vec![]),
},
MaxArgs::Any,
|args| {
if args.is_empty() {
return Err("drill-into-cell requires Cat/Item coordinates".into());
}
Ok(DrillIntoCell {
key: parse_cell_key_from_args(args),
key: parse_cell_key_from_args(args)?,
})
},
Some(|_, ctx| {
Some(|args, ctx| {
only_when_omitted(args)?;
Ok(DrillIntoCell {
key: cursor_key(ctx)?,
})
@@ -325,13 +416,13 @@ pub fn default_registry() -> CmdRegistry {
);
r.register_nullary(|| Box::new(ViewNavigate { forward: false }));
r.register_nullary(|| Box::new(ViewNavigate { forward: true }));
r.register_pure(&EnterMode(AppMode::Normal), |args| {
r.register_pure(&EnterMode(AppMode::Normal), MaxArgs::AtMost(1), |args| {
require_args(args, 1)?;
Ok(Box::new(EnterMode(parse_mode_name(&args[0])?)))
});
// ── Search ───────────────────────────────────────────────────────────
r.register_pure(&SearchNavigate(true), |args| {
r.register_pure(&SearchNavigate(true), MaxArgs::AtMost(1), |args| {
let forward = args.first().map(|s| s != "backward").unwrap_or(true);
Ok(Box::new(SearchNavigate(forward)))
});
@@ -344,6 +435,7 @@ pub fn default_registry() -> CmdRegistry {
panel: Panel::Formula,
target: None,
},
MaxArgs::AtMost(3),
|args| {
// Parse: toggle-panel-and-focus <panel> [open] [focused]
require_args(args, 1)?;
@@ -360,6 +452,7 @@ pub fn default_registry() -> CmdRegistry {
&TogglePanelVisibility {
panel: Panel::Formula,
},
MaxArgs::AtMost(1),
|args| {
require_args(args, 1)?;
let panel = parse_panel(&args[0])?;
@@ -373,6 +466,7 @@ pub fn default_registry() -> CmdRegistry {
panel: Panel::Formula,
delta: 0,
},
MaxArgs::AtMost(2),
|args| {
require_args(args, 2)?;
let panel = parse_panel(&args[0])?;
@@ -396,7 +490,7 @@ pub fn default_registry() -> CmdRegistry {
r.register_nullary(|| Box::new(DeleteViewAtCursor));
// ── Tile select ──────────────────────────────────────────────────────
r.register_pure(&MoveTileCursor(0), |args| {
r.register_pure(&MoveTileCursor(0), MaxArgs::AtMost(1), |args| {
require_args(args, 1)?;
let delta = args[0].parse::<i32>().map_err(|e| e.to_string())?;
Ok(Box::new(MoveTileCursor(delta)))
@@ -406,6 +500,7 @@ pub fn default_registry() -> CmdRegistry {
&TileAxisOp {
axis: Some(crate::view::Axis::Row),
},
MaxArgs::AtMost(1),
|args| {
require_args(args, 1)?;
let axis = parse_axis(&args[0])?;
@@ -423,6 +518,7 @@ pub fn default_registry() -> CmdRegistry {
&AppendChar {
buffer: String::new(),
},
MaxArgs::AtMost(1),
|args| {
require_args(args, 1)?;
Ok(Box::new(AppendChar {
@@ -434,6 +530,7 @@ pub fn default_registry() -> CmdRegistry {
&PopChar {
buffer: String::new(),
},
MaxArgs::AtMost(1),
|args| {
require_args(args, 1)?;
Ok(Box::new(PopChar {
@@ -454,13 +551,14 @@ pub fn default_registry() -> CmdRegistry {
edit_mode: AppMode::editing(),
opened: None,
},
MaxArgs::Any,
|args| {
if args.len() < 3 {
return Err("commit-cell-edit requires a mode, value, and coords".into());
}
let edit_mode = parse_mode_name(&args[0])?;
Ok(CommitAndAdvance {
key: parse_cell_key_from_args(&args[2..]),
key: parse_cell_key_from_args(&args[2..])?,
value: args[1].clone(),
advance: AdvanceDir::Down,
cursor: CursorState::default(),
@@ -479,6 +577,7 @@ pub fn default_registry() -> CmdRegistry {
edit_mode: AppMode::editing(),
opened: None,
},
MaxArgs::AtMost(1),
|_| Err("commit-and-advance-right requires context".into()),
Some(|args, ctx| commit_at_cursor(args, ctx, AdvanceDir::Right)),
);
@@ -486,6 +585,7 @@ pub fn default_registry() -> CmdRegistry {
&CommitBuffer {
kind: CommitKind::Category,
},
MaxArgs::AtMost(1),
|args| {
require_args(args, 1)?;
let kind = parse_commit_kind(&args[0], &AppMode::Normal)?;
@@ -534,6 +634,7 @@ pub(super) mod tests {
vec!["a"],
vec!["Region"],
vec!["Region", "East"],
vec!["Region", "row"],
vec!["Region", "East", "1"],
vec!["Profit = 1"],
vec![",.2f"],
@@ -544,6 +645,7 @@ pub(super) mod tests {
vec!["editing", "5", "Region/East"],
vec!["formula", "false", "false"],
vec!["Region/East"],
vec!["1", "Region/East"],
]
.into_iter()
.map(|v| v.into_iter().map(String::from).collect())
@@ -592,6 +694,87 @@ pub(super) mod tests {
);
}
/// Evidence: `context/test-evidence/cli.md`.
#[test]
fn no_command_silently_drops_an_argument_it_was_given() {
let r = default_registry();
let wb = two_cat_model();
let layout = make_layout(&wb);
let reg = make_registry();
let ctx = make_ctx(&wb, &layout, &reg);
let variadic = [
"add-items",
"set-cell",
"set-format",
"clear-cell",
"yank",
"paste",
"drill-into-cell",
"commit-cell-edit",
];
// Their own refusal states the shape, and it runs before the rule.
let own_shape = ["add-formula", "fit-measure", "commit-and-advance-right"];
let mut checked = 0;
for name in r.names().collect::<Vec<_>>() {
if variadic.contains(&name) {
continue;
}
for args in candidate_args() {
// Four more than any list accepted, so an optional argument
// cannot absorb them; the last one is surplus whatever the command.
let mut more = args.clone();
more.extend(["EXTRA1", "EXTRA2", "EXTRA3", "EXTRA4"].map(String::from));
let arms = [
r.parse(name, &args).map(|_| r.parse(name, &more)),
r.interactive(name, &args, &ctx)
.map(|_| r.interactive(name, &more, &ctx)),
];
for extended in arms.into_iter().flatten() {
checked += 1;
match extended {
Ok(_) => panic!("`{name} {more:?}` dropped arguments without a word"),
Err(e) if own_shape.contains(&name) || e.contains("\"EXTRA4\"") => {}
Err(e) => {
panic!("`{name}` refused an extra argument without naming it: {e}")
}
}
}
}
}
assert!(checked > 100, "only {checked} accepted lists were extended");
}
/// Codex on PR #316: a variadic coordinate list dropped a token that was no
/// coordinate. Evidence: `context/test-evidence/cli.md`.
#[test]
fn a_malformed_coordinate_is_refused_not_dropped() {
let lines = [
"set-cell 1 Type/Food unexpected",
"clear-cell Type/Food unexpected",
"yank Type/Food unexpected",
"paste Type/Food unexpected",
"drill-into-cell Type/Food unexpected",
"commit-cell-edit editing 5 Type/Food unexpected",
];
for line in lines {
let err = crate::command::parse_line(line)
.err()
.unwrap_or_else(|| panic!("`{line}` dropped \"unexpected\" without a word"));
assert!(
err.contains("\"unexpected\""),
"`{line}` did not name it: {err}"
);
}
for half_empty in ["set-cell 1 Type/", "clear-cell /Food"] {
assert!(
crate::command::parse_line(half_empty).is_err(),
"`{half_empty}`"
);
}
let control = "set-cell 1 Type/Food Month/Jan . clear-cell Type/Food";
assert!(crate::command::parse_line(control).is_ok(), "the control");
}
/// A variant mix-up shows up as a **collision**, not a mismatch: the wrong entry
/// takes its sibling's name and the sibling vanishes. Uniqueness catches it.
#[test]
@@ -216,6 +216,110 @@ mod tests {
app.apply_effects(effects);
}
fn cell(wb: &crate::workbook::Workbook, ty: &str, month: &str) -> Option<f64> {
use crate::model::cell::{CellKey, CellValue};
let key = CellKey::new(vec![
("Type".into(), ty.into()),
("Month".into(), month.into()),
]);
match wb.model.get_cell(&key) {
Some(CellValue::Number(n)) => Some(*n),
_ => None,
}
}
/// An app whose cursor (0,0) is on Food/Jan = 1, with Clothing/Feb = 2 beside it.
fn app_with_two_cells() -> crate::ui::app::App {
use crate::model::cell::{CellKey, CellValue};
let mut app = wizard_test_app();
for (ty, month, n) in [("Food", "Jan", 1.0), ("Clothing", "Feb", 2.0)] {
let key = CellKey::new(vec![
("Type".into(), ty.into()),
("Month".into(), month.into()),
]);
app.model_state
.workbook
.model
.set_cell(key, CellValue::Number(n));
}
app
}
/// PR #316 review: a malformed coordinate on `:` fell into the cursor fallback
/// and wrote the CURSOR's cell. Evidence: `context/test-evidence/cli.md`.
#[test]
fn colon_refuses_a_malformed_coordinate_and_leaves_the_cursor_cell_alone() {
let lines = [
"clear-cell Type/Clothing Month/Feb junk",
"yank Type/Clothing Month/Feb junk",
"paste Type/Clothing Month/Feb junk",
"drill-into-cell Type/Clothing Month/Feb junk",
];
for line in lines {
let mut app = app_with_two_cells();
app.view_state.yanked = Some(crate::model::cell::CellValue::Number(9.0));
let before = app.view_state.yanked.clone();
run_colon_command(&mut app, line);
assert!(app.last_batch_refused(), "`:{line}` was not refused");
assert!(
app.view_state.status_msg.contains("\"junk\""),
"`:{line}` did not name the token: {:?}",
app.view_state.status_msg
);
let wb = &app.model_state.workbook;
assert_eq!(
cell(wb, "Food", "Jan"),
Some(1.0),
"`:{line}` wrote the cursor cell"
);
assert_eq!(
cell(wb, "Clothing", "Feb"),
Some(2.0),
"`:{line}` wrote the named cell"
);
assert_eq!(app.view_state.yanked, before, "`:{line}` yanked");
assert!(app.drill_state.is_none(), "`:{line}` opened a drill");
}
}
/// The fallback still answers for coordinates that were OMITTED.
#[test]
fn colon_clear_cell_with_no_coordinates_still_uses_the_cursor() {
let mut app = app_with_two_cells();
run_colon_command(&mut app, "clear-cell");
let wb = &app.model_state.workbook;
assert_eq!(cell(wb, "Food", "Jan"), None);
assert_eq!(cell(wb, "Clothing", "Feb"), Some(2.0));
}
/// A later `:` segment sees what an earlier one made, as `improvise cmd` does:
/// the line is built before either half runs. Evidence: `context/test-evidence/cli.md`.
#[test]
fn colon_lets_a_later_segment_use_what_an_earlier_one_created() {
let mut app = wizard_test_app();
run_colon_command(&mut app, "create-view Second . switch-view Second");
assert!(!app.last_batch_refused(), "{:?}", app.view_state.status_msg);
assert_eq!(app.view_state.active_view, "Second");
let mut app = wizard_test_app();
run_colon_command(&mut app, "add-item Type Tools . set-page Type Tools");
assert!(!app.last_batch_refused(), "{:?}", app.view_state.status_msg);
}
/// The control: a name NOTHING creates is still refused, on `:` too.
#[test]
fn colon_still_refuses_a_name_nothing_creates() {
for line in [
"switch-view Nowhere",
"set-page Type Nowhere",
"hide-item Regoin Food",
] {
let mut app = wizard_test_app();
run_colon_command(&mut app, line);
assert!(app.last_batch_refused(), "`:{line}` was not refused");
}
}
/// improvise-toc: `StartImportWizard` swaps the mode only on success, so a
/// per-type answer snapped `:import` straight back out of the wizard.
#[test]
@@ -271,6 +375,43 @@ mod tests {
);
}
/// improvise-31oe.2 on the `:` line: the refusal is on the status line,
/// naming what is missing. Evidence: `context/test-evidence/cli.md`.
#[test]
fn a_colon_line_naming_something_missing_refuses_on_the_status_line() {
for (line, missing) in [
("set-axis Regoin row", "Regoin"),
("set-axis Type sideways", "sideways"),
("set-page Type Nowhere", "Nowhere"),
("create-view By area", "\"area\""),
] {
let mut app = wizard_test_app();
run_colon_command(&mut app, line);
let said = &app.view_state.status_msg;
assert!(
app.last_batch_refused(),
"`:{line}` was not refused: {said:?}"
);
assert!(
said.contains(missing),
"`:{line}` did not name {missing}: {said:?}"
);
assert!(matches!(app.view_state.mode, AppMode::Normal), "`:{line}`");
}
}
/// improvise-31oe.3 on the `:` line.
#[test]
fn a_colon_line_reads_a_pipe_quoted_name() {
let mut app = wizard_test_app();
run_colon_command(&mut app, "create-view |By area|");
let views: Vec<&String> = app.model_state.workbook.views.keys().collect();
assert!(
app.model_state.workbook.views.contains_key("By area"),
"{views:?}"
);
}
/// A command that does not touch the mode still returns to Normal.
#[test]
fn colon_plain_command_returns_to_normal() {
@@ -372,9 +513,12 @@ impl Cmd for ExecuteCommand {
// Built against the live cursor, exactly as a script's is — improvise-6uva.
let built: Result<Vec<Box<dyn Cmd>>, String> = crate::command::parse::segments(&raw)
.into_iter()
.map(|seg| crate::command::parse::parse_segment_interactive(seg, ctx))
.collect::<Result<Vec<_>, _>>()
.and_then(|segments| {
segments
.into_iter()
.map(|seg| crate::command::parse::parse_segment_interactive(seg, ctx))
.collect::<Result<Vec<_>, _>>()
})
.map(|built| built.into_iter().flatten().collect());
match built {
Ok(cmds) => {
@@ -65,6 +65,7 @@ fn review_proposal_keys(key: KeyCode) -> Vec<Box<dyn Effect>> {
fn configure_date_keys(key: KeyCode) -> Vec<Box<dyn Effect>> {
match key {
KeyCode::Char(' ') => op(WizardOp::ToggleDateComponent),
KeyCode::Char('s') => op(WizardOp::ToggleDateShape),
KeyCode::Enter => op(WizardOp::Advance),
_ => cursor_keys(key),
}
+208 -85
View File
@@ -1,5 +1,10 @@
//! Quasi-lisp prefix command parser: `word arg1 arg2 …`, `.` between commands,
//! `Category/Item` coordinate pairs, `"quoted strings"`.
//! `Category/Item` coordinate pairs, `"quoted strings"` and `|pipe-quoted names|`.
use std::iter::Peekable;
use std::str::CharIndices;
use improvise_formula::quote::unescape_pipe_body;
use super::cmd::{Cmd, CmdContext, CmdRegistry, default_registry};
@@ -11,39 +16,19 @@ pub fn parse_line(line: &str) -> Result<Vec<Box<dyn Cmd>>, String> {
/// Parse a line into commands using a given registry.
pub fn parse_line_with(registry: &CmdRegistry, line: &str) -> Result<Vec<Box<dyn Cmd>>, String> {
let line = line.trim();
if line.is_empty() || line.starts_with('#') || line.starts_with("//") {
return Ok(vec![]);
}
let mut commands = Vec::new();
for segment in split_on_dot(line) {
let segment = segment.trim();
if segment.is_empty() {
continue;
}
let tokens = tokenize(segment);
if tokens.is_empty() {
continue;
}
let word = &tokens[0];
let args = &tokens[1..];
commands.push(registry.parse(word, args)?);
}
Ok(commands)
groups(line.trim())?
.iter()
.map(|words| registry.parse(&words[0].text, &texts(&words[1..])))
.collect()
}
/// The command segments of a line, with comments and blank segments dropped.
pub fn segments(line: &str) -> Vec<&str> {
pub fn segments(line: &str) -> Result<Vec<&str>, String> {
let line = line.trim();
if line.is_empty() || line.starts_with('#') || line.starts_with("//") {
return Vec::new();
}
split_on_dot(line)
.into_iter()
.map(str::trim)
.filter(|s| !s.is_empty())
.collect()
Ok(groups(line)?
.iter()
.map(|words| &line[words[0].start..words[words.len() - 1].end])
.collect())
}
/// Build one segment's command against a LIVE context — `context/code/cli.md`.
@@ -51,7 +36,7 @@ pub fn parse_segment_interactive(
segment: &str,
ctx: &CmdContext,
) -> Result<Option<Box<dyn Cmd>>, String> {
let tokens = tokenize(segment.trim());
let tokens = texts(&lex(segment.trim())?);
let Some(word) = tokens.first() else {
return Ok(None);
};
@@ -67,65 +52,99 @@ pub fn parse_segment_interactive(
})
}
/// Split a line on ` . ` separators (dot must be a standalone word,
/// surrounded by whitespace or at line boundaries). Respects quoted strings.
fn split_on_dot(line: &str) -> Vec<&str> {
let mut segments = Vec::new();
let mut start = 0;
let mut in_quote = false;
let bytes = line.as_bytes();
for (i, c) in line.char_indices() {
match c {
'"' => in_quote = !in_quote,
'.' if !in_quote => {
let before_ws = i == 0 || bytes[i - 1].is_ascii_whitespace();
let after_ws = i + 1 >= bytes.len() || bytes[i + 1].is_ascii_whitespace();
if before_ws && after_ws {
segments.push(&line[start..i]);
start = i + 1;
}
}
_ => {}
}
}
segments.push(&line[start..]);
segments
/// One word of a line and the byte span it was read from. A `separator` is a
/// bare `.`; a quoted one is an argument like any other.
struct Word {
text: String,
start: usize,
end: usize,
separator: bool,
}
/// Tokenize a command segment into words, handling quoted strings.
fn tokenize(input: &str) -> Vec<String> {
let mut tokens = Vec::new();
let mut chars = input.chars().peekable();
fn texts(words: &[Word]) -> Vec<String> {
words.iter().map(|w| w.text.clone()).collect()
}
while let Some(&c) = chars.peek() {
/// The words of a trimmed line, split at separators, blank commands and
/// comment lines dropped.
fn groups(line: &str) -> Result<Vec<Vec<Word>>, String> {
if line.starts_with('#') || line.starts_with("//") {
return Ok(Vec::new());
}
let mut groups: Vec<Vec<Word>> = vec![Vec::new()];
for word in lex(line)? {
match word.separator {
true => groups.push(Vec::new()),
false => groups.last_mut().expect("never empty").push(word),
}
}
groups.retain(|g| !g.is_empty());
Ok(groups)
}
/// Split `line` into words. The quoting rules: `context/code/cli.md` §14.
fn lex(line: &str) -> Result<Vec<Word>, String> {
let mut words = Vec::new();
let mut chars = line.char_indices().peekable();
while let Some(&(start, c)) = chars.peek() {
if c.is_whitespace() {
chars.next();
continue;
}
if c == '"' {
chars.next(); // consume opening quote
let mut s = String::new();
for ch in chars.by_ref() {
if ch == '"' {
break;
}
s.push(ch);
let (text, bare) = match c {
'"' => (double_quoted(&mut chars), false),
_ => bare_word(&mut chars, line)?,
};
let end = chars.peek().map_or(line.len(), |&(i, _)| i);
words.push(Word {
separator: bare && text == ".",
text,
start,
end,
});
}
Ok(words)
}
/// `"…"` read verbatim: no escapes, and a `|` inside stays a `|`.
fn double_quoted(chars: &mut Peekable<CharIndices>) -> String {
chars.next();
chars
.by_ref()
.map(|(_, c)| c)
.take_while(|&c| c != '"')
.collect()
}
/// Text up to whitespace, each `|…|` piece unquoted in place; `bare` is false
/// once a piece was quoted, so a quoted `|.|` is an argument and not a separator.
fn bare_word(chars: &mut Peekable<CharIndices>, line: &str) -> Result<(String, bool), String> {
let mut text = String::new();
let mut bare = true;
while let Some((at, c)) = chars.next_if(|&(_, c)| !c.is_whitespace()) {
match c {
'|' => {
text.push_str(&pipe_body(chars, line, at)?);
bare = false;
}
tokens.push(s);
} else {
let mut s = String::new();
while let Some(&ch) = chars.peek() {
if ch.is_whitespace() {
break;
}
s.push(ch);
chars.next();
}
tokens.push(s);
_ => text.push(c),
}
}
tokens
Ok((text, bare))
}
/// The body of the `|…|` opened at `open`, escapes applied by the one shared
/// reader (gotcha 34).
fn pipe_body(chars: &mut Peekable<CharIndices>, line: &str, open: usize) -> Result<String, String> {
let mut body = String::new();
while let Some((_, c)) = chars.next() {
match c {
'|' => return Ok(unescape_pipe_body(&body)),
'\\' => body.extend(std::iter::once(c).chain(chars.next().map(|(_, e)| e))),
_ => body.push(c),
}
}
Err(format!("unterminated |…| in `{}`", &line[open..]))
}
#[cfg(test)]
@@ -302,11 +321,14 @@ mod tests {
#[test]
fn segments_drops_comments_and_blanks() {
assert_eq!(segments("# a comment"), Vec::<&str>::new());
assert_eq!(segments("// also a comment"), Vec::<&str>::new());
assert_eq!(segments(" "), Vec::<&str>::new());
assert_eq!(segments("a . . b"), vec!["a", "b"]);
assert_eq!(segments("move-selection 2 0"), vec!["move-selection 2 0"]);
assert_eq!(segments("# a comment"), Ok(Vec::<&str>::new()));
assert_eq!(segments("// also a comment"), Ok(Vec::<&str>::new()));
assert_eq!(segments(" "), Ok(Vec::<&str>::new()));
assert_eq!(segments("a . . b"), Ok(vec!["a", "b"]));
assert_eq!(
segments("move-selection 2 0"),
Ok(vec!["move-selection 2 0"])
);
}
/// Evidence: `context/test-evidence/cli.md`.
@@ -347,4 +369,105 @@ mod tests {
.expect_err("an unknown word is refused");
assert!(err.contains("no-such-command"), "got: {err}");
}
// ── pipe quoting and arity (improvise-31oe.3) ─────────────────────────
/// The one command `line` builds, as its `Debug` shows it.
fn built(line: &str) -> String {
let cmds = parse_line(line).unwrap_or_else(|e| panic!("`{line}` must parse: {e}"));
assert_eq!(cmds.len(), 1, "`{line}` is one command");
format!("{:?}", cmds[0])
}
/// Evidence: `context/test-evidence/cli.md`.
#[test]
fn a_pipe_quoted_argument_is_one_argument_without_its_pipes() {
assert_eq!(
built("create-view |By area|"),
r#"CreateViewCmd(["By area"])"#
);
assert_eq!(
built("set-page |Sales Region| |East Coast|"),
r#"SetPageCmd(["Sales Region", "East Coast"])"#
);
}
/// Evidence: `context/test-evidence/cli.md`.
#[test]
fn a_pipe_quoted_argument_reads_the_escapes_improv_does() {
assert_eq!(built(r"create-view |a\|b|"), r#"CreateViewCmd(["a|b"])"#);
assert_eq!(
built(r"create-view |back\\slash|"),
r#"CreateViewCmd(["back\\slash"])"#
);
assert_eq!(
built(r"create-view |two\nlines|"),
r#"CreateViewCmd(["two\nlines"])"#
);
}
/// A coordinate quotes each half, as a `.improv` data line does.
#[test]
fn a_coordinate_may_pipe_quote_either_half() {
assert_eq!(
built("set-cell 5 |Sales Region|/|East Coast| Measure/Revenue"),
r#"SetCellCmd(["5", "Sales Region/East Coast", "Measure/Revenue"])"#
);
}
#[test]
fn a_dot_inside_pipes_does_not_split_the_line() {
assert_eq!(built("create-view |a . b|"), r#"CreateViewCmd(["a . b"])"#);
assert_eq!(
segments("create-view |a . b|"),
Ok(vec!["create-view |a . b|"])
);
}
/// Double quotes stay verbatim: formula text carries its own pipes.
#[test]
fn a_double_quoted_argument_keeps_the_pipes_inside_it() {
assert_eq!(
built(r#"add-formula Measure "Net = |Gross Rev| - Cost""#),
r#"AddFormulaCmd(["Measure", "Net = |Gross Rev| - Cost"])"#
);
}
/// `context/code/cli.md` §14: only a BARE `.` separates commands. A quoted one
/// is an argument, so `|.|` and `"."` name a thing called `.`.
#[test]
fn a_quoted_dot_is_an_argument_not_a_separator() {
for line in ["create-view |.|", "create-view \".\""] {
let groups = groups(line).expect("lexes");
assert_eq!(groups.len(), 1, "`{line}` split into commands");
assert_eq!(texts(&groups[0]), ["create-view", "."], "`{line}`");
}
assert_eq!(groups("create-view A . create-view B").unwrap().len(), 2);
}
#[test]
fn an_unterminated_pipe_is_refused() {
let err = parse_line("create-view |By area").expect_err("an open pipe is refused");
assert!(
err.contains("|By area"),
"the refusal names the text: {err}"
);
}
/// Evidence: `context/test-evidence/cli.md`.
#[test]
fn a_command_given_more_arguments_than_it_takes_is_refused() {
let err = parse_line("create-view By area").expect_err("two words for one argument");
assert!(err.starts_with("create-view:"), "names the command: {err}");
assert!(err.contains("\"area\""), "names the extra argument: {err}");
let err = parse_line("set-axis Type row sideways").expect_err("a third argument");
assert!(
err.contains("\"sideways\""),
"names the extra argument: {err}"
);
assert!(
parse_line("create-view |By area|").is_ok(),
"the control parses"
);
}
}
+176 -10
View File
@@ -24,7 +24,7 @@ use crate::persistence;
use crate::ui::effect::{EffectKind, ModelChange, ViewTarget};
use crate::ui::project::Dirty;
use crate::ui::viewport::{
compute_col_widths, compute_row_header_width, compute_visible_cols, grid_width,
GridSummary, compute_row_header_width, compute_visible_cols, grid_summary, grid_width,
};
use crate::view::{GridLayout, View};
use crate::workbook::Workbook;
@@ -434,6 +434,20 @@ impl Default for ModelState {
}
}
/// What a layout was built from other than the model.
#[derive(Debug, Default, PartialEq, Eq)]
struct LayoutBasis {
view: String,
/// The address of the frozen records, never dereferenced.
drill: Option<usize>,
}
/// What the keypress path measures of a built layout.
pub(crate) struct GridGeometry {
pub summary: GridSummary,
pub row_header_width: u16,
}
/// Top-level application state: `model_state`, `view_state`, and a residue of
/// fields tagged below. The five residue tags are in `context/code/command.md`.
pub struct App {
@@ -453,8 +467,21 @@ pub struct App {
pub term_width: u16,
pub term_height: u16,
/// **Derived cache**: a pure function of the slices, rebuilt by
/// `rebuild_layout()`. Owned by `App` because the rebuild is the host's job.
/// `rebuild_layout()`; a writer that bypasses effects must call it (felo.13).
pub layout: GridLayout,
/// How many times `rebuild_layout` has run: what a keypress that changes
/// neither the model nor the view must leave alone (improvise-felo.13).
layout_builds: usize,
/// How many times the grid's column widths were measured, for the same reason.
width_passes: std::cell::Cell<usize>,
/// The widths of the layout above, measured on first use and dropped with it.
geometry: std::cell::OnceCell<GridGeometry>,
/// What the layout above was built from besides the model: the view it drew
/// and the drill snapshot it froze. `layout_current` compares it.
layout_basis: LayoutBasis,
/// The model changed since the layout was built. Set by the one writer that
/// does not rebuild at once.
layout_stale: bool,
/// **Transient**: set during `apply_effects` to skip the rest of the batch,
/// reset at the start of every call. Reach it through `AbortChain`.
pub abort_effects: bool,
@@ -553,6 +580,11 @@ impl App {
term_width: 80,
term_height: 24,
layout,
layout_builds: 0,
width_passes: std::cell::Cell::new(0),
geometry: std::cell::OnceCell::new(),
layout_basis: LayoutBasis::default(),
layout_stale: false,
abort_effects: false,
batch_aborted: false,
batch_refused: false,
@@ -566,6 +598,7 @@ impl App {
keymap_set: KeymapSet::default_keymaps(),
storage,
};
app.layout_basis = app.current_layout_basis();
app.settle_normal_mode();
app
}
@@ -610,6 +643,7 @@ impl App {
self.view_state.view_back_stack.clear();
self.view_state.view_forward_stack.clear();
self.model_state.workbook = workbook;
self.layout_stale = true;
// Every client's mirror is scoped to the outgoing document too, and an
// effect cannot reach the session list. The generation is how it hears.
self.document_generation += 1;
@@ -900,6 +934,42 @@ impl App {
let frozen = self.drill_state.as_ref().map(|s| Rc::clone(&s.records));
self.layout =
GridLayout::with_frozen_records(&self.model_state.workbook.model, view, frozen);
self.layout_builds += 1;
self.layout_stale = false;
self.layout_basis = self.current_layout_basis();
self.geometry = std::cell::OnceCell::new();
}
/// Rebuild only if the model, the view drawn or the drill snapshot moved; a
/// cursor key moves none of them (layout-cost.md).
fn refresh_layout(&mut self) {
if self.layout_stale || self.layout_basis != self.current_layout_basis() {
self.rebuild_layout();
}
}
fn current_layout_basis(&self) -> LayoutBasis {
LayoutBasis {
view: self.view_name().to_string(),
drill: self
.drill_state
.as_ref()
.map(|d| Rc::as_ptr(&d.records) as usize),
}
}
/// The measurements a keypress needs of the grid: column widths and the row
/// header. Counted, so a test can see whether a keypress paid for them.
pub(crate) fn grid_geometry(&self) -> &GridGeometry {
self.geometry.get_or_init(|| {
self.width_passes.set(self.width_passes.get() + 1);
let model = &self.model_state.workbook.model;
let plan = FormatPlan::of(model, self.view());
GridGeometry {
summary: grid_summary(model, &self.layout, &plan),
row_header_width: compute_row_header_width(&self.layout),
}
})
}
/// Editable text for the cell under the cursor: staged drill edit, else the
@@ -980,12 +1050,10 @@ impl App {
display_value: self.display_value_at_cursor(),
visible_rows: (self.term_height as usize).saturating_sub(8),
visible_cols: {
let model = &self.model_state.workbook.model;
let col_widths = compute_col_widths(model, layout, &FormatPlan::of(model, view));
let row_header_width = compute_row_header_width(layout);
let geometry = self.grid_geometry();
compute_visible_cols(
&col_widths,
row_header_width,
&geometry.summary.col_widths,
geometry.row_header_width,
self.grid_width(),
cursor.col_offset,
)
@@ -1116,7 +1184,8 @@ impl App {
// is read only behind the epoch gate — `context/code/command.md`.
let reshaped = self.last_reshaped_views.clone();
self.last_reframed_views.extend(reshaped);
self.rebuild_layout();
self.layout_stale |= touches_model;
self.refresh_layout();
// After the rebuild, or a batch that crosses into or out of records mode
// settles against the grid it started on — `context/code/command.md`.
self.settle_normal_mode();
@@ -1196,7 +1265,7 @@ impl App {
/// Resolve one keypress through the keymap and apply what it produces. The
/// projection half is factored out; `ui::reduce` is the named door (improvise-gxi).
pub fn dispatch_key(&mut self, key: KeyEvent) -> Dirty {
self.rebuild_layout();
self.refresh_layout();
// An armed prefix key takes priority. Resolve it back to its
// sub-keymap through the keymap set (improvise-jc8).
@@ -1594,6 +1663,32 @@ mod tests {
app
}
/// improvise-felo.13: `StartDrill` is `ViewOnly`, so a batch of it alone moves
/// the layout with no `Model` effect to mark it stale. Evidence: layout-cost.md.
#[test]
fn a_drill_snapshot_replaced_alone_rebuilds_the_layout() {
let mut app = records_app_for_drill();
assert_eq!(app.layout.row_count(), 3);
let rows: Vec<_> = app.layout.records.as_ref().unwrap().to_vec();
let start = |app: &mut App, n: usize| {
let snapshot = rows.iter().take(n).cloned().collect();
app.apply_effects(vec![Box::new(crate::ui::effect::StartDrill(snapshot))]);
};
start(&mut app, 1);
assert_eq!(
app.layout.row_count(),
1,
"the layout draws the frozen snapshot"
);
let builds = app.layout_builds;
start(&mut app, 2);
assert!(
app.layout_builds > builds,
"no rebuild for a second drill snapshot"
);
assert_eq!(app.layout.row_count(), 2);
}
/// The opening basis may be read off `App::layout` only when no drill froze
/// it. Evidence: `context/test-evidence/command.md`.
#[test]
@@ -1898,6 +1993,71 @@ mod tests {
App::new(wb, None)
}
/// improvise-felo.13: context/test-evidence/layout-cost.md.
#[test]
fn a_cursor_key_rebuilds_neither_the_layout_nor_the_widths() {
let mut app = two_col_model();
let down = || KeyEvent::new(KeyCode::Down, KeyModifiers::NONE);
app.dispatch_key(down());
let (builds, widths) = (app.layout_builds, app.width_passes.get());
app.dispatch_key(down());
app.dispatch_key(down());
assert_eq!(app.cursor().selected.0, 2, "the keys moved the cursor");
assert_eq!(app.layout_builds, builds, "layout rebuilt on a cursor key");
assert_eq!(app.width_passes.get(), widths, "widths remeasured");
}
/// improvise-felo.13: the control for the test above, and the guard on
/// invalidation: an edit must rebuild, and the widths must follow the value.
#[test]
fn an_edit_rebuilds_and_remeasures_so_the_widths_follow_the_value() {
use crate::ui::effect::SetCell;
let mut app = two_col_model();
let down = || KeyEvent::new(KeyCode::Down, KeyModifiers::NONE);
app.dispatch_key(down());
let narrow = app.grid_geometry().summary.col_widths.clone();
let (builds, widths) = (app.layout_builds, app.width_passes.get());
let key = CellKey::new(vec![("Row".into(), "A".into()), ("Col".into(), "X".into())]);
let long = CellValue::Text("a value far wider than a column".into());
app.apply_effects(vec![Box::new(SetCell(key, long))]);
assert!(
app.layout_builds > builds,
"an edit must rebuild the layout"
);
app.dispatch_key(down());
assert!(app.width_passes.get() > widths, "an edit must remeasure");
let model = &app.model_state.workbook.model;
let fresh = crate::ui::viewport::compute_col_widths(
model,
&app.layout,
&FormatPlan::of(model, app.view()),
);
assert_eq!(
app.grid_geometry().summary.col_widths,
fresh,
"stale widths"
);
assert_ne!(
fresh, narrow,
"the edit widened a column, or this proves nothing"
);
}
/// improvise-felo.13: a `ViewOnly` effect that changes which view is drawn
/// must still rebuild, so skipping the rebuild is not keyed on `EffectKind` alone.
#[test]
fn switching_view_rebuilds_though_the_effect_is_view_only() {
let mut app = two_col_model();
app.model_state.workbook.create_view("Second");
app.rebuild_layout();
let builds = app.layout_builds;
app.apply_effects(vec![Box::new(crate::ui::effect::SwitchView(
"Second".into(),
))]);
assert_eq!(app.view_state.active_view.as_str(), "Second");
assert!(app.layout_builds > builds, "the layout still draws Default");
}
/// improvise-htl6. Evidence: `context/test-evidence/command.md`.
#[test]
fn the_view_history_walks_both_ways_from_the_keyboard() {
@@ -2134,6 +2294,7 @@ mod tests {
.unwrap()
.add_item(format!("R{i}"));
}
app.rebuild_layout();
app.term_height = 28; // ~20 visible rows → delta = 15
app.cursor_mut().selected = (0, 0);
app.handle_key(KeyEvent::new(KeyCode::PageDown, KeyModifiers::NONE))
@@ -2158,6 +2319,7 @@ mod tests {
.unwrap()
.add_item(format!("R{i}"));
}
app.rebuild_layout();
app.term_height = 28;
app.cursor_mut().selected = (20, 0);
app.handle_key(KeyEvent::new(KeyCode::PageUp, KeyModifiers::NONE))
@@ -2177,6 +2339,7 @@ mod tests {
.unwrap()
.add_item(format!("R{i}"));
}
app.rebuild_layout();
app.term_height = 13; // ~5 visible rows
app.cursor_mut().selected = (0, 0);
// G jumps to last row (row 12)
@@ -2204,6 +2367,7 @@ mod tests {
.unwrap()
.add_item(format!("R{i}"));
}
app.rebuild_layout();
app.term_height = 13; // ~5 visible rows
app.cursor_mut().selected = (0, 0);
// Ctrl+d scrolls by 5 rows
@@ -3298,7 +3462,9 @@ mod tests {
/// when any segment refused, which is what its exit code reads.
fn run_line(app: &mut App, line: &str) -> bool {
let mut refused = false;
for segment in crate::command::parse::segments(line) {
let segments = crate::command::parse::segments(line)
.unwrap_or_else(|e| panic!("`{line}` must lex: {e}"));
for segment in segments {
let effects = {
let ctx = app.cmd_context(KeyCode::Null, KeyModifiers::NONE);
crate::command::parse::parse_segment_interactive(segment, &ctx)
+80 -32
View File
@@ -1053,6 +1053,7 @@ pub enum WizardOp {
ToggleProposal,
CycleProposalKind,
ToggleDateComponent,
ToggleDateShape,
StartFormulaEdit,
DeleteFormula,
ConfirmFormula,
@@ -1078,6 +1079,7 @@ impl WizardOp {
Self::ToggleProposal => wizard.toggle_proposal(),
Self::CycleProposalKind => wizard.cycle_proposal_kind(),
Self::ToggleDateComponent => wizard.toggle_date_component(),
Self::ToggleDateShape => wizard.toggle_date_shape(),
Self::StartFormulaEdit => wizard.start_formula_edit(),
Self::DeleteFormula => wizard.delete_formula(),
Self::ConfirmFormula => wizard.confirm_formula(),
@@ -1245,7 +1247,7 @@ pub struct ImportJsonHeadless {
}
impl Effect for ImportJsonHeadless {
fn apply(&self, app: &mut App) {
use crate::import::analyzer::{FieldKind, analyze_records};
use crate::import::analyzer::FieldKind;
use crate::import::wizard::{DuplicatePolicy, ImportPipeline};
let is_csv = self
@@ -1253,10 +1255,10 @@ impl Effect for ImportJsonHeadless {
.extension()
.is_some_and(|ext| ext.eq_ignore_ascii_case("csv"));
// Parse the file exactly once: `records` feeds analysis, `raw` is reused
// as the pipeline's value (improvise-oaq). CSV warnings go to the status.
// Parse the file exactly once, into the pipeline that reads it
// (improvise-oaq, improvise-8jk6.4). CSV warnings go to the status.
let mut warnings: Vec<String> = vec![];
let (records, raw) = if is_csv {
let mut pipeline = if is_csv {
let bytes = match app.storage.read(&self.path) {
Ok(b) => b,
Err(e) => {
@@ -1267,8 +1269,7 @@ impl Effect for ImportJsonHeadless {
match crate::import::csv_parser::parse_csv_bytes_with_warnings(&bytes) {
Ok((recs, w)) => {
warnings = w;
let raw = serde_json::Value::Array(recs.clone());
(recs, raw)
ImportPipeline::from_csv(recs)
}
Err(e) => {
refuse_and_stop(app, format!("CSV error: {e}"));
@@ -1291,38 +1292,28 @@ impl Effect for ImportJsonHeadless {
}
};
match json_import_records(&value, self.array_path.as_deref()) {
Ok(recs) => (recs, value),
Ok(recs) => {
let mut pipeline = ImportPipeline::new(value);
pipeline.selected_path = self.array_path.clone().unwrap_or_default();
pipeline.records = recs;
pipeline.analyze();
pipeline
}
Err(msg) => {
refuse_and_stop(app, msg);
return;
}
}
};
let proposals = analyze_records(&records);
let pipeline = ImportPipeline {
raw,
array_paths: vec![],
selected_path: self.array_path.as_deref().unwrap_or("").to_string(),
records,
proposals: proposals
.into_iter()
.map(|mut p| {
p.accepted = p.kind != FieldKind::Label;
p
})
.collect(),
model_name: self
.model_name
.as_deref()
.unwrap_or("Imported Model")
.to_string(),
formulas: vec![],
// Headless: nobody to ask, so a collision refuses (improvise-cgqt).
on_duplicate: DuplicatePolicy::Refuse,
sources: vec![],
};
pipeline.model_name = self
.model_name
.clone()
.unwrap_or_else(|| "Imported Model".to_string());
// Headless: nobody to ask, so a collision refuses (improvise-cgqt).
pipeline.on_duplicate = DuplicatePolicy::Refuse;
for p in &mut pipeline.proposals {
p.accepted = p.kind != FieldKind::Label;
}
match pipeline.build_model_with_warnings() {
Ok((new_workbook, build_warnings)) => {
@@ -1379,6 +1370,63 @@ impl Effect for SetPanelCursor {
// ── Chain control ────────────────────────────────────────────────────────────
/// What a command's effects name that the document must have.
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum Needs {
Category(String),
Item(String, String),
Group(String, String),
View(String),
}
/// Why the document cannot satisfy `needs` in the active view, if it cannot.
fn unknown_name(app: &App, needs: &Needs) -> Option<String> {
let workbook = &app.model_state.workbook;
let (category, rest) = match needs {
Needs::View(name) => {
return (!workbook.views.contains_key(name))
.then(|| format!("No view named \"{name}\""));
}
Needs::Category(c) => (c, None),
Needs::Item(c, i) => (c, Some((i, false))),
Needs::Group(c, g) => (c, Some((g, true))),
};
let view = workbook.views.get(app.view_name())?;
if view.try_axis_of(category).is_none() {
return Some(format!("Unknown category '{category}'"));
}
let (name, is_group) = rest?;
if !is_group {
// `effective_item_names`, so a formula target is a known `_Measure` item.
let known = workbook
.model
.effective_item_names(category)
.iter()
.any(|i| i == name);
return (!known).then(|| format!("Unknown item '{name}' in '{category}'"));
}
// An item's own group registers no `Group`; a parent group has no items.
let known = workbook.model.category(category).is_some_and(|c| {
!c.items_in_group(name).is_empty() || c.groups.iter().any(|g| g.name == *name)
});
(!known).then(|| format!("Unknown group '{name}' in '{category}'"))
}
/// Refuse and stop unless the document has what the next effects name, asked at
/// apply time so a `:` line sees its earlier segments (`cli.md` §15).
#[derive(Debug)]
pub struct RequireKnown(pub Needs);
impl Effect for RequireKnown {
fn apply(&self, app: &mut App) {
if let Some(why) = unknown_name(app, &self.0) {
refuse_and_stop(app, why);
}
}
fn kind(&self) -> EffectKind {
EffectKind::ViewOnly
}
}
/// Skip the remaining effects in this `apply_effects` batch, for when a
/// sequence's premise no longer holds. The flag resets per dispatch.
#[derive(Debug)]
+76 -5
View File
@@ -11,7 +11,7 @@ use improvise_core::model::record::{AggregateHash, RecordHash};
use improvise_core::view::layout::{AxisEntry, GridLayout, RecordCol};
use improvise_core::view::{Axis, View};
use improvise_core::workbook::Workbook;
use improvise_io::import::analyzer::{DateComponent, FieldKind};
use improvise_io::import::analyzer::{DateComponent, DateShape, FieldKind};
use improvise_io::import::wizard::{DuplicatePolicy, ImportWizard, WizardStep};
use improvise_protocol::{
AxisFrame, Basis, BasisCache, CategoryRow, CategoryTreeCache, CategoryTreeUpdate, CellDisplay,
@@ -25,7 +25,7 @@ use improvise_protocol::{
use crate::ui::app::App;
use crate::ui::effect::{EffectKind, ModelChange};
use crate::ui::reduce::{Checked, apply_command};
use crate::ui::viewport::{GridBody, grid_body};
use crate::ui::viewport::{GridBody, grid_body, grid_window};
/// Which widgets a batch of effects could have changed. A gate, not a description:
/// over-reporting costs work, under-reporting is a correctness bug (gotcha 35).
@@ -142,9 +142,19 @@ pub fn grid_in(app: &App, view_name: &str, viewport: Viewport) -> GridCache {
let plan = FormatPlan::of(model, view);
let rows = viewport.rows_in(layout.row_count());
let cols = viewport.cols_in(layout.col_count());
// ONE walk: the widths, the totals and this window's values all come off the
// same evaluation — `context/code/command.md` (improvise-jmjz).
let body = grid_body(model, layout, &plan, rows.clone(), cols.clone());
// One walk gives widths, totals and window (improvise-jmjz); the host's layout
// keeps the first two until rebuilt, so its window costs its own cells.
let hosted = elsewhere.is_none() && derived.is_none();
let body = match hosted {
true => grid_window(
model,
layout,
&app.grid_geometry().summary,
rows.clone(),
cols.clone(),
),
false => grid_body(model, layout, &plan, rows.clone(), cols.clone()),
};
GridCache {
row_frame: axis_frame(&layout.row_cats, &layout.row_items, rows.clone()),
@@ -652,6 +662,7 @@ pub fn wizard_cache(w: &ImportWizard) -> WizardCache {
((*label).to_string(), p.date_components.contains(component))
})
.collect(),
shape: date_shape_sentence(p),
})
.collect(),
collisions: w
@@ -690,6 +701,27 @@ pub fn wizard_cache(w: &ImportWizard) -> WizardCache {
}
}
/// What a date field's chosen components become, worded for every front end.
fn date_shape_sentence(p: &improvise_io::import::analyzer::FieldProposal) -> String {
let chosen = |order: &[DateComponent]| -> Vec<&str> {
order
.iter()
.filter(|c| p.date_components.contains(c))
.filter_map(|c| DATE_COMPONENTS.iter().find(|(d, _)| d == c))
.map(|(_, label)| *label)
.collect()
};
match (p.date_shape, chosen(&DateComponent::COARSE_TO_FINE)) {
(_, none) if none.is_empty() => "one category of the raw dates".to_string(),
(DateShape::Hierarchy, levels) => format!("one category: {}", levels.join(" > ")),
(DateShape::Split, _) => {
let parts = chosen(&DATE_COMPONENTS.map(|(c, _)| c));
let names: Vec<String> = parts.iter().map(|l| format!("{}_{l}", p.field)).collect();
format!("split: {}, {}", p.field, names.join(", "))
}
}
}
/// The date components the wizard offers, **in the order it offers them**:
/// `ImportWizard::date_config_at_cursor` counts these same rows.
const DATE_COMPONENTS: [(DateComponent, &str); 3] = [
@@ -6512,3 +6544,42 @@ mod column_totals {
);
}
}
/// improvise-8jk6.3: the wizard's date step says what each date field becomes,
/// and `s` switches it between one category and the split.
#[cfg(test)]
mod date_shape {
use super::wizard_cache;
use crate::import::wizard::{ImportWizard, WizardStep};
use serde_json::json;
#[test]
fn the_date_step_says_what_a_date_field_becomes_and_s_switches_it() {
let mut w = ImportWizard::new(json!([
{"Region": "East", "When": "2024-01-15", "Revenue": 1.0},
{"Region": "West", "When": "2024-04-02", "Revenue": 2.0},
]));
w.advance();
assert_eq!(w.step, WizardStep::ConfigureDates, "setup");
let shape = |w: &ImportWizard| wizard_cache(w).date_fields[0].shape.clone();
assert_eq!(shape(&w), "one category of the raw dates");
w.toggle_date_component();
w.move_cursor(1);
w.toggle_date_component();
w.move_cursor(1);
w.toggle_date_component();
assert_eq!(shape(&w), "one category: Year > Quarter > Month");
w.toggle_date_shape();
assert_eq!(
shape(&w),
"split: When, When_Year, When_Month, When_Quarter"
);
let built = w.pipeline.build_model().unwrap();
assert!(built.model.category("When_Quarter").is_some());
w.toggle_date_shape();
let built = w.pipeline.build_model().unwrap();
assert!(built.model.category("When_Quarter").is_none());
let when = built.model.category("When").unwrap();
assert_eq!(when.item_chain("2024-04"), vec!["2024", "2024-Q2"]);
}
}
@@ -63,6 +63,10 @@ const NOT_ON_THE_WIRE: &[(&str, Verdict)] = &[
"refreshes the host's layout cache mid-batch; a client holds none (gotcha 23)",
),
),
(
"RequireKnown",
Verdict::Off("a command-side name check that writes no state (gotcha 21, cli.md §15)"),
),
(
"AbortChain",
Verdict::Off("control flow inside one apply_effects batch; it writes no state (gotcha 21)"),
+74 -1
View File
@@ -101,6 +101,50 @@ pub fn grid_body(
}
}
/// What of a grid build no window changes: the widths, each column's total
/// format and the totals. Measured over every cell, so a host keeps one per layout.
pub struct GridSummary {
pub col_widths: Vec<u16>,
pub col_formats: Vec<NumberFormat>,
pub totals: Vec<Option<CellValue>>,
}
/// The whole-grid walk, answering only what a window cannot.
pub fn grid_summary(model: &Model, layout: &GridLayout, plan: &FormatPlan) -> GridSummary {
let body = grid_body(model, layout, plan, 0..0, 0..0);
GridSummary {
col_widths: body.col_widths,
col_formats: body.col_formats,
totals: body.totals,
}
}
/// [`grid_body`] for a window, given the `summary` of the same layout: evaluates
/// only the window's cells. Equal to `grid_body` where `summary` is current.
pub fn grid_window(
model: &Model,
layout: &GridLayout,
summary: &GridSummary,
rows: std::ops::Range<usize>,
cols: std::ops::Range<usize>,
) -> GridBody {
let eval = layout.evaluator(model);
let records = layout.is_records_mode();
let window = rows
.map(|ri| {
cols.clone()
.map(|ci| cell_value(layout, &eval, records, ri, ci).1)
.collect()
})
.collect();
GridBody {
col_widths: summary.col_widths.clone(),
col_formats: summary.col_formats.clone(),
totals: summary.totals.clone(),
window,
}
}
/// The one evaluation. Records mode reads its wide row; pivot mode aggregates.
fn cell_value(
layout: &GridLayout,
@@ -146,7 +190,7 @@ fn widen(widths: &mut [u16], col: usize, width: usize) {
/// Adaptive column widths alone, for the keypress path, which needs no cells.
/// It is [`grid_body`] with an empty window so there is one width rule.
pub fn compute_col_widths(model: &Model, layout: &GridLayout, plan: &FormatPlan) -> Vec<u16> {
grid_body(model, layout, plan, 0..0, 0..0).col_widths
grid_summary(model, layout, plan).col_widths
}
/// The total row-header width for a layout. The rule is the protocol's; this is the
@@ -272,4 +316,33 @@ mod tests {
"F0..F8 once for the Q1 row, once for the totals"
);
}
/// improvise-felo.13: a window read off a held summary is the window `grid_body`
/// builds. Evidence: `context/test-evidence/layout-cost.md`.
#[test]
fn a_window_over_a_held_summary_equals_the_one_walk_grid_body() {
let wb = folded_chain();
let view = wb.active_view();
let layout = GridLayout::new(&wb.model, view);
let plan = FormatPlan::of(&wb.model, view);
let summary = grid_summary(&wb.model, &layout, &plan);
let (rows, cols) = (layout.row_count(), layout.col_count());
assert!(
rows >= 2 && cols >= 3,
"a window strictly inside is possible"
);
for (r, c) in [
(0..rows, 0..cols),
(1..2, 1..3),
(rows - 1..rows, 0..1),
(0..0, 0..0),
] {
let walked = grid_body(&wb.model, &layout, &plan, r.clone(), c.clone());
let held = grid_window(&wb.model, &layout, &summary, r.clone(), c.clone());
assert_eq!(held.window, walked.window, "window {r:?} x {c:?}");
assert_eq!(held.col_widths, walked.col_widths);
assert_eq!(held.totals, walked.totals);
assert_eq!(held.col_formats, walked.col_formats);
}
}
}
+141
View File
@@ -194,6 +194,12 @@ impl RecordEntry {
}
}
#[cfg(test)]
thread_local! {
/// How many lookups fell through to set narrowing, for the exact-path test.
static NARROWED: std::cell::Cell<usize> = const { std::cell::Cell::new(0) };
}
#[derive(Clone, Default)]
pub struct DataStore {
/// Primary storage: interned keys, insertion-ordered so records mode can
@@ -206,6 +212,9 @@ pub struct DataStore {
/// Reverse index for the client-facing surrogate (ADR 002), plus each
/// record's write counter. Why it lives here: core.md §8.
records: HashMap<RecordHash, RecordEntry>,
/// How many stored cells have each number of coordinates; its largest key
/// says when a partial can match only one exact cell.
arities: std::collections::BTreeMap<usize, usize>,
}
/// Resolved through the symbol table and sorted wherever the storage is a
@@ -217,6 +226,7 @@ impl std::fmt::Debug for DataStore {
symbols,
index,
records,
arities: _,
} = self;
let cells: Vec<_> = cells
.iter()
@@ -291,7 +301,11 @@ impl DataStore {
for pair in &ikey.0 {
self.index.entry(*pair).or_default().insert(ikey.clone());
}
let width = ikey.0.len();
let is_new_cell = self.cells.insert(ikey, value).is_none();
if is_new_cell {
*self.arities.entry(width).or_default() += 1;
}
let entry = self.records.entry(hash).or_insert(RecordEntry {
record,
cells: 0,
@@ -327,6 +341,12 @@ impl DataStore {
return;
};
if self.cells.shift_remove(&ikey).is_some() {
if let Some(n) = self.arities.get_mut(&ikey.0.len()) {
*n -= 1;
if *n == 0 {
self.arities.remove(&ikey.0.len());
}
}
for pair in &ikey.0 {
if let Some(set) = self.index.get_mut(pair) {
set.remove(&ikey);
@@ -378,11 +398,28 @@ impl DataStore {
/// Positions in `cells` of every cell matching `partial`, **in insertion
/// order**. The index narrows; the primary map orders (§8).
fn matching_positions(&self, partial: &[(String, String)]) -> Vec<usize> {
if let Some(exact) = self.exact_positions(partial) {
return exact;
}
self.intern_partial(partial)
.map(|pairs| self.positions_of(&pairs))
.unwrap_or_default()
}
/// The answer without narrowing, when `partial` is as wide as the widest cell
/// stored: only an equal cell can match (layout-cost.md). `None` otherwise.
fn exact_positions(&self, partial: &[(String, String)]) -> Option<Vec<usize>> {
let widest = *self.arities.keys().next_back()?;
// Strictly sorted by category, the order a stored key is in, so one probe is the lookup.
let canonical = partial.windows(2).all(|w| w[0].0 < w[1].0);
(!partial.is_empty() && widest <= partial.len() && canonical).then(|| {
let found = self
.intern_partial(partial)
.and_then(|pairs| self.cells.get_index_of(&InternedKey(pairs)));
found.into_iter().collect()
})
}
/// `partial` as symbols, or `None` when a name was never interned, which
/// no stored cell can then match.
fn intern_partial(&self, partial: &[(String, String)]) -> Option<Vec<(Symbol, Symbol)>> {
@@ -412,6 +449,8 @@ impl DataStore {
}
sets.sort_by_key(|s| s.len());
#[cfg(test)]
NARROWED.with(|n| n.set(n.get() + 1));
let mut positions: Vec<usize> = match sets.split_first() {
Some((first, rest)) => first
@@ -1356,3 +1395,105 @@ mod record_index {
}
}
}
#[cfg(test)]
mod exact_path_tests {
use super::{CellKey, CellValue, DataStore, NARROWED};
fn key(a: &str, b: &str, c: &str) -> CellKey {
CellKey::new(vec![
("A".into(), a.into()),
("B".into(), b.into()),
("C".into(), c.into()),
])
}
fn store() -> DataStore {
let mut d = DataStore::new();
for i in 0..6 {
for j in 0..6 {
d.set(
key(&format!("a{i}"), &format!("b{j}"), "c"),
CellValue::Number((i * 6 + j) as f64),
);
}
}
d
}
fn narrowings(f: impl FnOnce()) -> usize {
NARROWED.with(|n| n.set(0));
f();
NARROWED.with(|n| n.get())
}
/// improvise-felo.13: context/test-evidence/layout-cost.md.
#[test]
fn a_full_width_partial_probes_one_cell_instead_of_narrowing() {
let d = store();
let full = key("a2", "b3", "c").0;
let mut got = Vec::new();
assert_eq!(
narrowings(|| got = d.matching_values(&full).into_iter().cloned().collect()),
0
);
assert_eq!(got, vec![CellValue::Number(15.0)]);
let missing = key("a2", "b9", "c").0;
assert!(
d.matching_values(&missing).is_empty(),
"an absent cell matches nothing"
);
let unknown = key("a2", "b3", "zzz").0;
assert!(
d.matching_values(&unknown).is_empty(),
"an unseen item matches nothing"
);
}
/// The control: a narrower partial still sums its supersets, so the shortcut
/// is not a blanket exact lookup.
#[test]
fn a_narrower_partial_still_narrows_and_matches_every_superset() {
let d = store();
let narrow = vec![
("A".to_string(), "a2".to_string()),
("C".to_string(), "c".to_string()),
];
let mut n = 0;
assert_eq!(narrowings(|| n = d.matching_values(&narrow).len()), 1);
assert_eq!(n, 6);
}
/// A wider cell appearing, or leaving, moves the line the shortcut is allowed at.
#[test]
fn a_wider_cell_turns_the_shortcut_off_until_it_is_removed() {
let mut d = store();
let wide = CellKey::new(vec![
("A".into(), "a2".into()),
("B".into(), "b3".into()),
("C".into(), "c".into()),
("D".into(), "d".into()),
]);
d.set(wide.clone(), CellValue::Number(100.0));
let full = key("a2", "b3", "c").0;
assert_eq!(
d.matching_values(&full).len(),
2,
"the exact cell and the wider cell under it"
);
d.remove(&wide);
assert_eq!(
narrowings(|| assert_eq!(d.matching_values(&full).len(), 1)),
0
);
}
/// An unsorted partial is not a stored key's order, so it must not be probed.
#[test]
fn an_unsorted_full_width_partial_is_still_answered() {
let d = store();
let mut full = key("a2", "b3", "c").0;
full.reverse();
assert_eq!(d.matching_values(&full).len(), 1);
}
}
+236 -34
View File
@@ -235,23 +235,47 @@ impl AxisEntry {
/// The data-item tuples on an axis, skipping group headers. Every public
/// `row`/`col` argument indexes this sequence, not the visual entry list.
#[cfg(test)]
fn data_items(items: &[AxisEntry]) -> impl Iterator<Item = &Vec<String>> {
items.iter().filter_map(|e| match e {
AxisEntry::DataItem { tuple, .. } => Some(tuple),
AxisEntry::GroupHeader { .. } | AxisEntry::GroupEnd { .. } => None,
})
}
/// Whether the data item at `idx` folds a collapsed group.
fn folds(items: &[AxisEntry], idx: usize) -> bool {
items
.iter()
.filter_map(|e| match e {
AxisEntry::DataItem { collapsed, .. } => Some(*collapsed),
AxisEntry::DataItem { tuple, .. } => Some(tuple),
AxisEntry::GroupHeader { .. } | AxisEntry::GroupEnd { .. } => None,
})
.nth(idx)
.unwrap_or(false)
.inspect(|_| DATA_ITEM_STEPS.with(|n| n.set(n.get() + 1)))
}
#[cfg(test)]
thread_local! {
/// Items `data_items` has yielded on this thread: the work a lookup did.
static DATA_ITEM_STEPS: std::cell::Cell<usize> = const { std::cell::Cell::new(0) };
}
/// Entry `positions(items)[n]` is data item `n`: a lookup by data index is O(1),
/// not a walk past every header before it (layout-cost.md).
fn positions(items: &[AxisEntry]) -> Vec<usize> {
items
.iter()
.enumerate()
.filter_map(|(at, e)| matches!(e, AxisEntry::DataItem { .. }).then_some(at))
.collect()
}
/// The data item at `idx`, by the axis's position index.
fn data_item<'a>(items: &'a [AxisEntry], pos: &[usize], idx: usize) -> Option<&'a Vec<String>> {
match items.get(*pos.get(idx)?)? {
AxisEntry::DataItem { tuple, .. } => Some(tuple),
AxisEntry::GroupHeader { .. } | AxisEntry::GroupEnd { .. } => None,
}
}
/// Whether the data item at `idx` folds a collapsed group.
fn folds(items: &[AxisEntry], pos: &[usize], idx: usize) -> bool {
match pos.get(idx).and_then(|at| items.get(*at)) {
Some(AxisEntry::DataItem { collapsed, .. }) => *collapsed,
Some(AxisEntry::GroupHeader { .. } | AxisEntry::GroupEnd { .. }) | None => false,
}
}
/// The data index of the first item in the run `cat`/`group` opens under
@@ -287,21 +311,15 @@ fn run_start(
}
/// The display label of the data item at `idx`, or empty when out of bounds.
fn axis_label(items: &[AxisEntry], idx: usize) -> String {
data_items(items)
.nth(idx)
fn axis_label(items: &[AxisEntry], pos: &[usize], idx: usize) -> String {
data_item(items, pos, idx)
.map(|v| v.join("/"))
.unwrap_or_default()
}
/// Visual index of the nth data entry (skipping group headers).
fn data_index_to_visual(items: &[AxisEntry], data_idx: usize) -> Option<usize> {
items
.iter()
.enumerate()
.filter(|(_, e)| matches!(e, AxisEntry::DataItem { .. }))
.map(|(vi, _)| vi)
.nth(data_idx)
fn data_index_to_visual(pos: &[usize], data_idx: usize) -> Option<usize> {
pos.get(data_idx).copied()
}
/// Where a run sits: `(level, depth)`, ordered, so "inside" is "greater than".
@@ -468,6 +486,10 @@ pub struct GridLayout {
pub page_coords: Vec<PageAxis>,
pub row_items: Vec<AxisEntry>,
pub col_items: Vec<AxisEntry>,
/// `positions(row_items)` / `positions(col_items)`: derived, so every writer of
/// the items calls `reindex`, and nothing outside this module writes them.
row_pos: Vec<usize>,
col_pos: Vec<usize>,
/// Categories on `Axis::None` — hidden, implicitly aggregated.
pub none_cats: Vec<String>,
/// Records mode: one entry per row, grouping the cells that share every
@@ -497,6 +519,7 @@ impl GridLayout {
.collect();
layout.row_items = row_items;
layout.records = Some(records);
layout.reindex();
}
if view.prune_empty {
layout.prune_empty(model);
@@ -556,6 +579,8 @@ impl GridLayout {
row_cats,
col_cats,
page_coords,
row_pos: positions(&row_items),
col_pos: positions(&col_items),
row_items,
col_items,
none_cats,
@@ -590,6 +615,8 @@ impl GridLayout {
row_cats: vec![INDEX_CATEGORY.to_string()],
col_cats: vec![DIM_CATEGORY.to_string()],
page_coords,
row_pos: positions(&row_items),
col_pos: positions(&col_items),
row_items,
col_items,
none_cats,
@@ -728,6 +755,13 @@ impl GridLayout {
prune_axis_items(&mut self.col_items, |ci| {
(0..rc).any(|ri| has_value[ri][ci])
});
self.reindex();
}
/// Rebuild the position indexes after the items change.
fn reindex(&mut self) {
self.row_pos = positions(&self.row_items);
self.col_pos = positions(&self.col_items);
}
/// Whether this layout is in records mode.
@@ -737,20 +771,20 @@ impl GridLayout {
/// Number of data rows (group headers excluded).
pub fn row_count(&self) -> usize {
data_items(&self.row_items).count()
self.row_pos.len()
}
/// Number of data columns (group headers excluded).
pub fn col_count(&self) -> usize {
data_items(&self.col_items).count()
self.col_pos.len()
}
pub fn row_label(&self, row: usize) -> String {
axis_label(&self.row_items, row)
axis_label(&self.row_items, &self.row_pos, row)
}
pub fn col_label(&self, col: usize) -> String {
axis_label(&self.col_items, col)
axis_label(&self.col_items, &self.col_pos, col)
}
/// The value of a pivot key as THIS grid reads it: aggregated over its None
@@ -804,8 +838,8 @@ impl GridLayout {
(DIM_CATEGORY.to_string(), col.to_string()),
]));
}
let row_item = data_items(&self.row_items).nth(row)?;
let col_item = data_items(&self.col_items).nth(col)?;
let row_item = data_item(&self.row_items, &self.row_pos, row)?;
let col_item = data_item(&self.col_items, &self.col_pos, col)?;
let mut coords = page_constraints(&self.page_coords);
for (cat, item) in self.row_cats.iter().zip(row_item.iter()) {
coords.push((cat.clone(), item.clone()));
@@ -819,7 +853,7 @@ impl GridLayout {
/// The coordinates a COLUMN answers for once its rows are aggregated away:
/// the page/filter constraints and the column's own tuple (core.md §6).
pub fn column_key(&self, col: usize) -> Option<CellKey> {
let col_item = data_items(&self.col_items).nth(col)?;
let col_item = data_item(&self.col_items, &self.col_pos, col)?;
let mut coords = page_constraints(&self.page_coords);
for (cat, item) in self.col_cats.iter().zip(col_item.iter()) {
coords.push((cat.clone(), item.clone()));
@@ -829,12 +863,12 @@ impl GridLayout {
/// Visual index of the nth data row (skipping group headers).
pub fn data_row_to_visual(&self, data_row: usize) -> Option<usize> {
data_index_to_visual(&self.row_items, data_row)
data_index_to_visual(&self.row_pos, data_row)
}
/// Visual index of the nth data column (skipping group headers).
pub fn data_col_to_visual(&self, data_col: usize) -> Option<usize> {
data_index_to_visual(&self.col_items, data_col)
data_index_to_visual(&self.col_pos, data_col)
}
/// The `(cat_name, group_name)` of the innermost group the Nth data row
@@ -846,12 +880,12 @@ impl GridLayout {
/// Whether the cell at `(row, col)` folds a collapsed group: an aggregate
/// with no record under it. The one home of that answer (§14).
pub fn folds_a_group(&self, row: usize, col: usize) -> bool {
self.row_folds_a_group(row) || folds(&self.col_items, col)
self.row_folds_a_group(row) || folds(&self.col_items, &self.col_pos, col)
}
/// The row half of [`Self::folds_a_group`], for a command about the row alone.
pub fn row_folds_a_group(&self, row: usize) -> bool {
folds(&self.row_items, row)
folds(&self.row_items, &self.row_pos, row)
}
/// The first data row of `cat`/`group`'s run under `prefix` — where a toggle
@@ -867,12 +901,12 @@ impl GridLayout {
/// The coordinate tuple of the Nth data row.
pub fn row_tuple(&self, row: usize) -> Option<&[String]> {
data_items(&self.row_items).nth(row).map(Vec::as_slice)
data_item(&self.row_items, &self.row_pos, row).map(Vec::as_slice)
}
/// The coordinate tuple of the Nth data column.
pub fn col_tuple(&self, col: usize) -> Option<&[String]> {
data_items(&self.col_items).nth(col).map(Vec::as_slice)
data_item(&self.col_items, &self.col_pos, col).map(Vec::as_slice)
}
/// The `(cat_name, group_name)` of the innermost group the Nth data
@@ -2970,3 +3004,171 @@ mod hierarchy {
assert_eq!(l.row_items[0], header(0, "2024", false));
}
}
#[cfg(test)]
mod lookup_cost_tests {
use super::{DATA_ITEM_STEPS, GridLayout};
use crate::view::Axis;
use crate::workbook::Workbook;
fn tall(rows: usize) -> GridLayout {
let mut wb = Workbook::new("T");
wb.add_category("Row").unwrap();
wb.add_category("Col").unwrap();
for i in 0..rows {
wb.add_item("Row", format!("r{i}")).unwrap();
}
wb.add_item("Col", "c0").unwrap();
wb.add_item("Col", "c1").unwrap();
wb.active_view_mut().set_axis("Row", Axis::Row);
wb.active_view_mut().set_axis("Col", Axis::Column);
GridLayout::new(&wb.model, wb.active_view())
}
fn steps(f: impl FnOnce()) -> usize {
DATA_ITEM_STEPS.with(|n| n.set(0));
f();
DATA_ITEM_STEPS.with(|n| n.get())
}
/// improvise-felo.13: context/test-evidence/layout-cost.md.
#[test]
fn an_axis_lookup_does_work_independent_of_the_row_index() {
let layout = tall(400);
let last = layout.row_count() - 1;
assert_eq!(last, 399, "the fixture is as tall as the test needs");
let lookups: [(&str, &dyn Fn(usize)); 4] = [
("cell_key", &|r| {
let _ = layout.cell_key(r, 1);
}),
("row_tuple", &|r| {
let _ = layout.row_tuple(r);
}),
("row_label", &|r| {
let _ = layout.row_label(r);
}),
("column_key", &|r| {
let _ = layout.column_key(r % 2);
}),
];
for (name, lookup) in lookups {
let first = steps(|| lookup(0));
let deep = steps(|| lookup(last));
assert!(
deep <= first + 2,
"{name}: row {last} cost {deep} steps against {first} at row 0"
);
}
}
/// improvise-felo.13: the index answers what the walk it replaced did, through
/// groups, a collapsed fold and a pruned axis. Evidence: layout-cost.md.
#[test]
fn the_position_index_answers_what_the_walk_answered() {
use super::{AxisEntry, data_items};
use crate::model::cell::{CellKey, CellValue};
let mut wb = Workbook::new("T");
for cat in ["Row", "Col"] {
wb.add_category(cat).unwrap();
}
for (i, g) in ["Q1", "Q1", "Q2", "Q2", "Q3"].iter().enumerate() {
wb.add_item_in_group("Row", format!("r{i}"), *g).unwrap();
}
wb.add_item("Row", "ghost").unwrap();
wb.add_item("Col", "c0").unwrap();
wb.active_view_mut().set_axis("Row", Axis::Row);
wb.active_view_mut().set_axis("Col", Axis::Column);
wb.active_view_mut().set_expand_depth("Row", 1);
wb.active_view_mut().toggle_group_collapse("Row", &["Q2"]);
wb.active_view_mut().prune_empty = true;
for i in 0..5 {
let key = CellKey::new(vec![
("Row".into(), format!("r{i}")),
("Col".into(), "c0".into()),
]);
wb.model.set_cell(key, CellValue::Number(1.0));
}
let layout = GridLayout::with_frozen_records(&wb.model, wb.active_view(), None);
assert!(
layout
.row_items
.iter()
.any(|e| !matches!(e, AxisEntry::DataItem { .. }))
);
assert!(layout.row_items.iter().any(|e| matches!(
e,
AxisEntry::DataItem {
collapsed: true,
..
}
)));
let walked: Vec<&Vec<String>> = data_items(&layout.row_items).collect();
assert_eq!(layout.row_count(), walked.len());
let visual: Vec<usize> = layout
.row_items
.iter()
.enumerate()
.filter(|(_, e)| matches!(e, AxisEntry::DataItem { .. }))
.map(|(at, _)| at)
.collect();
for i in 0..walked.len() + 2 {
assert_eq!(
layout.row_tuple(i),
walked.get(i).map(|t| t.as_slice()),
"row {i}"
);
assert_eq!(
layout.data_row_to_visual(i),
visual.get(i).copied(),
"visual {i}"
);
let folded = matches!(
data_items_entry(&layout.row_items, i),
Some(AxisEntry::DataItem {
collapsed: true,
..
})
);
assert_eq!(layout.row_folds_a_group(i), folded, "fold {i}");
}
}
/// improvise-felo.13: a drill held open while records are removed elsewhere.
/// Evidence: layout-cost.md.
#[test]
fn a_frozen_records_snapshot_replaces_the_live_row_index() {
use crate::model::cell::{CellKey, CellValue};
let mut wb = Workbook::new("T");
wb.add_category("Region").unwrap();
wb.add_item("_Measure", "Revenue").unwrap();
let cell = |i: usize| {
CellKey::new(vec![
("Region".into(), format!("r{i}")),
("_Measure".into(), "Revenue".into()),
])
};
for i in 0..3 {
wb.add_item("Region", format!("r{i}")).unwrap();
wb.model.set_cell(cell(i), CellValue::Number(1.0));
}
let v = wb.active_view_mut();
v.set_axis("_Index", Axis::Row);
v.set_axis("_Dim", Axis::Column);
let frozen = GridLayout::new(&wb.model, wb.active_view()).records;
wb.model.clear_cell(&cell(1));
wb.model.clear_cell(&cell(2));
let live = GridLayout::with_frozen_records(&wb.model, wb.active_view(), None);
assert_eq!(live.row_count(), 1, "the control: one record is live");
let held = GridLayout::with_frozen_records(&wb.model, wb.active_view(), frozen);
assert_eq!(held.row_count(), 3);
assert_eq!(held.row_tuple(2), Some(&["2".to_string()][..]));
assert!(held.cell_key(2, 0).is_some());
}
fn data_items_entry(items: &[super::AxisEntry], n: usize) -> Option<&super::AxisEntry> {
items
.iter()
.filter(|e| matches!(e, super::AxisEntry::DataItem { .. }))
.nth(n)
}
}
@@ -22,6 +22,26 @@ pub enum DateComponent {
Quarter,
}
impl DateComponent {
/// The components as a hierarchy nests them, coarsest first.
pub const COARSE_TO_FINE: [DateComponent; 3] = [
DateComponent::Year,
DateComponent::Quarter,
DateComponent::Month,
];
}
/// How a date column's components become categories (improvise-8jk6.3).
#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]
pub enum DateShape {
/// ONE category named after the column: its items are the finest chosen
/// component, grouped by the coarser ones. The user's default.
#[default]
Hierarchy,
/// The column's raw values, plus one flat category per chosen component.
Split,
}
#[derive(Debug, Clone)]
pub struct FieldProposal {
pub field: String,
@@ -32,6 +52,8 @@ pub struct FieldProposal {
pub date_format: Option<String>,
/// Which date components to extract as new categories.
pub date_components: Vec<DateComponent>,
/// Whether those components nest in one category or split into several.
pub date_shape: DateShape,
}
impl FieldProposal {
@@ -132,6 +154,7 @@ pub fn analyze_records(records: &[Value]) -> Vec<FieldProposal> {
accepted: true,
date_format: None,
date_components: vec![],
date_shape: DateShape::default(),
};
}
@@ -159,6 +182,7 @@ pub fn analyze_records(records: &[Value]) -> Vec<FieldProposal> {
accepted: true,
date_format,
date_components: vec![],
date_shape: DateShape::default(),
};
}
@@ -170,6 +194,7 @@ pub fn analyze_records(records: &[Value]) -> Vec<FieldProposal> {
accepted: true,
date_format: None,
date_components: vec![],
date_shape: DateShape::default(),
};
}
@@ -180,6 +205,7 @@ pub fn analyze_records(records: &[Value]) -> Vec<FieldProposal> {
accepted: true,
date_format: None,
date_components: vec![],
date_shape: DateShape::default(),
};
}
@@ -191,6 +217,7 @@ pub fn analyze_records(records: &[Value]) -> Vec<FieldProposal> {
accepted: true,
date_format: None,
date_components: vec![],
date_shape: DateShape::default(),
}
})
.collect()
+73 -50
View File
@@ -7,14 +7,32 @@ use serde_json::Value;
use crate::storage::{PhysicalFs, Storage};
/// Parsed CSV records. Every value is its source text, so they reach an import
/// only through `ImportPipeline::from_csv` (improvise-8jk6.4).
#[derive(Debug, Clone, Default, PartialEq)]
pub struct CsvRecords(Vec<Value>);
impl std::ops::Deref for CsvRecords {
type Target = [Value];
fn deref(&self) -> &[Value] {
&self.0
}
}
impl CsvRecords {
pub(crate) fn into_values(self) -> Vec<Value> {
self.0
}
}
pub fn csv_path_p(path: &Path) -> bool {
path.extension()
.is_some_and(|ext| ext.eq_ignore_ascii_case("csv"))
}
/// Parse a CSV file into a `serde_json::Value` array. Short-row warnings go to
/// stderr; [`parse_csv_with_warnings`] captures them instead.
pub fn parse_csv(path: &Path) -> Result<Vec<Value>> {
/// Parse a CSV file into records. Short-row warnings go to stderr;
/// [`parse_csv_with_warnings`] captures them instead.
pub fn parse_csv(path: &Path) -> Result<CsvRecords> {
let (records, warnings) = parse_csv_with_warnings(path)?;
for warning in &warnings {
eprintln!("warning: {warning}");
@@ -24,7 +42,7 @@ pub fn parse_csv(path: &Path) -> Result<Vec<Value>> {
/// Parse a CSV file, returning records plus warnings. A short row keeps every
/// header column, missing ones `Null`, each reported with a row count.
pub fn parse_csv_with_warnings(path: &Path) -> Result<(Vec<Value>, Vec<String>)> {
pub fn parse_csv_with_warnings(path: &Path) -> Result<(CsvRecords, Vec<String>)> {
let bytes = std::fs::read(path)
.with_context(|| format!("Failed to open CSV file: {}", path.display()))?;
parse_csv_bytes_with_warnings(&bytes)
@@ -32,9 +50,9 @@ pub fn parse_csv_with_warnings(path: &Path) -> Result<(Vec<Value>, Vec<String>)>
/// As [`parse_csv_with_warnings`] but over bytes, so import can come from any
/// [`Storage`](crate::storage::Storage) backend, not only a file (improvise-fuj).
pub fn parse_csv_bytes_with_warnings(bytes: &[u8]) -> Result<(Vec<Value>, Vec<String>)> {
pub fn parse_csv_bytes_with_warnings(bytes: &[u8]) -> Result<(CsvRecords, Vec<String>)> {
let (_, records, warnings) = parse_csv_bytes(bytes)?;
Ok((records, warnings))
Ok((CsvRecords(records), warnings))
}
/// The header (after renaming), the records and the warnings.
@@ -152,7 +170,7 @@ fn disambiguate_headers(raw: Vec<String>) -> (Vec<String>, Vec<String>) {
/// Parse multiple CSV files and merge into a single JSON array.
/// Each record gets a "File" field set to the filename stem (e.g., "sales" from "sales.csv").
pub fn merge_csvs(paths: &[impl AsRef<Path>]) -> Result<Vec<Value>> {
pub fn merge_csvs(paths: &[impl AsRef<Path>]) -> Result<CsvRecords> {
let merged = merge_csvs_from(&PhysicalFs, paths)?;
for warning in &merged.warnings {
eprintln!("warning: {warning}");
@@ -173,7 +191,7 @@ pub struct SourceFile {
/// Several CSV files as one record set, with each file's own columns kept.
#[derive(Debug, Clone, PartialEq)]
pub struct Merged {
pub records: Vec<Value>,
pub records: CsvRecords,
/// One per distinct `File` item, in the order the files were read.
pub sources: Vec<SourceFile>,
/// Each file's parse warnings, prefixed with its path.
@@ -184,7 +202,7 @@ pub struct Merged {
/// and two files with one stem share its item and its columns (improvise-lpzu).
pub fn merge_csvs_from(fs: &dyn Storage, paths: &[impl AsRef<Path>]) -> Result<Merged> {
let mut merged = Merged {
records: Vec::new(),
records: CsvRecords::default(),
sources: Vec::new(),
warnings: Vec::new(),
};
@@ -212,7 +230,7 @@ pub fn merge_csvs_from(fs: &dyn Storage, paths: &[impl AsRef<Path>]) -> Result<M
if let Value::Object(ref mut map) = record {
map.insert(FILE_CATEGORY.to_string(), Value::String(stem.clone()));
}
merged.records.push(record);
merged.records.0.push(record);
}
}
Ok(merged)
@@ -266,26 +284,33 @@ fn csv_documents_in(fs: &dyn Storage, dir: &Path) -> Result<Vec<PathBuf>> {
Ok(csvs)
}
/// A field is its source text: which ones are numbers is decided by the role
/// the import gives the column (improvise-8jk6.4, `context/code/formula-and-io.md`).
fn parse_csv_field(field: &str) -> Value {
if field.is_empty() {
return Value::Null;
match field.is_empty() {
true => Value::Null,
false => Value::String(field.to_string()),
}
}
// Try to parse as number (integer or float)
if let Ok(num) = field.parse::<i64>() {
return Value::Number(serde_json::Number::from(num));
/// The number a CSV field's text reads as, where a number is wanted.
pub fn number_in(text: &str) -> Option<serde_json::Number> {
if let Ok(num) = text.parse::<i64>() {
return Some(serde_json::Number::from(num));
}
// NaN and the infinities have no JSON number. A fabricated 0 is an invented
// measurement (improvise-jc95), so the token falls through and stays text.
if let Ok(num) = field.parse::<f64>()
&& let Some(number) = serde_json::Number::from_f64(num)
{
return Value::Number(number);
}
// measurement (improvise-jc95), so the token stays text.
text.parse::<f64>()
.ok()
.and_then(serde_json::Number::from_f64)
}
// Otherwise treat as string
Value::String(field.to_string())
/// A CSV value as a typed JSON value: numeric text becomes its number.
pub fn typed(value: &Value) -> Value {
match value.as_str().and_then(number_in) {
Some(number) => Value::Number(number),
None => value.clone(),
}
}
#[cfg(test)]
@@ -310,8 +335,9 @@ mod tests {
assert_eq!(records.len(), 2);
assert_eq!(records[0]["Region"], Value::String("East".to_string()));
assert_eq!(records[0]["Product"], Value::String("Shirts".to_string()));
assert_eq!(records[0]["Revenue"], Value::String("1000".to_string()));
assert_eq!(
records[0]["Revenue"],
typed(&records[0]["Revenue"]),
Value::Number(serde_json::Number::from(1000))
);
}
@@ -323,9 +349,10 @@ mod tests {
let records = parse_csv(&path).unwrap();
assert_eq!(records.len(), 2);
assert!(records[0]["Revenue"].is_f64());
assert_eq!(records[0]["Revenue"], Value::String("1000.50".to_string()));
assert!(typed(&records[0]["Revenue"]).is_f64());
assert_eq!(
records[0]["Revenue"],
typed(&records[0]["Revenue"]),
Value::Number(serde_json::Number::from_f64(1000.50).unwrap())
);
}
@@ -363,10 +390,10 @@ mod tests {
assert_eq!(records.len(), 2);
assert_eq!(records[0]["Name"], Value::String("Widget".to_string()));
assert_eq!(
records[0]["Count"],
typed(&records[0]["Count"]),
Value::Number(serde_json::Number::from(5))
);
assert!(records[0]["Price"].is_f64());
assert!(typed(&records[0]["Price"]).is_f64());
assert_eq!(records[0]["Active"], Value::String("true".to_string()));
}
@@ -386,10 +413,7 @@ mod tests {
assert_eq!(records[3]["File"], Value::String("expenses".to_string()));
// Original fields preserved
assert_eq!(records[0]["Region"], Value::String("East".to_string()));
assert_eq!(
records[2]["Revenue"],
Value::Number(serde_json::Number::from(50))
);
assert_eq!(records[2]["Revenue"], Value::String("50".to_string()));
}
#[test]
@@ -492,10 +516,10 @@ mod tests {
]
);
assert_eq!(
merged.records,
*merged.records,
[
serde_json::json!({"entity": "src/a.rs", "coupled": "src/b.rs", "degree": 80, "File": "coupling"}),
serde_json::json!({"entity": "src/a.rs", "age-months": 2, "File": "age"}),
serde_json::json!({"entity": "src/a.rs", "coupled": "src/b.rs", "degree": "80", "File": "coupling"}),
serde_json::json!({"entity": "src/a.rs", "age-months": "2", "File": "age"}),
]
);
}
@@ -656,10 +680,7 @@ mod tests {
assert!(warnings.is_empty());
assert_eq!(records.len(), 2);
assert_eq!(records[0]["Region"], Value::String("East".to_string()));
assert_eq!(
records[0]["Revenue"],
Value::Number(serde_json::Number::from(1))
);
assert_eq!(records[0]["Revenue"], Value::String("1".to_string()));
assert_eq!(records[1]["Region"], Value::String("West".to_string()));
let (control, _) = parse_csv_bytes_with_warnings(b"Region,Revenue\nEa\rst,1\n").unwrap();
@@ -714,7 +735,8 @@ mod tests {
token.parse::<f64>().is_ok_and(|n| !n.is_finite()),
"precondition: {token} parses as a non-finite f64"
);
assert_eq!(parse_csv_field(token), Value::String(token.to_string()));
let text = Value::String(token.to_string());
assert_eq!(typed(&text), text);
}
}
@@ -723,10 +745,10 @@ mod tests {
#[test]
fn an_ordinary_float_is_still_a_number() {
assert_eq!(
parse_csv_field("1000.50"),
typed(&Value::String("1000.50".into())),
Value::Number(serde_json::Number::from_f64(1000.50).unwrap())
);
assert_eq!(parse_csv_field("0"), Value::Number(0.into()));
assert_eq!(typed(&Value::String("0".into())), Value::Number(0.into()));
}
/// improvise-5hfh: two equal header names are one JSON key, so the later
@@ -735,9 +757,9 @@ mod tests {
fn a_repeated_header_is_renamed_rather_than_overwriting_its_twin() {
let (records, warnings) = parse_csv_bytes_with_warnings(b"A,B,A\n1,2,3\n").unwrap();
let map = records[0].as_object().unwrap();
assert_eq!(map["A"], Value::Number(1.into()));
assert_eq!(map["B"], Value::Number(2.into()));
assert_eq!(map["A_2"], Value::Number(3.into()));
assert_eq!(map["A"], Value::String("1".into()));
assert_eq!(map["B"], Value::String("2".into()));
assert_eq!(map["A_2"], Value::String("3".into()));
assert_eq!(warnings.len(), 1);
assert!(warnings[0].contains("'A'") && warnings[0].contains("'A_2'"));
assert!(warnings[0].contains("column 3"), "{}", warnings[0]);
@@ -750,8 +772,8 @@ mod tests {
let (records, _) = parse_csv_bytes_with_warnings(b"A,A_2,A\n1,2,3\n").unwrap();
let map = records[0].as_object().unwrap();
assert_eq!(map.len(), 3, "{map:?}");
assert_eq!(map["A_2"], Value::Number(2.into()));
assert_eq!(map["A_3"], Value::Number(3.into()));
assert_eq!(map["A_2"], Value::String("2".into()));
assert_eq!(map["A_3"], Value::String("3".into()));
}
#[test]
@@ -786,8 +808,9 @@ mod tests {
assert_eq!(records.len(), 2);
assert_eq!(records[0]["Date"], Value::String("03/31/2026".to_string()));
assert_eq!(records[0]["Amount"], Value::String("-50.00".to_string()));
assert_eq!(
records[0]["Amount"],
typed(&records[0]["Amount"]),
Value::Number(serde_json::Number::from_f64(-50.00).unwrap())
);
assert_eq!(records[0]["Flag"], Value::String("*".to_string()));
@@ -797,7 +820,7 @@ mod tests {
Value::String("VENMO PAYMENT 260331".to_string())
);
assert_eq!(
records[1]["Amount"],
typed(&records[1]["Amount"]),
Value::Number(serde_json::Number::from_f64(-240.00).unwrap())
);
}
+688 -36
View File
@@ -1,14 +1,14 @@
use std::collections::HashMap;
use std::collections::{HashMap, HashSet};
use anyhow::{Result, anyhow};
use serde_json::Value;
use super::analyzer::{
DateComponent, FieldKind, FieldProposal, analyze_records, extract_array_at_path,
DateComponent, DateShape, FieldKind, FieldProposal, analyze_records, extract_array_at_path,
extract_date_component, find_array_paths,
};
use super::csv_parser::{FILE_CATEGORY, Merged, SourceFile};
use crate::formula::parse_formula;
use super::csv_parser::{self, CsvRecords, FILE_CATEGORY, Merged, SourceFile};
use crate::formula::{Formula, parse_formula};
use crate::model::MAX_CATEGORIES;
use crate::model::cell::{CellKey, CellValue};
use crate::view::{Axis, VIRTUAL_VIEW_NAMES, View, ViewStep, records_steps};
@@ -33,6 +33,42 @@ pub struct ImportPipeline {
/// The files a CSV merge read, each of which gets its own view. Empty for
/// a single file or a JSON import (improvise-lpzu).
pub sources: Vec<SourceFile>,
/// What the records' string values are, fixed by the constructor (8jk6.4).
scalars: Scalars,
}
/// How a record's values read. Why CSV is text: `context/code/formula-and-io.md`.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum Scalars {
/// JSON: a string is text and a number is a number.
Typed,
/// CSV: every value is source text, read as a number only where one is wanted.
Text,
}
impl Scalars {
/// The record as analysis sees it: typed, whatever its source.
fn typed_record(self, record: &Value) -> Value {
match (self, record) {
(Scalars::Text, Value::Object(map)) => Value::Object(
map.iter()
.map(|(k, v)| (k.clone(), csv_parser::typed(v)))
.collect(),
),
(Scalars::Text, other) | (Scalars::Typed, other) => other.clone(),
}
}
/// The value a measure takes from `value`, if it is a number.
fn number(self, value: &Value) -> Option<f64> {
match self {
Scalars::Typed => value.as_f64(),
Scalars::Text => value
.as_str()
.and_then(csv_parser::number_in)
.and_then(|n| n.as_f64()),
}
}
}
/// Several source records that land on one coordinate tuple (improvise-cgqt).
@@ -57,7 +93,17 @@ pub enum DuplicatePolicy {
}
impl ImportPipeline {
/// A pipeline over a JSON value.
pub fn new(raw: Value) -> Self {
Self::reading(raw, Scalars::Typed)
}
/// A pipeline over CSV records, whose values are their source text.
pub fn from_csv(records: CsvRecords) -> Self {
Self::reading(Value::Array(records.into_values()), Scalars::Text)
}
fn reading(raw: Value, scalars: Scalars) -> Self {
let array_paths = find_array_paths(&raw);
let mut pipeline = Self {
raw: raw.clone(),
@@ -69,6 +115,7 @@ impl ImportPipeline {
formulas: vec![],
on_duplicate: DuplicatePolicy::Refuse,
sources: vec![],
scalars,
};
// Auto-select if root is an array or there is exactly one candidate path.
@@ -84,7 +131,7 @@ impl ImportPipeline {
/// A pipeline over a CSV merge, carrying its sources for the per-file views.
pub fn from_merge(merged: Merged) -> Self {
let mut pipeline = Self::new(Value::Array(merged.records));
let mut pipeline = Self::from_csv(merged.records);
pipeline.seed_sources(merged.sources);
pipeline
}
@@ -112,7 +159,52 @@ impl ImportPipeline {
self.selected_path = path.to_string();
if let Some(arr) = extract_array_at_path(&self.raw, path) {
self.records = arr.clone();
self.proposals = analyze_records(&self.records);
self.analyze();
}
}
/// Propose a kind for every field, reading the records as `scalars` says.
pub fn analyze(&mut self) {
let typed: Vec<Value> = self
.records
.iter()
.map(|r| self.scalars.typed_record(r))
.collect();
self.proposals = analyze_records(&typed);
}
/// The warning for the records the build will skip, if it skips any.
pub fn skip_warning(&self) -> Option<String> {
let skipped = self.records.iter().filter(|r| !r.is_object()).count();
skip_warnings(self.records.len(), skipped)
}
/// Why there is nothing to build, when no record is an object to place.
pub fn nothing_to_import(&self) -> Option<String> {
let total = self.records.len();
match (total, self.records.iter().any(Value::is_object)) {
(_, true) => None,
(0, false) => Some("Nothing to import: the source has no records".into()),
(_, false) => Some(format!(
"Nothing to import: all {total} records were skipped as not JSON objects"
)),
}
}
/// The formulas, parsed. Every one that does not parse is named with its
/// error, never dropped (improvise-8jk6.5).
pub fn parsed_formulas(&self) -> Result<Vec<Formula>> {
let mut parsed = vec![];
let mut refused: Vec<String> = vec![];
for raw in &self.formulas {
match import_formula(raw) {
Ok(formula) => parsed.push(formula),
Err(e) => refused.push(e.to_string()),
}
}
match refused.is_empty() {
true => Ok(parsed),
false => Err(anyhow!(refused.join("\n"))),
}
}
@@ -147,13 +239,23 @@ impl ImportPipeline {
.filter(|p| p.accepted)
.map(|p| match p.kind {
FieldKind::Category => 1,
FieldKind::TimeCategory if p.date_format.is_some() => 1 + p.date_components.len(),
FieldKind::TimeCategory if p.date_format.is_some() => match p.date_shape {
DateShape::Hierarchy => 1,
DateShape::Split => 1 + p.date_components.len(),
},
FieldKind::TimeCategory => 1,
FieldKind::Measure | FieldKind::Label => 0,
})
.sum()
}
/// Why the date step cannot advance: see [`Plan::spelling_clash`].
pub fn spelling_clash(&self) -> Option<String> {
let plan = self.plan();
let placements: Vec<Placement> = self.records.iter().map(|r| plan.place(r)).collect();
plan.spelling_clash(&placements)
}
/// Every coordinate tuple more than one record lands on, in first-seen
/// order. Known before the build, so the wizard can ask (improvise-cgqt).
pub fn collisions(&self) -> Vec<Collision> {
@@ -208,6 +310,7 @@ impl ImportPipeline {
&& p.kind == FieldKind::TimeCategory
&& p.date_format.is_some()
&& !p.date_components.is_empty()
&& p.date_shape == DateShape::Split
})
.flat_map(|p| {
let fmt = p.date_format.as_deref().unwrap();
@@ -222,11 +325,18 @@ impl ImportPipeline {
})
})
.collect();
let categories = accepted(&[FieldKind::Category, FieldKind::TimeCategory]);
let date_hierarchies = categories
.iter()
.filter_map(|p| DateHierarchy::of(p))
.collect();
Plan {
categories: accepted(&[FieldKind::Category, FieldKind::TimeCategory]),
categories,
measures: accepted(&[FieldKind::Measure]),
labels: accepted(&[FieldKind::Label]),
date_extractions,
date_hierarchies,
scalars: self.scalars,
}
}
@@ -237,11 +347,18 @@ impl ImportPipeline {
let categories = &plan.categories;
let date_extractions = &plan.date_extractions;
if let Some(why) = self.nothing_to_import() {
return Err(anyhow!(why));
}
if categories.is_empty() {
return Err(anyhow!("At least one category must be accepted"));
}
let formulas = self.parsed_formulas()?;
let placements: Vec<Placement> = self.records.iter().map(|r| plan.place(r)).collect();
if let Some(clash) = plan.spelling_clash(&placements) {
return Err(anyhow!(clash));
}
let collisions = collisions_among(&placements);
let occurrence = match (&self.on_duplicate, collisions.is_empty()) {
(_, true) | (DuplicatePolicy::Sum, false) => None,
@@ -253,11 +370,16 @@ impl ImportPipeline {
for cat_proposal in categories {
wb.add_category(&cat_proposal.field)?;
for val in cat_proposal
.distinct_values
// A hierarchical date column's items are its components, added below.
let raw_items = match plan
.date_hierarchies
.iter()
.filter(|v| !v.trim().is_empty())
.any(|h| h.field == cat_proposal.field)
{
true => &[][..],
false => &cat_proposal.distinct_values[..],
};
for val in raw_items.iter().filter(|v| !v.trim().is_empty()) {
wb.add_item(&cat_proposal.field, val)?;
}
}
@@ -288,10 +410,9 @@ impl ImportPipeline {
wb.add_item("_Measure", &m.field)?;
}
let mut skipped_non_objects = 0usize;
// Per-extraction tally of values the detected format could not parse. A
// derived coordinate that is merely absent is improvise-yss5.
let mut unparsed_dates = vec![0usize; date_extractions.len()];
let mut unparsed_dates = vec![0usize; date_extractions.len() + plan.date_hierarchies.len()];
// Two rows sharing every coordinate are one cell; last-row-wins lost the
// earlier one (improvise-2cw9). The map only deduplicates — gotcha 18.
let mut cells: Vec<PendingCell> = vec![];
@@ -304,12 +425,16 @@ impl ImportPipeline {
for (at, placement) in placements.into_iter().enumerate() {
match placement {
Placement::NotObject => skipped_non_objects += 1,
Placement::NotObject => {}
Placement::Placed {
mut coords,
measures,
unparsed,
chains,
} => {
for chain in &chains {
add_date_chain(&mut wb, chain)?;
}
for (cat, item) in &coords {
wb.add_item(cat, item)?;
}
@@ -352,23 +477,30 @@ impl ImportPipeline {
wb.model.set_cell(cell.key, CellValue::Number(cell.total));
}
// Parse and add formulas
// Formulas target the "_Measure" category by default.
let formula_cat: String = "_Measure".to_string();
for raw in &self.formulas {
if let Ok(formula) = parse_formula(raw, &formula_cat) {
wb.model.add_formula(formula);
}
for formula in formulas {
wb.model.add_formula(formula);
}
for view in source_views(&wb, &self.sources, &plan) {
wb.views.insert(view.name.clone(), view);
}
let mut warnings: Vec<String> = skip_warnings(self.records.len(), skipped_non_objects)
.into_iter()
.collect();
warnings.extend(unparsed_date_warnings(date_extractions, &unparsed_dates));
let mut warnings: Vec<String> = self.skip_warning().into_iter().collect();
let (split, whole) = unparsed_dates.split_at(date_extractions.len());
warnings.extend(unparsed_date_warnings(date_extractions, split));
warnings.extend(
plan.date_hierarchies
.iter()
.zip(whole)
.filter(|(_, n)| **n > 0)
.map(|(h, n)| {
format!(
"'{}' kept its raw value in {n} row(s), ungrouped; it did not match \
the date format '{}'",
h.field, h.format
)
}),
);
warnings.extend(collision_warning(&summed));
warnings.extend(occurrence.map(|name| separated_note(name, &collisions)));
@@ -386,6 +518,7 @@ fn unseen(field: &str, kind: FieldKind, distinct_values: Vec<String>) -> FieldPr
accepted: true,
date_format: None,
date_components: vec![],
date_shape: DateShape::default(),
}
}
@@ -463,6 +596,41 @@ struct Plan<'a> {
measures: Vec<&'a FieldProposal>,
labels: Vec<&'a FieldProposal>,
date_extractions: Vec<(&'a str, &'a str, DateComponent, String)>,
date_hierarchies: Vec<DateHierarchy<'a>>,
scalars: Scalars,
}
/// A date column imported as ONE category: its items are the finest chosen
/// component, grouped by the coarser ones (improvise-8jk6.3, core.md §19).
struct DateHierarchy<'a> {
field: &'a str,
format: &'a str,
/// The chosen components, coarsest first.
levels: Vec<DateComponent>,
}
impl<'a> DateHierarchy<'a> {
fn of(p: &'a FieldProposal) -> Option<Self> {
let levels: Vec<DateComponent> = DateComponent::COARSE_TO_FINE
.into_iter()
.filter(|c| p.date_components.contains(c))
.collect();
let hierarchical =
p.kind == FieldKind::TimeCategory && p.date_shape == DateShape::Hierarchy;
(hierarchical && !levels.is_empty()).then_some(Self {
field: &p.field,
format: p.date_format.as_deref()?,
levels,
})
}
/// Every level's name for one value, root first; the last is the item.
fn chain(&self, value: &str) -> Option<Vec<String>> {
self.levels
.iter()
.map(|c| extract_date_component(value, self.format, *c))
.collect()
}
}
/// Where one source record lands. Worked out once, for the build and for the
@@ -474,6 +642,7 @@ enum Placement {
coords: Vec<(String, String)>,
measures: Vec<(String, f64)>,
unparsed: Vec<usize>,
chains: Vec<DateChain>,
},
NotObject,
}
@@ -485,6 +654,7 @@ impl Plan<'_> {
};
let mut coords: Vec<(String, String)> = vec![];
let mut unparsed: Vec<usize> = vec![];
let mut chains: Vec<DateChain> = vec![];
// No value is no coordinate: records are sparse (improvise-8jk6.2).
let named = |field: &str| map.get(field).and_then(scalar_name);
@@ -493,7 +663,24 @@ impl Plan<'_> {
.iter()
.filter_map(|p| Some((p, named(&p.field)?)))
{
coords.push((cat_proposal.field.clone(), v.clone()));
let hierarchy = self
.date_hierarchies
.iter()
.enumerate()
.find(|(_, h)| h.field == cat_proposal.field);
let item = match hierarchy.map(|(at, h)| (at, h.chain(&v))) {
Some((_, Some(chain))) => {
let item = chain.last().cloned().unwrap_or_default();
chains.push((cat_proposal.field.clone(), chain));
item
}
Some((at, None)) => {
unparsed.push(self.date_extractions.len() + at);
v.clone()
}
None => v.clone(),
};
coords.push((cat_proposal.field.clone(), item));
// Extract date components from this field's value
for (at, (field, fmt, comp, derived_name)) in self.date_extractions.iter().enumerate() {
@@ -519,16 +706,89 @@ impl Plan<'_> {
let measures = self
.measures
.iter()
.filter_map(|m| Some((m.field.clone(), map.get(&m.field)?.as_f64()?)))
.filter_map(|m| Some((m.field.clone(), self.scalars.number(map.get(&m.field)?)?)))
.collect();
Placement::Placed {
coords,
measures,
unparsed,
chains,
}
}
}
impl Plan<'_> {
/// Why the build stops when an unparsed date is spelled like a generated
/// item or group (improvise-d0n6; formula-and-io.md, date hierarchy).
fn spelling_clash(&self, placements: &[Placement]) -> Option<String> {
let generated = |field: &str| -> HashSet<&str> {
placements
.iter()
.filter_map(|p| match p {
Placement::Placed { chains, .. } => Some(chains),
Placement::NotObject => None,
})
.flatten()
.filter(|(cat, _)| cat == field)
.flat_map(|(_, chain)| chain.iter().map(String::as_str))
.collect()
};
for (at, h) in self.date_hierarchies.iter().enumerate() {
let tally = self.date_extractions.len() + at;
let names = generated(h.field);
let mut clashing: Vec<&str> = vec![];
for p in placements {
let Placement::Placed {
coords, unparsed, ..
} = p
else {
continue;
};
let raw = coords.iter().find(|(c, _)| c == h.field).map(|(_, v)| v);
let clash = raw.filter(|v| {
unparsed.contains(&tally)
&& names.contains(v.as_str())
&& !clashing.contains(&v.as_str())
});
clashing.extend(clash.map(String::as_str));
}
if !clashing.is_empty() {
return Some(format!(
"'{}' has value(s) that do not match the date format '{}' but are spelled \
like a month, quarter or year it generates: {}; fix them, or import the \
column with the split shape",
h.field,
h.format,
clashing.join(", ")
));
}
}
None
}
}
/// One hierarchical date coordinate: its category and its chain, root first.
type DateChain = (String, Vec<String>);
/// Register a date item under its groups, and each group under its parent.
fn add_date_chain(wb: &mut Workbook, (cat, chain): &DateChain) -> Result<()> {
let Some((item, groups)) = chain.split_last() else {
return Ok(());
};
match groups.last() {
Some(group) => wb.add_item_in_group(cat, item, group)?,
None => wb.add_item(cat, item)?,
};
let category = wb
.model
.category_mut(cat)
.ok_or_else(|| anyhow!("category '{cat}' vanished mid-import"))?;
for pair in groups.windows(2) {
category.add_group(crate::model::category::Group::new(&pair[1]).with_parent(&pair[0]));
}
Ok(())
}
/// A tuple, and each record on it: its 1-based position and the measures it writes.
type OnTuple<'a> = (&'a Vec<(String, String)>, Vec<(usize, Vec<&'a str>)>);
@@ -635,6 +895,11 @@ fn unparsed_date_warnings(
.collect()
}
/// An import formula, targeting `_Measure`, or the error naming it (improvise-8jk6.5).
fn import_formula(raw: &str) -> Result<Formula> {
parse_formula(raw, "_Measure").map_err(|e| anyhow!("Formula error in '{raw}': {e}"))
}
/// The warning for records that could not be placed in the grid. A record
/// missing a category value is placed, so only a non-object is skipped.
fn skip_warnings(total: usize, skipped_non_objects: usize) -> Option<String> {
@@ -822,9 +1087,13 @@ impl ImportWizard {
fn validation_error(&self) -> Option<String> {
match self.step {
WizardStep::ReviewProposals => self
.no_category_message()
.pipeline
.nothing_to_import()
.or_else(|| self.no_category_message())
.or_else(|| self.category_limit_message()),
WizardStep::ConfigureDates => self.category_limit_message(),
WizardStep::ConfigureDates => self
.category_limit_message()
.or_else(|| self.pipeline.spelling_clash()),
WizardStep::ResolveDuplicates => match &self.pipeline.on_duplicate {
DuplicatePolicy::Sum => None,
DuplicatePolicy::Separate(_) => self.separate_blocked(),
@@ -1102,6 +1371,18 @@ impl ImportWizard {
}
}
/// Switch the field at the cursor between ONE hierarchical category and the
/// flat split into one category per component (improvise-8jk6.3).
pub fn toggle_date_shape(&mut self) {
if let Some((pi, _)) = self.date_config_at_cursor() {
let proposal = &mut self.pipeline.proposals[pi];
proposal.date_shape = match proposal.date_shape {
DateShape::Hierarchy => DateShape::Split,
DateShape::Split => DateShape::Hierarchy,
};
}
}
// ── Formula editing ────────────────────────────────────────────────────────
/// Buffer for typing a new formula in the DefineFormulas step.
@@ -1117,14 +1398,24 @@ impl ImportWizard {
self.formula_buffer.pop();
}
/// Commit the current formula buffer to the pipeline's formula list.
/// Commit the current formula buffer to the pipeline's formula list. One
/// that does not parse stays in the buffer, with the error (improvise-8jk6.5).
pub fn confirm_formula(&mut self) {
let text = self.formula_buffer.trim().to_string();
if !text.is_empty() {
self.pipeline.formulas.push(text);
match (text.is_empty(), import_formula(&text)) {
(false, Err(e)) => self.message = Some(e.to_string()),
(false, Ok(_)) => {
self.pipeline.formulas.push(text);
self.close_formula_edit();
}
(true, _) => self.close_formula_edit(),
}
}
fn close_formula_edit(&mut self) {
self.formula_buffer.clear();
self.formula_editing = false;
self.message = None;
self.cursor = self.pipeline.formulas.len().saturating_sub(1);
}
@@ -1404,6 +1695,7 @@ mod tests {
prop.date_format = Some("%m/%d/%Y".to_string());
prop.date_components.push(DateComponent::Month);
prop.date_components.push(DateComponent::Year);
prop.date_shape = crate::import::analyzer::DateShape::Split;
}
}
p
@@ -1507,6 +1799,7 @@ mod tests {
for prop in &mut p.proposals {
if prop.field == "Date" && prop.kind == FieldKind::TimeCategory {
prop.date_components.push(DateComponent::Month);
prop.date_shape = crate::import::analyzer::DateShape::Split;
}
}
let wb = p.build_model().unwrap();
@@ -2244,8 +2537,11 @@ mod tests {
),
];
fn code_maat_records() -> Value {
Value::Array(merged_pipeline(&CODE_MAAT).records)
/// The code-maat merge without its sources, so no per-file views.
fn code_maat_without_views() -> ImportPipeline {
let mut p = merged_pipeline(&CODE_MAAT);
p.sources.clear();
p
}
/// Every non-empty cell a view's grid draws, row by row.
@@ -2374,11 +2670,39 @@ mod tests {
let date = p.proposals.iter_mut().find(|f| f.field == "date").unwrap();
assert_eq!(date.kind, FieldKind::TimeCategory);
date.date_components = vec![DateComponent::Month];
date.date_shape = crate::import::analyzer::DateShape::Split;
let (wb, _) = p.build_model_with_warnings().unwrap();
assert_ne!(wb.views["abs-churn"].axis_of("date_Month"), Axis::None);
assert_eq!(wb.views["age"].axis_of("date_Month"), Axis::None);
}
/// improvise-8jk6.3 under improvise-8jk6.12: the default shape puts the date
/// parts IN the file's own column, so that column is what its view seats.
#[test]
fn a_files_view_seats_its_own_hierarchical_date_column() {
use crate::import::analyzer::DateComponent;
let mut p = merged_pipeline(&[
(
"m/abs-churn.csv",
"date,added
2026-01-02,10
",
),
(
"m/age.csv",
"entity,age-months
src/a.rs,2
",
),
]);
let date = p.proposals.iter_mut().find(|f| f.field == "date").unwrap();
date.date_components = vec![DateComponent::Year, DateComponent::Month];
let (wb, _) = p.build_model_with_warnings().unwrap();
assert!(wb.model.category("date_Month").is_none(), "one category");
assert_ne!(wb.views["abs-churn"].axis_of("date"), Axis::None);
assert_eq!(wb.views["age"].axis_of("date"), Axis::None);
}
/// improvise-8jk6.12 (2): two files of one batch never share a view name.
/// Evidence: `context/test-evidence/formula-and-io.md`.
#[test]
@@ -2519,7 +2843,7 @@ mod tests {
/// 0, "" or a skip. Evidence: `context/test-evidence/formula-and-io.md`.
#[test]
fn files_with_different_headers_import_as_one_sparse_model() {
let p = ImportPipeline::new(code_maat_records());
let p = code_maat_without_views();
let kinds: Vec<(&str, FieldKind)> = p
.proposals
.iter()
@@ -2561,7 +2885,7 @@ mod tests {
/// "". Evidence: `context/test-evidence/formula-and-io.md`.
#[test]
fn a_label_column_a_record_lacks_is_no_coordinate_not_an_empty_item() {
let mut p = ImportPipeline::new(code_maat_records());
let mut p = code_maat_without_views();
p.proposals
.iter_mut()
.find(|f| f.field == "coupled")
@@ -2634,6 +2958,7 @@ mod tests {
for prop in &mut p.proposals {
if prop.field == "Date" {
prop.date_components.push(DateComponent::Month);
prop.date_shape = crate::import::analyzer::DateShape::Split;
}
}
let wb = p.build_model().unwrap();
@@ -3146,6 +3471,7 @@ mod tests {
w.advance();
assert_eq!(w.step, WizardStep::ConfigureDates, "setup");
w.toggle_date_component();
w.toggle_date_shape();
w.advance();
assert_eq!(w.step, WizardStep::ResolveDuplicates, "setup");
w.move_cursor(1);
@@ -3164,4 +3490,330 @@ mod tests {
w.message
);
}
/// improvise-8jk6.5. Evidence: `context/test-evidence/formula-and-io.md`.
#[test]
fn a_formula_that_does_not_parse_stops_the_build_and_is_named() {
let mut p = ImportPipeline::new(json!([{"region": "East", "revenue": 1.0}]));
p.formulas = vec![
"Good = revenue".into(),
"Bad = revenue +".into(),
"Worse = * 2".into(),
];
let err = p.build_model().expect_err("an unparseable formula refuses");
let msg = err.to_string();
assert!(msg.contains("Formula error in 'Bad = revenue +'"), "{msg}");
assert!(msg.contains("Formula error in 'Worse = * 2'"), "{msg}");
assert!(!msg.contains("Good"), "{msg}");
}
/// improvise-8jk6.5: the wizard refuses where the text can still be
/// fixed. Evidence: `context/test-evidence/formula-and-io.md`.
#[test]
fn confirming_a_formula_that_does_not_parse_keeps_it_out_and_the_text_in() {
let mut w = sample_wizard();
w.start_formula_edit();
for c in "Bad = revenue +".chars() {
w.push_formula_char(c);
}
w.confirm_formula();
assert!(w.pipeline.formulas.is_empty(), "{:?}", w.pipeline.formulas);
assert_eq!(w.formula_buffer, "Bad = revenue +");
assert!(w.formula_editing);
let message = w.message.clone().unwrap_or_default();
assert!(
message.starts_with("Formula error in 'Bad = revenue +'"),
"{message}"
);
}
/// improvise-8jk6.4: the role reads the text. Evidence:
/// `context/test-evidence/formula-and-io.md`.
#[test]
fn a_csv_column_read_as_a_category_keeps_its_text_and_as_a_measure_its_number() {
use crate::import::csv_parser::parse_csv_bytes_with_warnings;
use crate::model::cell::{CellKey, CellValue};
let csv = b"commit,lines\n00565796,3\n220944e2,5\n";
let (records, _) = parse_csv_bytes_with_warnings(csv).unwrap();
let mut p = ImportPipeline::from_csv(records);
let kind = |p: &ImportPipeline, f: &str| {
p.proposals
.iter()
.find(|x| x.field == f)
.unwrap()
.kind
.clone()
};
assert_eq!(kind(&p, "commit"), FieldKind::Measure, "analysis types");
let commit = p.proposals.iter_mut().find(|x| x.field == "commit");
commit.unwrap().kind = FieldKind::Category;
let wb = p.build_model().unwrap();
let items: Vec<&String> = wb.model.category("commit").unwrap().items.keys().collect();
assert_eq!(items, ["00565796", "220944e2"]);
let key = CellKey::new(vec![
("_Measure".into(), "lines".into()),
("commit".into(), "220944e2".into()),
]);
assert_eq!(wb.model.get_cell(&key), Some(&CellValue::Number(5.0)));
}
/// improvise-8jk6.6 meets lpzu's header-only rule: a folder of nothing but
/// headers kept no record. Evidence: `context/test-evidence/formula-and-io.md`.
#[test]
fn a_folder_of_only_header_only_files_keeps_no_record_and_refuses() {
let p = merged_pipeline(&[("m/a.csv", "entity,x\n"), ("m/b.csv", "entity,y\n")]);
assert!(p.proposals.iter().any(|f| f.field == "File"), "setup");
let msg = p.build_model().expect_err("refuses").to_string();
assert_eq!(msg, "Nothing to import: the source has no records");
}
/// improvise-8jk6.6: the wizard asks on the review step, where another
/// path can still be chosen. Evidence: the test-evidence doc.
#[test]
fn the_review_step_says_when_no_record_can_be_imported() {
let mut w = ImportWizard::new(json!([1, 2]));
assert_eq!(w.step, WizardStep::ReviewProposals, "setup");
w.advance();
assert_eq!(w.step, WizardStep::ReviewProposals);
assert_eq!(
w.message.as_deref(),
Some("Nothing to import: all 2 records were skipped as not JSON objects")
);
}
/// improvise-8jk6.6. Evidence: `context/test-evidence/formula-and-io.md`.
#[test]
fn a_build_that_keeps_no_record_refuses_and_says_why() {
let skipped = ImportPipeline::new(json!([1, "two", null]));
let msg = skipped.build_model().expect_err("refuses").to_string();
assert_eq!(
msg,
"Nothing to import: all 3 records were skipped as not JSON objects"
);
let empty = ImportPipeline::new(json!([]));
let msg = empty.build_model().expect_err("refuses").to_string();
assert_eq!(msg, "Nothing to import: the source has no records");
}
}
/// improvise-8jk6.3: a date column imports as ONE hierarchical category.
#[cfg(test)]
mod date_hierarchy {
use super::{DuplicatePolicy, ImportPipeline};
use crate::import::analyzer::{DateComponent, DateShape, FieldKind};
use crate::model::cell::CellKey;
use crate::view::GridLayout;
use serde_json::json;
/// Two days of one month, a month in the next quarter, and January of the
/// NEXT year, so a month must stay unique across years.
fn records() -> serde_json::Value {
json!([
{"Date": "01/15/2024", "Amount": 1.0},
{"Date": "01/20/2024", "Amount": 10.0},
{"Date": "04/02/2024", "Amount": 100.0},
{"Date": "01/03/2025", "Amount": 1000.0},
])
}
fn pipeline(components: &[DateComponent], shape: DateShape) -> ImportPipeline {
let mut p = ImportPipeline::new(records());
p.on_duplicate = DuplicatePolicy::Sum;
let date = p.proposals.iter_mut().find(|p| p.field == "Date").unwrap();
assert_eq!(date.kind, FieldKind::TimeCategory, "precondition");
date.date_components = components.to_vec();
date.date_shape = shape;
p
}
fn cell(month: &str) -> CellKey {
CellKey::new(vec![
("Date".into(), month.into()),
("_Measure".into(), "Amount".into()),
])
}
/// The default shape: ONE category named after the column, its items the
/// months, grouped into quarters grouped into years.
#[test]
fn a_date_column_imports_as_one_category_of_months_in_quarters_in_years() {
use DateComponent::*;
let p = pipeline(&[Month, Year, Quarter], DateShape::default());
assert_eq!(p.proposed_category_count(), 1, "one slot, not four");
let wb = p.build_model().unwrap();
let names: Vec<&str> = wb.model.regular_category_names();
assert_eq!(
names,
vec!["Date"],
"no Date_Year, Date_Quarter or Date_Month"
);
let date = wb.model.category("Date").unwrap();
let items: Vec<&str> = date.items.keys().map(String::as_str).collect();
assert_eq!(items, vec!["2024-01", "2024-04", "2025-01"]);
assert_eq!(date.item_chain("2024-01"), vec!["2024", "2024-Q1"]);
assert_eq!(date.item_chain("2025-01"), vec!["2025", "2025-Q1"]);
assert_eq!(
wb.model.get_cell(&cell("2024-01")).and_then(|v| v.as_f64()),
Some(11.0),
"two days of one month are one cell"
);
assert_eq!(
wb.model.get_cell(&cell("2025-01")).and_then(|v| v.as_f64()),
Some(1000.0),
"January 2025 is not January 2024"
);
}
/// Chosen levels only: without Quarter, months group straight into years.
#[test]
fn a_level_left_unchosen_is_skipped_not_emptied() {
use DateComponent::*;
let wb = pipeline(&[Year, Month], DateShape::Hierarchy)
.build_model()
.unwrap();
let date = wb.model.category("Date").unwrap();
assert_eq!(date.item_chain("2024-04"), vec!["2024"]);
let wb = pipeline(&[Quarter], DateShape::Hierarchy)
.build_model()
.unwrap();
let date = wb.model.category("Date").unwrap();
let items: Vec<&str> = date.items.keys().map(String::as_str).collect();
assert_eq!(
items,
vec!["2024-Q1", "2024-Q2", "2025-Q1"],
"flat quarters"
);
assert!(date.item_chain("2024-Q1").is_empty());
}
/// The user's ruling on improvise-felo.6: the hierarchy opens folded, one
/// row per year.
#[test]
fn an_imported_date_hierarchy_opens_folded_to_its_years() {
use DateComponent::*;
let wb = pipeline(&[Year, Quarter, Month], DateShape::Hierarchy)
.build_model()
.unwrap();
let layout = GridLayout::with_frozen_records(&wb.model, wb.active_view(), None);
let rows: Vec<String> = (0..layout.row_count())
.map(|r| layout.row_label(r))
.collect();
assert_eq!(rows, vec!["2024", "2025"]);
let v = layout.evaluate(&wb.model, &layout.cell_key(0, 0).unwrap());
assert_eq!(
v.and_then(|v| v.as_f64()),
Some(111.0),
"2024 folds its months"
);
}
/// The flat split stays available as a choice, and is what it was.
#[test]
fn the_split_shape_keeps_the_raw_dates_and_one_category_per_component() {
use DateComponent::*;
let p = pipeline(&[Year, Month], DateShape::Split);
assert_eq!(p.proposed_category_count(), 3);
let wb = p.build_model().unwrap();
let mut names: Vec<&str> = wb.model.regular_category_names();
names.sort_unstable();
assert_eq!(names, vec!["Date", "Date_Month", "Date_Year"]);
assert!(
wb.model
.category("Date")
.unwrap()
.items
.contains_key("01/15/2024")
);
}
/// A value the detected format cannot read keeps its raw text as an
/// ungrouped item, and the build says so rather than dropping the record.
#[test]
fn an_unparsed_date_keeps_its_value_and_is_warned_about() {
use DateComponent::*;
let mut p = pipeline(&[Year, Month], DateShape::Hierarchy);
let date = p.proposals.iter_mut().find(|p| p.field == "Date").unwrap();
date.date_format = Some("%Y-%m-%d".into());
let (wb, warnings) = p.build_model_with_warnings().unwrap();
assert!(wb.model.category("Date_Year").is_none(), "one category");
let date = wb.model.category("Date").unwrap();
assert!(
date.items.contains_key("01/15/2024"),
"the raw value is kept"
);
assert!(
date.item_chain("01/15/2024").is_empty(),
"and it is ungrouped"
);
assert!(
warnings
.iter()
.any(|w| w.contains("'Date' kept its raw value in 4 row(s)")),
"{warnings:?}"
);
}
/// A %Y-%m-%d column imported Year > Month, one valid date and one raw
/// value spelled `raw`.
fn with_raw(raw: &str) -> ImportPipeline {
use DateComponent::*;
let mut p = ImportPipeline::new(json!([
{"Date": "2024-01-15", "Amount": 1.0},
{"Date": raw, "Amount": 2.0},
]));
p.on_duplicate = DuplicatePolicy::Sum;
let date = p.proposals.iter_mut().find(|p| p.field == "Date").unwrap();
date.kind = FieldKind::TimeCategory;
date.date_format = Some("%Y-%m-%d".into());
date.date_components = vec![Year, Month];
date.date_shape = DateShape::Hierarchy;
p
}
/// A raw value spelled like a generated month would merge with it.
#[test]
fn a_raw_date_spelled_like_a_generated_item_is_refused() {
let err = with_raw("2024-01").build_model_with_warnings().unwrap_err();
let msg = err.to_string();
assert!(msg.contains("'Date'") && msg.contains("2024-01"), "{msg}");
assert!(msg.contains("split"), "{msg}");
}
/// A raw value spelled like a generated year would sit beside its group.
#[test]
fn a_raw_date_spelled_like_a_generated_group_is_refused() {
let err = with_raw("2024").build_model_with_warnings().unwrap_err();
let msg = err.to_string();
assert!(msg.contains("'Date'") && msg.contains("2024"), "{msg}");
}
/// The control: a raw value spelled like nothing generated still imports,
/// so the refusal above is not a blanket one.
#[test]
fn a_raw_date_spelled_like_nothing_generated_is_kept_ungrouped() {
let (wb, warnings) = with_raw("sometime").build_model_with_warnings().unwrap();
let date = wb.model.category("Date").unwrap();
assert!(date.item_chain("sometime").is_empty());
assert!(date.items.contains_key("2024-01"), "{warnings:?}");
assert!(
warnings.iter().any(|w| w.contains("1 row(s)")),
"{warnings:?}"
);
}
/// The clash is answered on the date step, where `s` fixes it, not on the
/// last step three screens later (improvise-907).
#[test]
fn the_date_step_will_not_advance_over_a_spelling_clash() {
use super::{ImportWizard, WizardStep};
let mut w = ImportWizard::from_pipeline(with_raw("2024"));
w.step = WizardStep::ConfigureDates;
w.advance();
assert_eq!(w.step, WizardStep::ConfigureDates);
let msg = w.message.clone().unwrap_or_default();
assert!(msg.contains("'Date'") && msg.contains("2024"), "{msg}");
w.toggle_date_shape();
w.advance();
assert_ne!(w.step, WizardStep::ConfigureDates, "split clears it");
}
}
@@ -1031,7 +1031,7 @@ fn a_csv_header_with_an_embedded_newline_survives_a_save() {
let csv = "\"Region\nName\",Amount\nEast,1\nWest,2\n";
let (records, _) =
crate::import::csv_parser::parse_csv_bytes_with_warnings(csv.as_bytes()).unwrap();
let wb = crate::import::wizard::ImportPipeline::new(serde_json::Value::Array(records))
let wb = crate::import::wizard::ImportPipeline::from_csv(records)
.build_model()
.unwrap();
assert!(
+3
View File
@@ -967,6 +967,9 @@ pub struct WizardDateField {
/// what is drawn, so the substitution is made once, here.
pub format: String,
pub components: Vec<(String, bool)>,
/// What the chosen components become, as a sentence: one hierarchical
/// category, or a split (improvise-8jk6.3). Worded once, in the projection.
pub shape: String,
}
/// Several source records sharing every coordinate (improvise-cgqt). Data,
+8
View File
@@ -144,6 +144,7 @@ invariant(an_unprojected_cell_is_unknown,'Every client read of the grid cache go
invariant(default_duplicate_policy_is_refuse,'ImportPipeline::on_duplicate defaults to DuplicatePolicy::Refuse, so any path that never asked the question, the headless CLI included (--on-duplicate defaults to refuse), errors naming the collisions rather than summing. On ResolveDuplicates, Refuse blocks advance with a message; the build under Refuse returns an error when collisions exist (improvise-cgqt).').
invariant(a_collision_is_a_cell_written_twice,'A collision is a cell -- a coordinate tuple AND a measure -- written by two or more records, never merely a shared tuple: records with disjoint measures on one tuple write different cells and collide with nothing, and only the records that share a cell are listed and numbered (improvise-9bit). Plan::place is the one placement pass, used by both collisions() and build_model_with_warnings, so the question asked and the model built cannot disagree.').
invariant(wizard_draws_are_clipped_to_the_panel,'Every import-wizard write goes through Panel in src/ui/import_wizard_ui.rs, which clips to the panel rows and width, so no step can draw past the buffer; text is measured in cells by ui::text::truncate and wrap, never in bytes (review B1/B2 and a Codex P2 on PR #304).').
invariant(a_window_over_a_held_summary_equals_grid_body,'grid_window over the GridSummary of the same layout returns what the one-walk grid_body returns for that window, so the browser window and the TUI whole grid cannot disagree on a width, a format or a total. Pinned by a_window_over_a_held_summary_equals_the_one_walk_grid_body in improvise-command ui/viewport.rs; it holds only where the summary is current, which is why App::grid_geometry is dropped with the layout.').
doctrine(bug_first,'write a failing test demonstrating the bug before fixing it').
doctrine(unit_over_e2e,'prefer small unit tests over integration/e2e').
doctrine(suite_speed,'Two numbers, because the ambiguity between them is what let this drift: NO SINGLE TEST BINARY may exceed 2 seconds, and the WORKSPACE WALL CLOCK is the sum of them. Measured at 22d43b2: 1309 tests over 33 binaries, ~2.2s wall, longest single binary 1.42s -- so the workspace is already past 2s and the rule that bites is the per-binary one. Re-MEASURE rather than re-assert: cargo test --workspace prints a \"finished in\" per binary. Excludes compilation, and excludes harness/ (Playwright, minutes, its own CI job).').
@@ -448,6 +449,12 @@ work_done('improvise-cgqt',feature,'PR #304, merge 4f38a35e. The import wizard a
work_done('improvise-9bit',bugfix,'PR #304 (commit f00ed3e9), merge 4f38a35e. The collision check counted records with disjoint measures on one tuple as colliding; collisions_among now keeps only records that write a measure another record on the tuple also writes. Rival: the tuple-only rule, which fails only_the_records_that_share_a_cell_are_listed_and_numbered.').
work_done('improvise-4klp',test,'PR #304 (commit f00ed3e9), merge 4f38a35e. the_name_check_covers_categories_derived_from_dates: a date-derived category name such as when_Year is refused as the occurrence category name. Green on arrival; its evidence is the reviewer rival that drops date-derived names from the taken names.').
work_done('improvise-felo.1',feature,'PR #307, merge 1719f2cf. MAX_CATEGORIES raised from 12 to 24 in improvise-core model/types.rs, pub and re-exported as improvise_core::model::MAX_CATEGORIES; the import wizard private copy is deleted and it reads the model constant. Fill-to-the-cap tests derive the number from the constant. crates/improvise-core/examples/category_width_bench.rs measures 12 against 24; the coupling inventory is context/code/core.md.').
work_done('improvise-felo.13',bugfix,'PR #333, merge 8da5a310. GridLayout is no longer O(rows^2): a private per-axis position index (row_pos/col_pos, rebuilt by reindex) answers cell_key/row_tuple/column_key/folds/data_to_visual in O(1); App::refresh_layout rebuilds only when layout_stale, the drawn view name or the drill snapshot pointer moved, so a cursor key rebuilds nothing; the column widths and row-header width are measured once per layout (App::grid_geometry); GridSummary/grid_summary/grid_window in ui/viewport.rs let project::grid_in evaluate only a window; DataStore::exact_positions with arities answers a full-width sorted partial with one probe. Measurements: context/test-evidence/layout-cost.md and scale-bisect.md; harness benches/scale_bisect.rs and bench/*.py.').
work_done('improvise-31oe.3',bugfix,'PR #316, merge 66931785. The command language reads |pipe quoting| through improvise_formula::quote, so create-view |By area| names By area; a surplus argument is refused; cmd::core::only_when_omitted gates the cursor fallbacks of clear-cell, yank, paste and drill-into-cell so a typed coordinate is never overridden. context/code/cli.md section 14.').
work_done('improvise-31oe.2',bugfix,'PR #316, merge 66931785. set-axis, set-page, toggle-group, hide-item, show-item and switch-view check at APPLY time, through the ViewOnly off-wire effect RequireKnown(Needs) built by effect_cmds::unless_unknown, that the category, item, group or view exists, and refuse and abort the batch otherwise; a refused name stops the rest of a : line, while headless improvise cmd runs each segment as its own batch (improvise-31oe.14, open). context/code/cli.md section 15.').
work_done('improvise-8jk6.3',feature,'PR #322, merge d571abbd. A date column imports as ONE hierarchical category (Year > Quarter > Month) by default and opens folded to its years; --split-date <field> and s on the wizard date step restore the flat split (DateShape::Split); --split-date naming a non-date field and --axis naming a category the import did not build are refused with exit 1. context/code/formula-and-io.md, Import section.').
work_done('improvise-d0n6',bugfix,'PR #322, merge d571abbd. Plan::spelling_clash refuses a raw unparsed date spelled like a generated month, quarter or year item, because the two would silently merge; asked at the wizard ConfigureDates step and again at build time so the headless import cannot reach it.').
work_done('improvise-8jk6.6',bugfix,'PR #321, merge 7e879702. headless import --strict: with --no-wizard any skipped record is a refusal before the build and nothing is written; without it a partial skip writes the model and warns with a count. A folder whose CSVs are all header-only is refused as an import that kept no record. context/code/cli.md section 13.').
work_followup('improvise-8tk','improvise-lae','no-file-path UX: prompt for a path rather than only erroring. ExportPrompt is CSV-only, so a new SavePrompt TextEntry mode committing to SaveAs is the likely shape.').
work_followup('improvise-6sq','improvise-ct5','i on an empty records grid reports \'cannot drill\', leaking a pivot-mode concept into records mode').
work_followup('improvise-h0b','improvise-a2x','ViewState field types (AppMode, MinibufferConfig, ViewFrame, KeyCode, KeyModifiers) still derive no Serialize, and improvise-command depends only on serde_json. Deferred: nothing serializes ViewState until improvise-cqi builds a Snapshot, so the derives would have no consumer.').
@@ -573,6 +580,7 @@ decision('improvise-gtn7.1',scroll_measured_where_drawn,'A scroll offset is chos
decision('improvise-cgqt',import_collisions_are_asked_not_defaulted,'PR #304, merge 4f38a35e. When records would write the same cell, the import wizard asks: ResolveDuplicates offers sum, keep separate under a new category, or go back, and appears only when ImportPipeline::collisions() is non-empty, after the field and date steps and before DefineFormulas. Silently summing was improvise-2cw9 fix for last-row-wins, and it too was a default the user never chose. The headless --no-wizard import takes the answer as --on-duplicate refuse|sum|separate. context/code/formula-and-io.md, Colliding records are a question, not a default.').
decision('improvise-cgqt',occurrence_category_goes_on_axis_none,'Under DuplicatePolicy::Separate the build adds the occurrence category with items 1..=deepest collision and sets it to Axis::None in the active view, so the default grid still shows each tuple total and nothing new is pivoted. A record that collides with nothing is occurrence 1 (improvise-cgqt, improvise-9bit).').
decision('improvise-felo.1',category_cap_has_one_home,'The regular-category cap is improvise_core::model::MAX_CATEGORIES and nothing else. Until felo.1 the import wizard kept a private const of its own, so raising the model number alone would have left the wizard refusing a 13th category the model accepts -- a derivation stated twice that agreed only by coincidence. The only place the literal 24 is spelled is the test add_category_max_limit; every other test that fills to the cap derives it. No recorded reason for 12 was found; the user raised it on 2026-09-25 (12 categories is feeling small, we should increase to 24).').
decision('improvise-felo.13',skip_rebuild_keyed_on_declared_model_effects_plus_observed_view_and_drill,'Renamed from skip_rebuild_keyed_on_observed_inputs_not_effect_kind, whose name overstated. App::refresh_layout skips the rebuild unless layout_stale (DECLARED: set from EffectKind::Model(_) in apply_effects, and by install_workbook), or the drawn view name or the drill snapshot pointer (OBSERVED, compared as LayoutBasis) moved. It is not a hand-kept list of effects, the shape cursor_epoch and document_generation avoid, but it inherits gotcha 3 classification hole: a ViewOnly effect that changes what the layout reads leaves app.layout stale, where before felo.13 every batch rebuilt. A writer outside effects must call rebuild_layout itself (gotcha 23).').
wire_carries(command,both_directions,'user intent up, projection commands down').
wire_carries(snapshot,on_connect_only,'improvise-q08: full ViewState + render cache filled from current viewport').
wire_carries(command,both_directions,'user intent client->server, projections server->client').
+2
View File
@@ -81,6 +81,8 @@ const SECTIONS = [
["context/test-evidence/cli.md", "test-evidence/cli", "The headless CLI"],
["context/test-evidence/gp.md", "test-evidence/gp", "gp-tree and improvise-gp"],
["context/test-evidence/tooling.md", "test-evidence/tooling", "The repository's own gates"],
["context/test-evidence/layout-cost.md", "test-evidence/layout-cost", "Layout and keypress cost"],
["context/test-evidence/scale-bisect.md", "test-evidence/scale-bisect", "Scale bisect"],
],
},
{
+76 -9
View File
@@ -38,6 +38,8 @@ trait Runnable {
fn run(self, model_file: Option<PathBuf>) -> Result<()>;
}
// Parsed once per process, so a variant's size costs nothing worth boxing for.
#[allow(clippy::large_enum_variant)]
#[derive(Subcommand)]
#[enum_dispatch(Runnable)]
enum Commands {
@@ -72,10 +74,16 @@ struct ImportArgs {
#[arg(long)]
skip: Vec<String>,
/// Extract date component, e.g. "Date:Month" (repeatable)
/// Extract date component, e.g. "Date:Month" (repeatable). The components
/// nest in ONE category named after the column unless --split-date names it
#[arg(long)]
extract: Vec<String>,
/// Split a date field's components into one flat category each, beside its
/// raw values, rather than one hierarchical category (repeatable)
#[arg(long)]
split_date: Vec<String>,
/// Set category axis, e.g. "Payee:row" (repeatable)
#[arg(long)]
axis: Vec<String>,
@@ -97,6 +105,10 @@ struct ImportArgs {
#[arg(long, value_enum, default_value_t = OnDuplicate::Refuse)]
on_duplicate: OnDuplicate,
/// With --no-wizard, refuse (and write nothing) if any record is skipped
#[arg(long)]
strict: bool,
/// Save to file instead of opening TUI
#[arg(short, long)]
output: Option<PathBuf>,
@@ -153,10 +165,12 @@ impl Runnable for ImportArgs {
time_fields: self.time,
skip_fields: self.skip,
extractions: parse_colon_pairs(&self.extract),
split_dates: self.split_date,
axes: parse_colon_pairs(&self.axis),
formulas: self.formula,
name: self.name,
on_duplicate: self.on_duplicate,
strict: self.strict,
};
if self.no_wizard {
@@ -187,10 +201,12 @@ struct ImportConfig {
time_fields: Vec<String>,
skip_fields: Vec<String>,
extractions: Vec<(String, String)>,
split_dates: Vec<String>,
axes: Vec<(String, String)>,
formulas: Vec<String>,
name: Option<String>,
on_duplicate: OnDuplicate,
strict: bool,
}
fn parse_colon_pairs(args: &[String]) -> Vec<(String, String)> {
@@ -205,7 +221,7 @@ fn parse_colon_pairs(args: &[String]) -> Vec<(String, String)> {
use import::wizard::{DuplicatePolicy, ImportPipeline};
fn apply_config_to_pipeline(pipeline: &mut import::wizard::ImportPipeline, config: &ImportConfig) {
use import::analyzer::{DateComponent, FieldKind};
use import::analyzer::{DateComponent, DateShape, FieldKind};
// Override field kinds
for p in &mut pipeline.proposals {
@@ -238,6 +254,13 @@ fn apply_config_to_pipeline(pipeline: &mut import::wizard::ImportPipeline, confi
}
}
// The wizard's `s`, headless: which date fields split rather than nest.
for p in &mut pipeline.proposals {
if config.split_dates.contains(&p.field) {
p.date_shape = DateShape::Split;
}
}
// Set formulas
pipeline.formulas = config.formulas.clone();
@@ -288,6 +311,23 @@ fn apply_axis_overrides(wb: &mut Workbook, axes: &[(String, String)]) {
}
}
/// Why `--axis` is refused: the categories the import did build, and a date
/// hint only for a name shaped like one of the split date parts.
fn axis_refusal(cat: &str, built: &[String]) -> String {
let date_part = ["_Year", "_Month", "_Quarter"]
.iter()
.any(|suffix| cat.ends_with(suffix));
let hint = if date_part {
" A date's parts are one category unless --split-date names it."
} else {
""
};
format!(
"--axis names '{cat}', which this import did not build.{hint} Built: {}",
built.join(", ")
)
}
fn run_headless_import(
mut pipeline: ImportPipeline,
config: &ImportConfig,
@@ -295,6 +335,19 @@ fn run_headless_import(
model_file: Option<PathBuf>,
) -> Result<()> {
apply_config_to_pipeline(&mut pipeline, config);
// A strict skip refuses before the build, so nothing is written (8jk6.6).
if let (true, Some(skipped)) = (config.strict, pipeline.skip_warning()) {
anyhow::bail!("--strict: {skipped}");
}
if let Some(field) = config.split_dates.iter().find(|f| {
!pipeline.proposals.iter().any(|p| {
&p.field == *f
&& p.kind == import::analyzer::FieldKind::TimeCategory
&& p.date_format.is_some()
})
}) {
anyhow::bail!("--split-date '{field}' is not a date column of this import");
}
if pipeline.on_duplicate == DuplicatePolicy::Refuse {
let collisions = pipeline.collisions();
if !collisions.is_empty() {
@@ -305,6 +358,16 @@ fn run_headless_import(
for warning in &warnings {
eprintln!("warning: {warning}");
}
if let Some((cat, _)) = config
.axes
.iter()
.find(|(cat, _)| wb.model.category(cat).is_none())
{
anyhow::bail!(axis_refusal(
cat,
&wb.model.categories.keys().cloned().collect::<Vec<_>>()
));
}
apply_axis_overrides(&mut wb, &config.axes);
if let Some(path) = output.or(model_file) {
@@ -381,7 +444,7 @@ fn get_import_data(args: &[PathBuf]) -> Option<ImportPipeline> {
Ok(content) => {
if csv_path_p(path) {
match crate::import::csv_parser::parse_csv(path) {
Ok(records) => Some(ImportPipeline::new(Value::Array(records))),
Ok(records) => Some(ImportPipeline::from_csv(records)),
Err(e) => {
eprintln!("CSV parse error: {e}");
None
@@ -422,12 +485,16 @@ fn run_headless_commands(cmds: &[String], file: &Option<PathBuf>) -> Result<()>
for line in cmds {
// The whole line is syntax-checked before any of it runs, exactly as
// before; only CONSTRUCTION moved — `context/code/cli.md`.
if let Err(e) = command::parse_line(line) {
eprintln!("Parse error: {e}");
exit_code = 1;
continue;
}
for segment in command::parse::segments(line) {
let segments = command::parse_line(line).and_then(|_| command::parse::segments(line));
let segments = match segments {
Ok(segments) => segments,
Err(e) => {
eprintln!("Parse error: {e}");
exit_code = 1;
continue;
}
};
for segment in segments {
let mut failed = None;
let effects = {
// Built against the cursor it is about to run against, so a
+7 -4
View File
@@ -180,7 +180,10 @@ impl<'a> Widget for ImportWizardWidget<'a> {
if y >= inner.bottom().saturating_sub(2) {
break;
}
let header = format!(" {} (format: {})", proposal.field, proposal.format);
let header = format!(
" {} (format: {}) — {}",
proposal.field, proposal.format, proposal.shape
);
panel.put(
x,
y,
@@ -221,7 +224,7 @@ impl<'a> Widget for ImportWizardWidget<'a> {
x,
hint_y,
w,
"Space: toggle Enter: next \u{2190} back Esc: cancel",
"Space: toggle s: one category / split Enter: next \u{2190} back Esc: cancel",
Style::default().fg(Color::DarkGray),
);
}
@@ -1178,7 +1181,7 @@ mod tests {
"ConfigureDates\n",
"┌ Import Wizard — Date Components ─────────────────────────────────────────────┐\n",
"│Select date components to extract (Space toggle): │\n",
"│ When (format: %Y-%m-%d) │\n",
"│ When (format: %Y-%m-%d) — one category of the raw dates │\n",
"│ [ ] Year │\n",
"│ [ ] Month │\n",
"│ [ ] Quarter │\n",
@@ -1196,7 +1199,7 @@ mod tests {
"│ │\n",
"│ │\n",
"│ │\n",
"│Space: toggle Enter: next ← back Esc: cancel │\n",
"│Space: toggle s: one category / split Enter: next ← back Esc: cancel │\n",
"└──────────────────────────────────────────────────────────────────────────────┘\n",
"\n",
"====\n",
+5 -5
View File
@@ -64,8 +64,8 @@ fn duplicate_csv_headers_not_silent() {
2,
"both columns must survive as separate fields: {fields:?}"
);
assert_eq!(fields["A"].as_f64(), Some(1.0), "{fields:?}");
assert_eq!(fields["A_2"].as_f64(), Some(2.0), "{fields:?}");
assert_eq!(fields["A"], "1", "{fields:?}");
assert_eq!(fields["A_2"], "2", "{fields:?}");
assert!(
warnings
.iter()
@@ -78,7 +78,7 @@ fn duplicate_csv_headers_not_silent() {
fn blank_measure_still_infers_numeric() {
use improvise_io::import::{analyzer::FieldKind, wizard::ImportPipeline};
let (records, _) = parse_csv_bytes_with_warnings(b"Region,Revenue\nEast,10\nWest,\n").unwrap();
let p = ImportPipeline::new(serde_json::Value::Array(records));
let p = ImportPipeline::from_csv(records);
let proposal = p.proposals.iter().find(|p| p.field == "Revenue").unwrap();
assert_eq!(proposal.kind, FieldKind::Measure);
}
@@ -87,7 +87,7 @@ fn repeated_import_coordinates_not_silently_overwritten() {
use improvise_io::import::wizard::ImportPipeline;
let (records, _) =
parse_csv_bytes_with_warnings(b"Region,Revenue\nEast,10\nEast,20\n").unwrap();
let mut pipeline = ImportPipeline::new(serde_json::Value::Array(records));
let mut pipeline = ImportPipeline::from_csv(records);
pipeline.on_duplicate = improvise_io::import::wizard::DuplicatePolicy::Sum;
let (wb, warnings) = pipeline.build_model_with_warnings().unwrap();
let values: Vec<f64> = wb
@@ -400,7 +400,7 @@ fn ordinary_controls() {
let (records, warnings) =
parse_csv_bytes_with_warnings(b"Region,Revenue\nEast,10\nWest,20\n").unwrap();
assert!(warnings.is_empty());
let p = ImportPipeline::new(serde_json::Value::Array(records));
let p = ImportPipeline::from_csv(records);
assert_eq!(
p.proposals
.iter()
+399
View File
@@ -121,6 +121,58 @@ fn a_parse_error_still_exits_non_zero() {
);
}
/// improvise-31oe.2 at the process boundary, with a control that exits 0.
/// Evidence: `context/test-evidence/cli.md`.
#[test]
fn an_unknown_category_axis_or_item_exits_non_zero_and_names_it() {
let scratch = Scratch::new("unknown_names");
let doc = seeded(&scratch, "m.improv");
let out = run_cmd(&["add-item Region East"], &doc);
assert!(out.status.success(), "the seed item: {}", stderr_of(&out));
for (line, missing) in [
("set-axis Regoin row", "Regoin"),
("set-page Region Nowhere", "Nowhere"),
("set-axis Region sideways", "sideways"),
] {
let out = run_cmd(&[line], &doc);
let said = stderr_of(&out);
assert!(!out.status.success(), "`{line}` exited 0; stderr={said:?}");
assert!(
said.contains(missing),
"`{line}` did not name {missing}: {said:?}"
);
}
let out = run_cmd(&["set-page Region East"], &doc);
assert!(out.status.success(), "the control: {}", stderr_of(&out));
}
/// improvise-31oe.3: one pipe-quoted argument, and a surplus one refused.
/// Evidence: `context/test-evidence/cli.md`.
#[test]
fn a_pipe_quoted_name_reaches_the_document_whole() {
let scratch = Scratch::new("pipes");
let doc = seeded(&scratch, "m.improv");
let out = run_cmd(&["create-view |By area|"], &doc);
assert!(out.status.success(), "{}", stderr_of(&out));
let text = std::fs::read_to_string(&doc).expect("the document");
assert!(text.contains("## View: By area\n"), "{text}");
assert!(
!text.contains("|By"),
"a pipe reached the view name: {text}"
);
let out = run_cmd(&["create-view By area"], &doc);
let said = stderr_of(&out);
assert!(
!out.status.success(),
"a surplus argument exited 0: {said:?}"
);
assert!(
said.contains("\"area\""),
"the refusal does not name it: {said:?}"
);
}
/// Diagnostics go to stderr so that stdout stays free for a data output, and
/// so `2>/dev/null` is the whole suppression story. `context/code/cli.md`.
#[test]
@@ -613,6 +665,167 @@ fn a_headless_import_keeps_duplicates_separate_when_told_to() {
assert!(!doc.contains("= 30"), "{doc}");
}
/// Dates across two years, one per month, so nothing collides.
fn dated_csv(scratch: &Scratch) -> PathBuf {
let csv = scratch.path("dated.csv");
std::fs::write(
&csv,
"Date,Revenue\n2024-01-15,1\n2024-04-02,10\n2025-01-03,100\n",
)
.unwrap();
csv
}
/// improvise-8jk6.3, headless: `--extract` builds ONE hierarchical category by
/// default, as the wizard does, and it is written with its `>` lines.
#[test]
fn a_headless_date_import_is_one_hierarchical_category() {
let scratch = Scratch::new("date_hierarchy");
let out_path = scratch.path("out.improv");
let out = run_import(
&dated_csv(&scratch),
&out_path,
&[
"--extract",
"Date:Year",
"--extract",
"Date:Quarter",
"--extract",
"Date:Month",
],
);
assert!(out.status.success(), "{}", stderr_of(&out));
let doc = std::fs::read_to_string(&out_path).unwrap();
assert!(!doc.contains("Date_Month"), "{doc}");
assert!(doc.contains("- |2024-01|[|2024-Q1|]"), "{doc}");
assert!(doc.contains("> |2024-Q1|[|2024|]"), "{doc}");
assert!(!doc.contains("expanded"), "it opens folded: {doc}");
}
/// The flat split stays available headless, as `s` makes it in the wizard.
#[test]
fn a_headless_date_import_splits_when_told_to() {
let scratch = Scratch::new("date_split");
let out_path = scratch.path("out.improv");
let out = run_import(
&dated_csv(&scratch),
&out_path,
&["--extract", "Date:Month", "--split-date", "Date"],
);
assert!(out.status.success(), "{}", stderr_of(&out));
let doc = std::fs::read_to_string(&out_path).unwrap();
assert!(doc.contains("## Category: Date_Month"), "{doc}");
assert!(doc.contains("2024-01-15"), "the raw dates stay: {doc}");
}
/// improvise-8jk6.3 review: a `--split-date` that names nothing splittable
/// said nothing and exited 0.
#[test]
fn split_date_naming_an_unknown_or_non_date_field_is_refused() {
let scratch = Scratch::new("split_date_unknown");
for field in ["Nope", "Revenue"] {
let out_path = scratch.path("out.improv");
let out = run_import(
&dated_csv(&scratch),
&out_path,
&["--extract", "Date:Month", "--split-date", field],
);
assert!(!out.status.success(), "{field}: {}", stderr_of(&out));
let err = stderr_of(&out);
assert!(err.contains("--split-date") && err.contains(field), "{err}");
}
}
/// improvise-8jk6.3 review: `--axis Date_Month:column` names a category the
/// hierarchical import no longer builds, and was dropped without a word.
#[test]
fn an_axis_naming_a_category_the_import_did_not_build_is_refused() {
let scratch = Scratch::new("axis_unknown");
let out_path = scratch.path("out.improv");
let out = run_import(
&dated_csv(&scratch),
&out_path,
&["--extract", "Date:Month", "--axis", "Date_Month:column"],
);
assert!(!out.status.success(), "{}", stderr_of(&out));
let err = stderr_of(&out);
assert!(
err.contains("Date_Month") && err.contains("--split-date"),
"{err}"
);
}
/// `--time` on a column with no date format makes it a time category without
/// dates, so there is nothing to split.
#[test]
fn split_date_on_a_time_field_with_no_date_format_is_refused() {
let scratch = Scratch::new("split_date_no_format");
let out_path = scratch.path("out.improv");
let out = run_import(
&dated_csv(&scratch),
&out_path,
&["--time", "Revenue", "--split-date", "Revenue"],
);
assert!(!out.status.success(), "{}", stderr_of(&out));
assert!(
stderr_of(&out).contains("--split-date"),
"{}",
stderr_of(&out)
);
}
/// A name that is no date part gets the categories that exist, never the date
/// hint (the README's old `--axis Measure:column` was this).
#[test]
fn an_absent_axis_category_that_is_no_date_part_names_what_was_built() {
let scratch = Scratch::new("axis_unknown_plain");
let out_path = scratch.path("out.improv");
let out = run_import(
&dated_csv(&scratch),
&out_path,
&["--axis", "Measure:column"],
);
assert!(!out.status.success(), "{}", stderr_of(&out));
let err = stderr_of(&out);
assert!(
err.contains("'Measure'") && err.contains("_Measure"),
"{err}"
);
assert!(!err.contains("--split-date"), "no date hint: {err}");
}
/// The README's import recipe runs as printed.
#[test]
fn the_readmes_import_recipe_runs() {
let root = Path::new(env!("CARGO_MANIFEST_DIR"));
let readme = std::fs::read_to_string(root.join("README.md")).unwrap();
let start = readme.find("improvise import examples/demo.csv").unwrap();
let recipe = readme[start..]
.split("```")
.next()
.unwrap()
.replace("\\\n", " ");
let mut args: Vec<String> = vec![];
for (i, part) in recipe.split('"').enumerate() {
match i % 2 {
1 => args.push(part.to_string()),
_ => args.extend(part.split_whitespace().map(String::from)),
}
}
let at = args.iter().position(|a| a == "-o").unwrap();
args.truncate(at);
let scratch = Scratch::new("readme_recipe");
let out = Command::new(bin())
.arg("import")
.arg(root.join("examples/demo.csv"))
.args(args.iter().skip(3))
.arg("-o")
.arg(scratch.path("out.improv"))
.output()
.unwrap();
assert!(out.status.success(), "{}", stderr_of(&out));
}
/// code-maat's reports: three schemas, a stray note, and room for the output.
fn code_maat_folder(scratch: &Scratch) -> PathBuf {
let dir = scratch.path("maat");
@@ -709,6 +922,192 @@ fn a_headless_import_takes_files_and_folders_together_and_refuses_an_empty_folde
assert!(!scratch.path("never.improv").exists());
}
/// Runs `import <source> --no-wizard <extra> -o out.improv` on a source it writes.
fn import_of(scratch: &Scratch, name: &str, body: &str, extra: &[&str]) -> (Output, PathBuf) {
let source = scratch.path(name);
std::fs::write(&source, body).unwrap();
let out_path = scratch.path("out.improv");
(run_import(&source, &out_path, extra), out_path)
}
/// improvise-8jk6.4: a forced category's values are item names, so they keep
/// their source text. Evidence: `context/test-evidence/cli.md`.
#[test]
fn a_csv_column_forced_to_category_keeps_its_text_verbatim() {
let scratch = Scratch::new("forced_category_text");
let (out, out_path) = import_of(
&scratch,
"commits.csv",
"commit,lines\n00565796,3\n220944e2,5\n",
&["--category", "commit", "--measure", "lines"],
);
assert!(out.status.success(), "{}", stderr_of(&out));
let doc = std::fs::read_to_string(&out_path).unwrap();
assert!(
doc.lines().any(|l| l == "- |00565796|, |220944e2|"),
"the items are the source text:\n{doc}"
);
for line in [
"_Measure=lines, commit=|00565796| = 3",
"_Measure=lines, commit=|220944e2| = 5",
] {
assert!(doc.lines().any(|l| l == line), "{line} missing from\n{doc}");
}
}
/// improvise-8jk6.4: JSON has no source text for a number, so a forced
/// category names it as serde_json writes it. Evidence file.
#[test]
fn a_json_number_forced_to_category_is_named_as_json_writes_it() {
let scratch = Scratch::new("forced_category_json");
let (out, out_path) = import_of(
&scratch,
"commits.json",
r#"[{"commit": 42, "n": 1}, {"commit": 1.5, "n": 2}, {"commit": 1e3, "n": 3}]"#,
&["--category", "commit", "--measure", "n"],
);
assert!(out.status.success(), "{}", stderr_of(&out));
let doc = std::fs::read_to_string(&out_path).unwrap();
assert!(
doc.lines().any(|l| l == "- |42|, |1.5|, |1000.0|"),
"the items are the numbers' JSON text:\n{doc}"
);
}
/// improvise-8jk6.5: a formula the user named and that does not parse is a
/// refusal, not a silent drop. Evidence: `context/test-evidence/cli.md`.
#[test]
fn an_import_formula_that_does_not_parse_refuses_names_it_and_writes_nothing() {
let scratch = Scratch::new("bad_formula");
let (out, out_path) = import_of(
&scratch,
"f.csv",
"a,x\nk,1\n",
&[
"--measure",
"x",
"--formula",
"Y = x +",
"--formula",
"Z = x",
"--formula",
"W = * 2",
],
);
let err = stderr_of(&out);
assert!(!out.status.success(), "a dropped formula exited 0: {err}");
assert!(err.contains("'Y = x +'"), "names the formula: {err}");
assert!(err.contains("'W = * 2'"), "names every bad one: {err}");
assert!(err.contains("expected"), "carries the parse error: {err}");
assert!(!err.contains("'Z = x'"), "names only the bad one: {err}");
assert!(!out_path.exists(), "nothing is written");
}
/// improvise-8jk6.6: an import that kept no record says why, and exits
/// non-zero writing nothing. Evidence: `context/test-evidence/cli.md`.
#[test]
fn an_import_that_keeps_no_record_refuses_and_says_why() {
let scratch = Scratch::new("kept_none");
let (out, out_path) = import_of(&scratch, "r.json", r#"[1, 2, "three"]"#, &[]);
let err = stderr_of(&out);
assert!(!out.status.success(), "{err}");
assert!(
err.contains("all 3 records were skipped as not JSON objects"),
"says why: {err}"
);
assert!(!out_path.exists(), "nothing is written");
}
/// improvise-8jk6.6: a partial skip warns with a count and exits 0, a choice
/// put to the user. Evidence: `context/test-evidence/cli.md`.
#[test]
fn an_import_that_skips_some_records_warns_with_a_count_and_still_writes() {
let scratch = Scratch::new("kept_some");
let (out, out_path) = import_of(
&scratch,
"p.json",
r#"[{"a": "k", "x": 1}, 2, {"a": "m", "x": 4}]"#,
&[],
);
let err = stderr_of(&out);
assert!(out.status.success(), "{err}");
assert!(
err.contains("warning: Skipped 1 of 3 records that were not JSON objects"),
"{err}"
);
let doc = std::fs::read_to_string(&out_path).unwrap();
assert!(doc.contains("_Measure=x, a=m = 4"), "{doc}");
}
/// A JSON array with one stray scalar among its records.
const ONE_STRAY: &str = r#"[{"a": "k", "x": 1}, 2, {"a": "m", "x": 4}]"#;
/// improvise-8jk6.6, the user's ruling: `--strict` makes any skip a refusal
/// that writes nothing. Evidence: `context/test-evidence/cli.md`.
#[test]
fn a_strict_import_that_skips_a_record_refuses_and_writes_nothing() {
let scratch = Scratch::new("strict_skip");
let (out, out_path) = import_of(&scratch, "p.json", ONE_STRAY, &["--strict"]);
let err = stderr_of(&out);
assert!(!out.status.success(), "{err}");
assert!(
err.contains("--strict: Skipped 1 of 3 records that were not JSON objects"),
"{err}"
);
assert!(!out_path.exists(), "a strict refusal writes nothing");
}
/// The control: `--strict` refuses a skip, not every import.
#[test]
fn a_strict_import_that_skips_nothing_still_writes() {
let scratch = Scratch::new("strict_clean");
let body = r#"[{"a": "k", "x": 1}, {"a": "m", "x": 4}]"#;
let (out, out_path) = import_of(&scratch, "p.json", body, &["--strict"]);
assert!(out.status.success(), "{}", stderr_of(&out));
let doc = std::fs::read_to_string(&out_path).unwrap();
assert!(doc.contains("_Measure=x, a=m = 4"), "{doc}");
}
/// improvise-8jk6.17 (F3): a CSV decimal measure is its number, not dropped
/// text. Evidence: `context/test-evidence/cli.md`.
#[test]
fn a_csv_decimal_measure_imports_as_its_number() {
let scratch = Scratch::new("csv_decimal");
let body = "Region,Revenue\nEast,1000.50\nWest,-2.25\n";
let (out, out_path) = import_of(&scratch, "f.csv", body, &[]);
assert!(out.status.success(), "{}", stderr_of(&out));
let doc = std::fs::read_to_string(&out_path).unwrap();
for line in [
"Region=East, _Measure=Revenue = 1000.5",
"Region=West, _Measure=Revenue = -2.25",
] {
assert!(doc.lines().any(|l| l == line), "{line} missing from\n{doc}");
}
}
/// improvise-8jk6.17 (F1): `import-json` reads a CSV as CSV, so its numeric
/// column is a measure. Evidence: `context/test-evidence/cli.md`.
#[test]
fn import_json_on_a_csv_reads_its_numeric_column_as_a_measure() {
let scratch = Scratch::new("import_json_csv");
let csv = scratch.path("f.csv");
std::fs::write(&csv, "Region,Revenue\nEast,1000.50\nWest,7\n").unwrap();
let doc = scratch.path("m.improv");
let out = run_cmd(&[&format!("import-json {}", csv.display())], &doc);
assert!(out.status.success(), "{}", stderr_of(&out));
let text = std::fs::read_to_string(&doc).unwrap();
assert!(!text.contains("## Category: Revenue"), "{text}");
for line in [
"Region=East, _Measure=Revenue = 1000.5",
"Region=West, _Measure=Revenue = 7",
] {
assert!(
text.lines().any(|l| l == line),
"{line} missing from\n{text}"
);
}
}
/// improvise-lpzu, the user's ruling: a folder always merges, so even one CSV
/// keeps its file identity. Evidence: `context/test-evidence/cli.md`.
#[test]
+1 -1
View File
@@ -18,7 +18,7 @@ fn imported_model_shows_one_row_per_record_with_measure_columns() {
.expect("write csv");
let records = improvise::import::csv_parser::parse_csv(&csv).expect("csv parses");
let pipeline = ImportPipeline::new(serde_json::Value::Array(records));
let pipeline = ImportPipeline::from_csv(records);
let workbook = pipeline.build_model().expect("model builds");
let saved = dir.path().join("w.improv");