787e354b9b
The `build` job fails on main and on every branch merged from it: doc-site: context/code/pr-review.md is neither published nor a named omission NOT a merge loss -- scripts/build-doc-site.mjs in this branch is byte-identical to main's, and neither lists the file. PR #96 added context/code/pr-review.md without a manifest line, which is exactly the gate improvise-res9 added: a doc added without a line publishes nothing and says nothing, so the generator refuses. Omitted rather than published, and the line is cloud-agents.md's rather than tooling.md's. tooling.md explains checkers a reader of the code will meet; pr-review.md is about THIS REPOSITORY'S PROCESS -- where a Codex review appears, how to tell it from our own agents' comments, why absence is not approval. Nobody visiting a website is about to read a PR thread. Both homes updated, because an omission is restated beside the manifest AND in doc-site.md section 2, and the prose there counted them ("Neither is referenced...") so it went stale at three. This strays into scripts/, which is Wave N's ground. Flagged in the hand-back; it is a one-line data entry either way if the orchestrator would rather publish it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
160 lines
61 KiB
HTML
160 lines
61 KiB
HTML
<!doctype html>
|
||
<html lang="en">
|
||
<head>
|
||
<meta charset="utf-8" />
|
||
<meta name="viewport" content="width=device-width, initial-scale=1" />
|
||
<title>improvise-core — improvise docs</title>
|
||
<link rel="stylesheet" href="../style.css" />
|
||
</head>
|
||
<body>
|
||
<div class="shell">
|
||
<nav class="side"><a class="brand" href="../index.html">improvise docs</a><input type="checkbox" id="navtoggle" class="navtoggle" /><label class="navsummary" for="navtoggle">Pages</label><div class="navlinks"><ul><li><a href="../../index.html">← Project home</a></li><li><a href="../../app/">Browser demo</a></li><li><a href="https://github.com/fiddlerwoaroof/improvise">Source on GitHub</a></li></ul><h3>Start here</h3><ul><li><a href="../design-notes.html">Design notes</a></li><li><a href="../design-principles.html">Design principles</a></li><li><a href="../repo-map.html">Repository map</a></li></ul><h3>Decision records</h3><ul><li><a href="../adr/001-concurrent-edits.html">ADR 001 — Concurrent edits</a></li><li><a href="../adr/002-naming-a-cell.html">ADR 002 — Naming a cell</a></li><li><a href="../adr/003-live-backed-models.html">ADR 003 — Live-backed models</a></li></ul><h3>The arguments, by area</h3><ul><li><a href="../code/core.html" class="here">improvise-core</a></li><li><a href="../code/formula-and-io.html">improvise-formula and improvise-io</a></li><li><a href="../code/protocol.html">improvise-protocol</a></li><li><a href="../code/command.html">improvise-command</a></li><li><a href="../code/clients.html">The clients</a></li><li><a href="../code/tui.html">The TUI</a></li><li><a href="../code/gp.html">gp-tree and improvise-gp</a></li><li><a href="../code/kb-tool.html">The kb tool</a></li><li><a href="../code/tooling.html">The repository's own checkers</a></li></ul><h3>Test evidence</h3><ul><li><a href="../test-evidence/core.html">improvise-core</a></li><li><a href="../test-evidence/formula-and-io.html">improvise-formula and improvise-io</a></li><li><a href="../test-evidence/protocol.html">improvise-protocol</a></li><li><a href="../test-evidence/command.html">improvise-command</a></li><li><a href="../test-evidence/clients.html">The clients</a></li><li><a href="../test-evidence/tui.html">The TUI</a></li><li><a href="../test-evidence/gp.html">gp-tree and improvise-gp</a></li><li><a href="../test-evidence/tooling.html">The repository's own gates</a></li></ul><h3>About this site</h3><ul><li><a href="../doc-site.html">About this site</a></li></ul><h3>Reference</h3><ul><li><a href="../tickets.html">Ticket index</a></li></ul></div></nav>
|
||
<main>
|
||
<p class="src">Source: <a href="https://github.com/fiddlerwoaroof/improvise/blob/main/context/code/core.md">context/code/core.md</a></p><details class="toc" open><summary>On this page</summary><ul><li class="l1"><a href="#improvise-core-the-arguments">improvise-core — the arguments</a></li><li class="l2"><a href="#1-cellkey-is-canonical-by-construction">1. CellKey is canonical by construction</a></li><li class="l2"><a href="#2-aggregating-a-cell-s-raw-values">2. Aggregating a cell's raw values</a></li><li class="l3"><a href="#evaluate-aggregated-takes-aggregation-not-a-list-it-cannot-read">evaluate_aggregated takes Aggregation, not a list it cannot read</a></li><li class="l3"><a href="#non-numeric-values-reduce-by-unanimity-and-the-branch-order-is-the-rule">Non-numeric values reduce by unanimity, and the branch order is the rule</a></li><li class="l2"><a href="#3-formula-evaluation">3. Formula evaluation</a></li><li class="l2"><a href="#4-float-equality-has-one-epsilon-and-the-div-0-guard-shares-it">4. Float equality has one epsilon, and the div/0 guard shares it</a></li><li class="l2"><a href="#5-surrogate-identities-model-record-rs-adr-002">5. Surrogate identities — model/record.rs (ADR 002)</a></li><li class="l2"><a href="#6-records-mode-is-wide-format-view-layout-rs">6. Records mode is wide format (view/layout.rs)</a></li><li class="l3"><a href="#prune-axis-items-takes-a-predicate-not-a-flag-vector">prune_axis_items takes a predicate, not a flag vector</a></li><li class="l2"><a href="#7-axes-page-and-filter-are-two-different-questions">7. Axes: Page and Filter are two different questions</a></li><li class="l3"><a href="#on-category-added-a-virtual-on-an-axis-is-a-configuration-not-a-vacancy">on_category_added: a virtual on an axis is a configuration, not a vacancy</a></li><li class="l3"><a href="#known-defect-two-answers-to-is-this-name-a-virtual">Known defect: two answers to "is this name a virtual"</a></li><li class="l2"><a href="#8-ordering-is-correctness-not-tidiness-model-cell-rs">8. Ordering is correctness, not tidiness (model/cell.rs)</a></li><li class="l3"><a href="#the-record-index-recordentry">The record index: RecordEntry</a></li><li class="l2"><a href="#9-categories">9. Categories</a></li><li class="l2"><a href="#10-workbook-exists-to-break-the-model-view-cycle">10. Workbook exists to break the Model ↔ View cycle</a></li><li class="l2"><a href="#11-virtual-views">11. Virtual views</a></li><li class="l2"><a href="#12-axis-of-panics-try-axis-of-does-not">12. axis_of panics, try_axis_of does not</a></li><li class="l3"><a href="#record-cell-key-is-a-match-and-the-match-is-the-guard-improvise-ir7l">record_cell_key is a match, and the match is the guard (improvise-ir7l)</a></li><li class="l2"><a href="#13-route-then-aggregate-sum-folds-over-the-member-records">13. Route then aggregate: SUM(<expr>) folds over the member records</a></li><li class="l2"><a href="#pageaxis-what-constrains-and-what-the-user-may-turn-improvise-p1ym">PageAxis — what constrains, and what the user may turn (improvise-p1ym)</a></li><li class="l2"><a href="#the-number-format-format-rs-improvise-711n">The number format — format.rs (improvise-711n)</a></li></ul></details><h1 id="improvise-core-the-arguments"><code>improvise-core</code> — the arguments</h1>
|
||
<p>Written for a reader with the code open. This is where the reasoning that used to live in <code>crates/improvise-core/**</code>'s comments now lives; the comments themselves are capped at two lines (design-principles §6). Bug narratives keyed by test name are in <a href="../test-evidence/core.html"><code>context/test-evidence/core.md</code></a>.</p>
|
||
<p>The crate is pure data: <code>Model</code>, <code>View</code>, <code>GridLayout</code>, <code>Workbook</code>, number formatting. It depends on <code>improvise-formula</code> for AST types and knows nothing about UI, IO or commands.</p>
|
||
<hr />
|
||
<h2 id="1-cellkey-is-canonical-by-construction"><a class="anchor" href="#1-cellkey-is-canonical-by-construction">#</a>1. <code>CellKey</code> is canonical by construction</h2>
|
||
<p><code>CellKey(Vec<(category, item)>)</code> is <strong>always sorted by category name</strong>, and the constructors are the only way to get one: <code>CellKey::new(coords)</code> sorts, <code>with(cat, item)</code> replaces-or-inserts-and-re-sorts, <code>without(cat)</code> filters and so preserves the order. Equality, hashing and storage all rely on the canonical form, so two keys naming the same intersection in different orders <em>are</em> the same key. Never build the inner <code>Vec</code> directly (gotcha 5).</p>
|
||
<p>That <code>without</code> preserves the sort is what lets <code>record_of</code> be a one-liner rather than a normalisation step — see §5.</p>
|
||
<h2 id="2-aggregating-a-cell-s-raw-values"><a class="anchor" href="#2-aggregating-a-cell-s-raw-values">#</a>2. Aggregating a cell's raw values</h2>
|
||
<p>Three functions, deliberately with three different emptiness rules, and the differences are the whole content of the design (<a class="tid" href="../tickets.html#improvise-kdp" title="Duplicated aggregation dispatch block">improvise-kdp</a>):</p>
|
||
<ul>
|
||
<li><strong><code>apply_agg(values: &[f64], func)</code></strong> — reduces numbers. <code>None</code> on empty: an empty cell reads as <em>absent</em>, not as zero, or every blank intersection in a pivot would render <code>0</code>. Both aggregation paths (<code>evaluate_aggregated</code> and <code>aggregate_raw</code>) go through it so their semantics cannot drift.</li>
|
||
<li><strong><code>aggregate_cell_values(values: &[&CellValue], func)</code></strong> — reduces a mixed set. See below.</li>
|
||
<li><strong><code>eval_agg(...)</code></strong> — evaluates a formula-level <code>SUM(x)</code> / <code>AVG(x WHERE …)</code>. Its emptiness rule is <em>not</em> <code>apply_agg</code>'s: an aggregate over no rows is <code>0</code> for <code>SUM</code>/<code>COUNT</code>, which have an identity, and an error for <code>AVG</code>/<code>MIN</code>/<code>MAX</code>, which do not.</li>
|
||
</ul>
|
||
<h3 id="evaluate-aggregated-takes-aggregation-not-a-list-it-cannot-read"><a class="anchor" href="#evaluate-aggregated-takes-aggregation-not-a-list-it-cannot-read">#</a><code>evaluate_aggregated</code> takes <code>Aggregation</code>, not a list it cannot read</h3>
|
||
<p>The parameter was <code>none_cats: &[String]</code>, and the body consulted it exactly once, for emptiness — a boolean wearing a list's type, since the aggregation itself runs over <code>key.0</code> through <code>matching_values</code>. It never filtered by that list, in any revision: <code>git log -S</code> back to the commit that introduced <code>Axis::None</code> shows the same <code>is_empty()</code> and nothing else. So the name promised a filter the body has never performed, and the next caller to build a careful list would have threaded it into a parameter that cannot use it.</p>
|
||
<p><code>Aggregation::{Exact, OverHidden}</code> admits only the two states the body distinguishes, and <code>Aggregation::of(none_cats)</code> performs the reduction once, at the boundary that holds the list (<a class="tid" href="../tickets.html#improvise-5jm5" title="Model::evaluate_aggregated takes a list it only tests for emptiness">improvise-5jm5</a>, <code>doctrine(an_invariant_is_made_unrepresentable_not_validated)</code> — the same answer <code>prune_axis_items</code> and <code>gp-tree</code>'s <code>Row<T></code> got). It is an enum rather than a <code>bool</code> because the two states have names worth reading at the call site, and the regression guard is the <strong>signature</strong>: the thing the ticket describes — two different lists, same emptiness, same answer — is no longer constructible.</p>
|
||
<p>Note what does <em>not</em> change: <code>recompute_formulas</code> and <code>aggregate_raw</code> keep <code>&[String]</code>, because they walk the elements (stripping each hidden coordinate off a stem). Two parameters that looked alike were doing two different jobs.</p>
|
||
<h3 id="non-numeric-values-reduce-by-unanimity-and-the-branch-order-is-the-rule"><a class="anchor" href="#non-numeric-values-reduce-by-unanimity-and-the-branch-order-is-the-rule">#</a>Non-numeric values reduce by unanimity, and the branch order is the rule</h3>
|
||
<p><code>aggregate_cell_values</code> checks the <strong>all-numeric branch first</strong>. If every value is a number it reduces through <code>apply_agg</code>; otherwise, if every value equals the first, the result is that value; otherwise it is <code>CellValue::Error("mixed")</code>.</p>
|
||
<p>That ordering is the entire difference between this rule and "a single value passes through". <code>Count</code> is the only function that can tell them apart — every other function is the identity on a one-element set — so <code>Count</code> of a lone number must still be <code>1</code> and not the number itself. <code>a_lone_number_still_counts_ as_one_rather_than_passing_through</code> is the pin.</p>
|
||
<p>What the rule forbids is <strong>dropping</strong>. The old code did <code>filter_map(as_f64)</code>, and since <code>_Index</code>/<code>_Dim</code> sit on <code>Axis::None</code> in every default view, <code>none_cats</code> is non-empty almost always and the aggregating path was almost always taken — so every <code>CellValue::Text</code> in every document rendered blank, including the pipe-quoted text <code>.improv</code> carries as a first-class value (<code>Region=West, Measure=Revenue = |pending|</code>). A user could type a text value, save it, load it, and never see it again (<a class="tid" href="../tickets.html#improvise-l4qx" title="[bug] A text cell is invisible whenever any category is on the None axis">improvise-l4qx</a>).</p>
|
||
<p>Both <code>evaluate_aggregated</code> and <code>aggregate_raw</code> share the helper, so a formula ref over mixed values now errors <code>"mixed"</code> rather than silently summing the numeric ones and reading as authoritative while under-reporting.</p>
|
||
<h2 id="3-formula-evaluation"><a class="anchor" href="#3-formula-evaluation">#</a>3. Formula evaluation</h2>
|
||
<p><strong>Fixed point.</strong> <code>recompute_formulas(none_cats)</code> clears <code>formula_cache</code>, collects the distinct coordinate <em>stems</em> (each data key with the formula's target category and every <code>none_cat</code> stripped), and iterates <code>stem × formula target</code> until nothing changes, bounded by <code>MAX_EVAL_DEPTH = 16</code>. Chained formulas (<code>Margin = Profit / Revenue</code> over <code>Profit = Revenue - Cost</code>) converge naturally; circular references converge to <code>CellValue::Error("circular")</code> rather than overflowing the stack. The stems are an <code>IndexSet</code>: dedupe is O(1) per cell and insertion order is preserved (<a class="tid" href="../tickets.html#improvise-6os" title="O(n^2) stem deduplication in recompute_formulas">improvise-6os</a> replaced an O(n²) <code>Vec::contains</code>).</p>
|
||
<p>There are <strong>two</strong> evaluators, and they must be changed together:</p>
|
||
<ul>
|
||
<li><code>eval_formula_depth</code> / <code>eval_expr</code> — the direct, uncached path, recursion bounded by <code>depth</code>.</li>
|
||
<li><code>eval_formula_with_cache</code> / <code>eval_expr_cached</code> — the fixed-point path, which resolves a <code>Expr::Ref</code> through <code>formula_cache</code> first and falls back to <code>aggregate_raw</code>.</li>
|
||
</ul>
|
||
<p>A rule stated in one and not the other is a live bug; <a class="tid" href="../tickets.html#improvise-0bf" title="Inconsistent float-equality tolerance in formula eval">improvise-0bf</a> was exactly that (see §4).</p>
|
||
<p><strong><code>find_item_category</code></strong> searches real categories first and then formula targets, because <code>add_formula</code> never inserts its target into <code>_Measure</code> (<a class="tid" href="../tickets.html#improvise-nrs" title="Duplicated find_item_category helper in eval paths">improvise-nrs</a>). Without the fallback, a reference to a computed measure fails to resolve.</p>
|
||
<p><strong>Display rounding never reaches eval.</strong> <code>format_f64</code> is called only in rendering paths. Feeding a rounded value back into a formula is the bug <code>formula_chain_preserves_full_precision</code> exists to catch.</p>
|
||
<h2 id="4-float-equality-has-one-epsilon-and-the-div-0-guard-shares-it"><a class="anchor" href="#4-float-equality-has-one-epsilon-and-the-div-0-guard-shares-it">#</a>4. Float equality has one epsilon, and the div/0 guard shares it</h2>
|
||
<p><code>FLOAT_EQ_EPSILON = 1e-10</code> is used by <code>=</code>/<code>!=</code> <strong>and</strong> by the division-by-zero guard, in both evaluators. The two must agree or <code>IF(x = 0, a, y / x)</code> is incoherent: the condition calls <code>x</code> zero and the else branch divides by it anyway. With <code>x = 1e-11</code> the old exact <code>rv == 0.0</code> guard returned <code>1e13</code> (<a class="tid" href="../tickets.html#improvise-0bf" title="Inconsistent float-equality tolerance in formula eval">improvise-0bf</a>). <code>1e-10</code> is far below typical data magnitudes but absorbs accumulated f64 rounding noise from chained formulas.</p>
|
||
<p>Note the repo map's gotcha 17 (which called this "currently mixed") describes the <em>pre-fix</em> state; the guard and the comparison share the constant now.</p>
|
||
<h2 id="5-surrogate-identities-model-record-rs-adr-002"><a class="anchor" href="#5-surrogate-identities-model-record-rs-adr-002">#</a>5. Surrogate identities — <code>model/record.rs</code> (ADR 002)</h2>
|
||
<p>A client sees a grid of label strings indexed by viewport position. It holds no <code>Model</code>, no category names and no page/filter constraints, so it cannot build a <code>CellKey</code> — which is why <a class="tid" href="../tickets.html#improvise-q08" title="improvise-ws-server binary (tokio + tungstenite session wrapper)">improvise-q08</a> found a thin client cannot compose <code>SetCell</code>. The answer is an opaque surrogate the server projects per visible cell and the client echoes back.</p>
|
||
<p><strong>Two surrogates, because two kinds of thing are nameable and only one is settable.</strong> <code>RecordHash</code> names a <strong>record</strong> — the coordinate tuple minus <code>_Measure</code> (<code>record_of</code>). That unit is not chosen here: it is already ADR 001's unit of atomicity and already what the write counters are keyed by, because renaming a category coordinate in wide records mode moves every measure cell in the row (§6). One identity therefore serves addressing <em>and</em> staleness. <code>AggregateHash</code> names an <strong>aggregate's coordinate constraint</strong> — row and column coordinates plus the page <em>and filter</em> selections that decide which stored cells it sums (<code>View::filtering_cats()</code>, §7). An aggregate is not the <em>absence</em> of an identity; it is a different kind of identity, which is what makes <code>Drill</code> addressable at all. It is deliberately <strong>not</strong> normalised: a measure on Row or Column is part of what the aggregate sums, so dropping <code>_Measure</code> would give two visibly different totals one name.</p>
|
||
<p><strong>The hash is specified here, not inherited.</strong> Three ways it breaks silently, each producing a <em>wrong write</em> rather than an error:</p>
|
||
<ol>
|
||
<li><strong>Not <code>DefaultHasher</code>, not <code>RandomState</code>.</strong> <code>std</code>'s default <code>HashMap</code> hasher is randomly seeded per process, so a surrogate would not survive a restart or agree between two processes. This is SipHash-2-4 under a constant key.</li>
|
||
<li><strong>Not <code>#[derive(Hash)]</code>.</strong> Rust gives no stability guarantee for a derived <code>Hash</code> across compiler versions or platforms. What is hashed is the explicit byte serialization in <code>hash_coords</code>, so the surrogate is defined by that format rather than by whatever the compiler did this week.</li>
|
||
<li><strong>128 bits, argued rather than defaulted.</strong> A collision is a silent write to the wrong record — precisely what got <code>_Index</code> rejected as a surrogate — and no later check catches it: a colliding surrogate resolves <em>successfully</em> to a record the client never saw. 64 bits reaches birthday risk around 2^32 records, which a pivot tool will not see by live record count; but the risk is over every record a long-lived document <em>ever names</em>, and the consequence is unbounded and undetectable. The other choice costs 8 more bytes per <em>visible</em> cell — 8 KB on a full 50×20 projection and far less on the diffs that carry steady-state traffic. Bounded cost against silent corruption is not close.</li>
|
||
</ol>
|
||
<p><strong>Serialization details that are load-bearing.</strong> The byte format is <code>tag ‖ count ‖ (len(category) ‖ category ‖ len(item) ‖ item)*</code>, every length a little-endian <code>u64</code>. The lengths are what make it injective: without them <code>("ab","c")</code> and <code>("a","bc")</code> serialize alike. Only <code>Hasher::write</code> is used, never <code>write_u64</code>/<code>write_u8</code>, which are free to feed the hasher in native byte order. The SipHash key constants are written as big-endian readings of <code>b"improvis"</code>/<code>b"e-record"</code> so the constant is the same on every platform. A domain tag byte (<code>0x01</code> record, <code>0x02</code> aggregate) is hashed first, because a record and an aggregate may be built from the same coordinates — an aggregate over a single record is the degenerate case — and they must not hash alike.</p>
|
||
<p><strong>Hex on the wire, not a JSON number.</strong> <code>serde_json</code> writes a <code>u128</code> as a bare number and a JavaScript host parses every JSON number as <code>f64</code>, so anything above 2^53 comes back changed — which for a surrogate means resolving to a different record or to none. The wire form is a fixed 32-character lowercase hex string.</p>
|
||
<h2 id="6-records-mode-is-wide-format-view-layout-rs"><a class="anchor" href="#6-records-mode-is-wide-format-view-layout-rs">#</a>6. Records mode is wide format (<code>view/layout.rs</code>)</h2>
|
||
<p>A row is one <strong>coordinate tuple</strong> — the cells sharing everything but <code>_Measure</code> — and each raw measure gets its own editable column. The columns are <code>[coordinates…, +cat, measures…, +]</code>: one per regular category, then the column that creates a <strong>new category</strong> (<a class="tid" href="../tickets.html#improvise-2zq" title="Records mode has no inline way to add a category, unlike the + column for measures">improvise-2zq</a>), then one per raw measure, then the column that creates a new measure (<a class="tid" href="../tickets.html#improvise-c5v" title="Improve records view data entry by making each measure a normal value column (wide format instead of synthetic Value column)">improvise-c5v</a>). Each <code>+</code> sits adjacent to what it adds to; two trailing columns would have made the user read a label to tell them apart, and position says it instead. Formula targets are not columns — they are computed, so there is nothing to type into them.</p>
|
||
<p>Consequences: two measures on one coordinate are <strong>one row</strong>; editing a category column moves <strong>every</strong> measure cell in the row, not just the one under the cursor; and <code>_Dim</code> in the synthetic key carries the column <strong>index</strong>, not a label, because a category and a measure item may share a name and a label could not say which column an edit landed on. Resolve the index through <code>GridLayout::record_col</code> into a <code>RecordCol</code>. Drill pending edits are keyed by <code>(row, RecordCol)</code>.</p>
|
||
<p><code>RecordRow::group</code> collapses loose cells into wide rows in <strong>first-appearance</strong> order, so the result follows the store's order rather than a hash walk (§8). A cell with no <code>_Measure</code> coordinate still anchors a row — that is how <code>add-record-row</code> seeds one — but contributes no column value.</p>
|
||
<p><strong>Records mode is a property of the view's axes, and <code>View</code> owns the one definition.</strong> <code>View::is_records_layout()</code> (<code>_Index</code> on Row and <code>_Dim</code> on Column) is what <code>GridLayout::new</code> asks and what <code>on_category_added</code> asks; two derivations would drift (<a class="tid" href="../tickets.html#improvise-edp" title="Establish ViewModel layer: derived data between RenderCache and widgets">improvise-edp</a>, the disease this codebase keeps curing).</p>
|
||
<h3 id="prune-axis-items-takes-a-predicate-not-a-flag-vector"><a class="anchor" href="#prune-axis-items-takes-a-predicate-not-a-flag-vector">#</a><code>prune_axis_items</code> takes a predicate, not a flag vector</h3>
|
||
<p><code>prune_empty</code> hands it <code>impl Fn(usize) -> bool</code> over the <strong>data</strong> index — the position a <code>row</code>/<code>col</code> argument means, which group headers do not consume. The predicate's domain is derived from <code>items</code> itself, so there is no parallel length a caller can get wrong and debug and release builds cannot disagree about an out-of-range flag. The earlier <code>&[bool]</code> + <code>debug_assert</code> pair could have, had a third caller ever sized <code>keep</code> from something other than <code>row_count()</code>/<code>col_count()</code> (<a class="tid" href="../tickets.html#improvise-n3wt" title="prune_axis_items: keep-slice length invariant is only a debug_assert">improvise-n3wt</a>, <code>doctrine(an_invariant_is_made_unrepresentable_not_validated)</code>).</p>
|
||
<p>A group header survives only when a data item that follows it survives. In <code>prune_empty</code> the row pass does not touch <code>col_items</code> or <code>has_value</code>, so the column pass still reads the original grid. Records mode is exempt entirely — the user drilled in to see all the raw data.</p>
|
||
<h2 id="7-axes-page-and-filter-are-two-different-questions"><a class="anchor" href="#7-axes-page-and-filter-are-two-different-questions">#</a>7. Axes: <code>Page</code> and <code>Filter</code> are two different questions</h2>
|
||
<p><code>Axis</code> is <code>Row | Column | Page | Filter | None</code>. <code>Filter</code> names a coordinate the <strong>drill</strong> fixed (<a class="tid" href="../tickets.html#improvise-dz8" title="Add Axis::Filter for drill-fixed coordinates">improvise-dz8</a>): like <code>Page</code> it constrains which records are visible, but unlike <code>Page</code> the user cannot navigate it. <code>Axis::next()</code> maps <code>Filter</code> to itself and no navigable axis rotates into it, so tile select cannot hand one out, and <code>is_user_assignable()</code> is false for it.</p>
|
||
<p>Picking the wrong question is the trap:</p>
|
||
<ul>
|
||
<li><strong>"what data is visible"</strong> → <code>View::filtering_cats()</code> (Page ∪ Filter). Used by <code>GridLayout</code>, by <code>AggregateHash</code>, and by <code>add-record-row</code> when it builds a key from the current constraints.</li>
|
||
<li><strong>"what can the user act on"</strong> → <code>categories_on(Axis::Page)</code> for <code>[</code>/<code>]</code> page cycling, or <code>View::tileable_cats()</code> for all three tile-select sites.</li>
|
||
</ul>
|
||
<p><code>Axis</code> also owns four presentation vocabularies — <code>plain_label()</code>, <code>glyph()</code> (which <code>Display</code> appends, giving "Row ↕"), <code>short_label()</code> (three columns for the tile bar; <code>None</code> is "·", <strong>not</strong> a truncation of "None") and <code>initial()</code> (one letter for the view panel; <code>None</code> is empty) — plus <code>color() -> AxisColor</code>. <code>AxisColor</code> is a <em>semantic</em> palette because <code>improvise-core</code> must not name a ratatui type; <code>draw.rs::axis_fg</code> does the one mapping, exactly as <code>minibuf_fg</code> does for <code>MinibufColor</code> (<a class="tid" href="../tickets.html#improvise-rml" title="Consolidate Axis display into a method on Axis">improvise-rml</a>). The tile bar and the category panel each carried a copy of the colour table before, with nothing keeping them in agreement.</p>
|
||
<h3 id="on-category-added-a-virtual-on-an-axis-is-a-configuration-not-a-vacancy"><a class="anchor" href="#on-category-added-a-virtual-on-an-axis-is-a-configuration-not-a-vacancy">#</a><code>on_category_added</code>: a virtual on an axis is a configuration, not a vacancy</h3>
|
||
<p>Virtual (underscore) categories seat on <code>Axis::None</code>. A regular category takes the first slot no <em>regular</em> category holds — Row, then Column, then Page.</p>
|
||
<p>This used to <strong>bump</strong> a virtual off Row or Column to make room, assuming a virtual sitting there was a placeholder. It never is, and the assumption cost two live bugs (<a class="tid" href="../tickets.html#improvise-2zq" title="Records mode has no inline way to add a category, unlike the + column for measures">improvise-2zq</a>):</p>
|
||
<ul>
|
||
<li><code>_Index</code> on Row together with <code>_Dim</code> on Column <em>is</em> records mode, and the only production writers of those axes are the records and drill setups. Bumping <code>_Index</code> <strong>ejected the user from the grid they were standing in</strong>: naming a category with <code>N</code> in records mode silently returned them to a pivot.</li>
|
||
<li><code>_Measure</code> on Column is an ordinary user configuration — it is what <a href="https://github.com/fiddlerwoaroof/improvise/blob/main/web/demo.improv"><code>web/demo.improv</code></a> ships. Bumping it made every measure vanish from the grid the moment a second regular category was added.</li>
|
||
</ul>
|
||
<p>So a virtual is never moved. A records view seats the new category on <code>Axis::None</code>, which is where records mode keeps <strong>every</strong> regular category and where <code>records_columns</code> — which filters on <code>kind.is_regular()</code> and never looks at an axis — picks it up as a column immediately. Elsewhere the new category simply joins whatever virtual is already there, nesting a header rather than destroying one.</p>
|
||
<p>The negative arm is load-bearing and must stay tested: without <code>a_pivot_view_still_seats_categories_row_then_column_then_page</code>, "always seat on <code>None</code>" would pass every assertion about records mode. Probing confirmed the asymmetry — seating everything on <code>None</code> fails 12 tests, while removing the virtual bump failed none at all, which is how the bump came to be wrong in two directions with nothing noticing.</p>
|
||
<h3 id="known-defect-two-answers-to-is-this-name-a-virtual"><a class="anchor" href="#known-defect-two-answers-to-is-this-name-a-virtual">#</a>Known defect: two answers to "is this name a virtual"</h3>
|
||
<p><code>on_category_added</code> tests <code>cat_name.starts_with('_')</code> (twice) while <code>is_records_layout()</code> four lines away asks <code>CategoryKind::of_name</code>. That is <strong><a class="tid" href="../tickets.html#improvise-bidq" title="Two answers to 'is this category virtual', four lines apart in view/types.rs">improvise-bidq</a></strong> and it is open; do not assume either spelling is the intended one when editing near it.</p>
|
||
<h2 id="8-ordering-is-correctness-not-tidiness-model-cell-rs"><a class="anchor" href="#8-ordering-is-correctness-not-tidiness-model-cell-rs">#</a>8. Ordering is correctness, not tidiness (<code>model/cell.rs</code>)</h2>
|
||
<p><code>DataStore</code> holds cells in an <code>IndexMap</code> keyed by <code>InternedKey</code>, plus a secondary index <code>HashMap<(Symbol, Symbol), HashSet<InternedKey>></code>.</p>
|
||
<p><strong>An index whose values are <code>HashSet</code>s is for O(1) lookup, never for iteration.</strong> <code>matching_cells</code>/<code>matching_values</code> used to walk the index's <code>HashSet</code> directly, so their order was hash order — different in every process, and different between two stores in one process, because <code>RandomState</code> keys each <code>HashMap</code> afresh. That defeated the whole reason <code>cells</code> is an <code>IndexMap</code>, and it was not tidiness (<a class="tid" href="../tickets.html#improvise-shyt" title="[bug] Records-mode row order is not stable across processes">improvise-shyt</a>). It caused, in ascending order of seriousness:</p>
|
||
<ul>
|
||
<li><strong>records-mode rows reordering between sessions</strong>;</li>
|
||
<li><strong><code>SUM</code>/<code>AVG</code> drifting in the last <code>f64</code> bits</strong>, because f64 addition is not associative (<code>Min</code>/<code>Max</code>/<code>Count</code> and the text-unanimity rule of §2 are order-free, so they were never affected);</li>
|
||
<li>worst, the <strong>drill snapshot</strong> freezing its record list in hash order while <code>drill_state.pending_edits</code> are keyed by <em>row index</em> (§6) — so the same drill opened in a second process addressed <strong>different records</strong>. Silent misdirected writes, and nothing in the tracker knew.</li>
|
||
</ul>
|
||
<p><code>matching_positions</code> is the fix: the index still <strong>narrows</strong> the candidates, and the primary map <strong>orders</strong> them — each surviving candidate resolves to its position with one hash lookup and the positions are sorted. Shape stays O(smallest set × |partial|) plus O(m log m) over the <em>matches</em>, not a scan. Measured at 18,000 cells: +20% worst case on <code>matching_values</code>, and <code>matching_cells</code> 7% <em>faster</em>, sorted positions resolving <code>CellKey</code>s with better locality. The alternatives both move the cost — an ordered index value type pays on every <code>set</code>; scanning the primary is O(store) per query.</p>
|
||
<p><strong>An empty <code>partial</code> matches every cell, vacuously, and <code>matching_positions</code> is where that is answered</strong> (<a class="tid" href="../tickets.html#improvise-hkt6" title="Three small totality gaps found in review">improvise-hkt6</a>). It used to index <code>sets[0]</code> and panic; both public callers happened to guard, so it was unreachable — but the guard lived one level away from the indexing, which is how this class of bug returns. The guards could not simply be pushed down as <code>vec![]</code>: "no coordinates constrain anything" means <em>all</em> positions, and answering "none" here would have made removing a caller's fast path change behaviour silently. The callers keep theirs as a fast path, not as a guard.</p>
|
||
<p><strong><code>live_records</code> has a decided order, not an inherited one</strong> (<a class="tid" href="../tickets.html#improvise-2yc2" title="DataStore::live_records walks a HashMap -- decide its order before writing the caller">improvise-2yc2</a>). It walks <code>cells</code> and projects insertion order onto records: a record sits where its <strong>first</strong> cell sits. It deliberately pays O(cells) rather than O(records), because <code>cells</code>' order is already the answer <code>matching_positions</code> gives and a second answer to "what order is this store in" is the disease. It is derived per call rather than memoised on <code>RecordEntry</code>, since a stored position is a copy and a record deleted and re-created would hold its old one while its cells sit at the end of the store. Note what is <strong>not</strong> on offer: an unordered return. Every iterator has an order, and a type that merely disclaims one still hands the caller hash order — the guard living one level away from the hazard, which is the shape this whole class of bug returns through. The function had no callers when this was decided, so the order was still free to <em>decide</em> rather than to discover; with no caller to benchmark against, correctness is the defensible default, and a hot-path caller can argue for a memoised position with numbers.</p>
|
||
<h3 id="the-record-index-recordentry"><a class="anchor" href="#the-record-index-recordentry">#</a>The record index: <code>RecordEntry</code></h3>
|
||
<p><code>DataStore::records: HashMap<RecordHash, RecordEntry></code> answers both of a client surrogate's questions — "which record do you mean?" (ADR 002) and "is your basis stale?" (ADR 001) — from <strong>one</strong> entry in <strong>one</strong> lookup, because both are keyed by the record and both are invalidated at the same moment.</p>
|
||
<p>It lives in the store because this is where the same job is already done: the store already interns every name and already maintains a secondary index updated at exactly these two points (<code>set</code>, <code>remove</code>), and it is that existing <code>(Symbol, Symbol)</code> index which makes <strong>invalidation</strong> tractable — when a category or item disappears, the cells naming it are found without a scan, so the records built from them are too.</p>
|
||
<p>It is deliberately <strong>not</strong> regenerated from the layout on demand. That would be O(visible cells) per resolve <em>and</em> would only ever cover the visible window, so a record scrolled out of view would look identical to a record that no longer exists — conflating "reject, nothing to write to" with "apply".</p>
|
||
<p>It is <strong>not serialized</strong>: <code>Deserialize</code> rebuilds it through <code>set</code>, and every version starts at zero, which is correct because a freshly loaded document is a new <code>document_generation</code> and no client holds a basis against it.</p>
|
||
<p><strong><code>cells == 0</code> is a tombstone, not a reason to forget the entry.</strong> A record that is deleted and then re-created must not hand a client holding the old basis its own version number back; dropping the entry would reset the counter to zero and make a stale basis look <em>current</em>, which is the silent accept the basis check exists to prevent. The counter tracks <strong>liveness, not writes</strong> — <code>set</code> bumps <code>cells</code> only for a genuinely new cell, and <code>version</code> is moved by the observed diff in <code>App::apply_effects</code>, never from <code>set</code>.</p>
|
||
<p><code>resolve_record</code> returning <code>None</code> means <strong>the record is gone</strong> — never stored, or every cell removed, which is what happens when a category or item it named is deleted. That is one arm of staleness and needs no version comparison: there is nothing to write to. <code>record_version</code> reporting <code>0</code> for an unknown surrogate is deliberate: a basis composed against an absent record and one composed against an untouched record are the same claim.</p>
|
||
<h2 id="9-categories"><a class="anchor" href="#9-categories">#</a>9. Categories</h2>
|
||
<p><code>CategoryKind</code> is <code>Regular | VirtualIndex | VirtualDim | VirtualMeasure | Label</code>. Business rules match on the enum rather than sniffing name prefixes.</p>
|
||
<p><code>CategoryKind::category_name()</code> is the <strong>one</strong> place the name↔kind mapping is written down, so adding a variant forces a decision there (<a class="tid" href="../tickets.html#improvise-2lh" title="Migrate name-string comparisons to CategoryKind enum checks">improvise-2lh</a>). <code>of_name(name)</code> is safe to use in place of looking the <code>Category</code> up, because <code>Model::add_category</code> hands back the existing virtual category rather than shadowing a reserved name — <code>stored_virtual_categories_match_their_names</code> is what makes that sound. <code>Label</code> categories are user-named and so read as <code>Regular</code> from <code>of_name</code>; consult the stored <code>Category::kind</code> when the distinction matters.</p>
|
||
<p><code>VirtualMeasure</code> items come from two sources: numeric data fields listed in the file, and formula targets added implicitly. They are virtual because formula-derived items are <em>implied</em> by the formula definitions — listing them explicitly would be redundant in the file format and confusing in the UI. <code>add_formula</code> does <strong>not</strong> add items to <code>_Measure</code>; use <code>measure_item_names()</code> / <code>effective_item_names("_Measure")</code>.</p>
|
||
<p><code>Label</code> is a high-cardinality per-row field (description, id, note): stored alongside the data so it shows up in record/drill views, but defaulting to <code>Axis::None</code> and excluded from the <code>MAX_CATEGORIES = 12</code> pivot limit and from auto Row/Column assignment.</p>
|
||
<p><code>Category::add_item</code> deduplicates by name and assigns IDs from a private counter, so external code cannot fabricate an <code>ItemId</code>.</p>
|
||
<h2 id="10-workbook-exists-to-break-the-model-view-cycle"><a class="anchor" href="#10-workbook-exists-to-break-the-model-view-cycle">#</a>10. <code>Workbook</code> exists to break the <code>Model ↔ View</code> cycle</h2>
|
||
<p><code>Model</code> knows nothing about views; <code>View</code> depends on <code>Model</code>, one direction. Cross-slice operations — adding or removing a category must update both the model's categories and <strong>every</strong> view's axis map — live on <code>Workbook</code>, so <code>Model</code> stays pure data and <code>improvise-core</code> can be extracted without pulling view code along. That is also why <code>AddCategory</code>/<code>RemoveCategory</code> are <code>LayoutGeometry</code> effects rather than <code>CellData</code> ones (gotcha 3).</p>
|
||
<p><code>Workbook::new</code> seeds a <code>Default</code> view, registers the three virtual categories through <code>on_category_added</code> (so all three land on <code>Axis::None</code>), then bumps <code>_Measure</code> to <code>Axis::Page</code> so aggregated pivot views show a single measure at a time (<a class="tid" href="../tickets.html#improvise-kos" title="_Dim and _Index should work as pivot axes outside Records/Drill view; must default to None axis on non-records views including empty models">improvise-kos</a>, commit 709f2df). Leaving <code>_Index</code>/<code>_Dim</code> on <code>None</code> keeps pivot mode the default — records mode activates only when the user moves both onto axes.</p>
|
||
<p><code>Workbook::add_item</code> errors on an unknown category so callers do not reach through <code>model.category_mut(..)</code> and invent their own handling (<a class="tid" href="../tickets.html#improvise-tvt" title="Route mutations through Workbook facade instead of app.workbook.model chains">improvise-tvt</a>). There is no <code>Model::add_item</code>.</p>
|
||
<h2 id="11-virtual-views"><a class="anchor" href="#11-virtual-views">#</a>11. Virtual views</h2>
|
||
<p><code>_Records</code> and <code>_Drill</code> (<code>view::VIRTUAL_VIEW_NAMES</code>) are scratch views the app builds on demand. Both put the virtual <code>_Index</code>/<code>_Dim</code> categories on axes, and those categories are never persisted — so these views are rebuilt from scratch each time rather than saved (<a class="tid" href="../tickets.html#improvise-60z" title="Virtual views _Records and _Drill should not be persisted to .improv files">improvise-60z</a>).</p>
|
||
<h2 id="12-axis-of-panics-try-axis-of-does-not"><a class="anchor" href="#12-axis-of-panics-try-axis-of-does-not">#</a>12. <code>axis_of</code> panics, <code>try_axis_of</code> does not</h2>
|
||
<p>Registration is guaranteed for every model category: <code>Workbook::new</code>, <code>add_category</code>, <code>add_label_category</code> and <code>create_view</code> all call <code>View::on_category_added</code> for every view. So any name obtained from <code>Model::categories</code> or from the view's own <code>category_axes</code> is safe for <code>axis_of</code>. Use <code>try_axis_of</code> when the name comes from anywhere else — <code>cycle_axis</code> does, and no-ops on an unknown category to match <code>set_axis</code> (<a class="tid" href="../tickets.html#improvise-t8s" title="View::axis_of panics on missing category">improvise-t8s</a>).</p>
|
||
<h3 id="record-cell-key-is-a-match-and-the-match-is-the-guard-improvise-ir7l"><a class="anchor" href="#record-cell-key-is-a-match-and-the-match-is-the-guard-improvise-ir7l">#</a><code>record_cell_key</code> is a match, and the match is the guard (<a class="tid" href="../tickets.html#improvise-ir7l" title="[bug] A formula cell is offered as editable and silently reverts">improvise-ir7l</a>)</h3>
|
||
<p><code>GridLayout::record_cell_key(row, col)</code> is the one mapping from a records position to a model cell, and it answers <code>Some</code> for <code>RecordCol::Measure</code> alone. The other three arms are spelled out rather than swept into a <code>_</code>, and that exhaustive match is the whole guard: a new <code>RecordCol</code> variant fails the build until someone decides whether it names a stored cell.</p>
|
||
<p><strong>There is no test for the <code>Category</code> arm, and the reason is not that the wrong answer would be harmless.</strong> It would be a real fault, and a subtle one worth naming so nobody re-derives it wrongly. <code>RecordCol::Category(name)</code> carries a <strong>category</strong> name, while <code>RecordRow::cell_key(arg)</code> puts its argument under <code>_Measure</code>. So answering <code>Some</code> there yields <code>coords + _Measure=<category name></code>, and <code>Model::formula_for</code> would match it whenever a <code>_Measure</code> item happens to share a name with a regular category — projecting <code>CellId::Derived</code> for a <strong>coordinate column</strong>, which is never a formula target and which the pivot path does not have at all. The two front ends would then disagree, which is the design-principles §5 defect this ticket exists to close. <code>RecordCol</code> exists for exactly that collision: its doc says the enum resolves a column from its index "so a category and a measure sharing a name stay distinguishable".</p>
|
||
<p>What earns the decision to leave it untested is narrower: the <strong>reachable</strong> version of that fault is a new variant answered carelessly, and the exhaustive match makes that unrepresentable rather than merely unlikely (<code>doctrine(an_invariant_is_made_unrepresentable_not_validated)</code>). A test would pin behaviour the type system already pins, and would not survive the arm being renamed. Found in review, and the first recorded reason for declining it was <strong>wrong</strong> — it claimed the injected behaviour was defensible by analogy with a formula targeting a regular category, which is a different key entirely.</p>
|
||
<h2 id="13-route-then-aggregate-sum-folds-over-the-member-records"><a class="anchor" href="#13-route-then-aggregate-sum-folds-over-the-member-records">#</a>13. Route then aggregate: <code>SUM(<expr>)</code> folds over the member records</h2>
|
||
<p><a class="tid" href="../tickets.html#improvise-4zl7" title="A fitted tree must route per record and aggregate the results, not route on the aggregate">improvise-4zl7</a>, and the one place the evaluator reaches past the cell it was asked about.</p>
|
||
<p><strong>The problem.</strong> A formula is evaluated at a <code>CellKey</code>. In an aggregating cell <code>evaluate_aggregated</code> aggregates each <code>Expr::Ref</code> <em>first</em> and evaluates the expression once on the totals. That is exactly right for a linear formula — <code>SUM(Revenue) - SUM(Cost)</code> and <code>SUM(Revenue - Cost)</code> are the same number — and exactly wrong for anything that <strong>routes</strong>. <code>IF(Cost < 50, 1, 10)</code> over records costing 10 and 100 answers <code>1 + 10 = 11</code> per record and <code>10</code> on the total, because <em>aggregate-then-route is not route-then-aggregate</em>. Every fitted tree (<a class="tid" href="../tickets.html#improvise-8xhy" title="[bug] A fit over a categorical feature emits an unevaluable formula">improvise-8xhy</a>, <a class="tid" href="../tickets.html#improvise-vx3" title="fit-measure command: fit a tree and add it as a formula">improvise-vx3</a>) is such a formula, so it meant one thing on a record row and something else one row up. The user's ruling was to <strong>aggregate the rule over the members</strong>.</p>
|
||
<p><strong>The mechanism, and why it needed no new vocabulary.</strong> <code>SUM(e)</code> already means "fold <code>e</code> over the members". It simply did not: <code>eval_agg</code> specialised <code>Expr::Ref</code> and, for every other inner expression, summed the raw cells matching the partial key and <em>ignored the expression entirely</em>. It now folds — one entry per member record, the expression evaluated at that record's own coordinates:</p>
|
||
<pre><code>partial = context.without(target_category) (plus any inline WHERE)
|
||
records = distinct(matching_cells(partial).map(key.without("_Measure")))
|
||
values = records.map(|r| eval(inner, context.with(every coord of r)))</code></pre>
|
||
<p>Three consequences worth stating.</p>
|
||
<p>The rule is <strong>written once, and so is the evaluator it folds with.</strong> <code>eval_agg</code> is shared by <code>eval_expr</code> and <code>eval_expr_cached</code>, and <strong>both</strong> hand it the same <code>eval_at</code>: the record-level <code>eval_expr</code>. That is not a convenience, it is the correctness. A member key is a <em>record</em>, and <code>formula_cache</code> is keyed by <strong>stems with <code>none_cats</code> stripped</strong> — so the cached evaluator's <code>Expr::Ref</code> can never hit the cache at a member key, falls through to <code>aggregate_raw</code>, finds no raw cell behind a formula target, and answers <code>Err("ref:…")</code>. A first version did exactly that, and the two evaluators then disagreed on any routing formula reading a <em>derived</em> measure: <code>Number(4.0)</code> uncached, <code>Error("ref:Profit")</code> cached, which is the very divergence this section exists to remove (§3: "a rule stated in one and not the other is a live bug").</p>
|
||
<p><strong>And making the lookup "work" would have been worse than the error.</strong> The only way to hit the cache from a member key is to strip <code>none_cats</code> off it — which returns the <em>group's</em> value for that formula and hands it to every member. That is aggregate-then-route again, laundered through a cache lookup, and it would have been silent. The cache cannot answer a per-record question, so the fold does not ask it. Only <code>Expr::Is</code> needed an arm in each evaluator, and since the fold routes through <code>eval_expr</code>, the cached arm is now reached <strong>only by a bare <code>Is</code> outside a <code>SUM</code></strong> — which is what <code>the_cached_evaluator_has_its_own_working_coordinate_predicate</code> pins.</p>
|
||
<p>The <code>Expr::Ref</code> fast path <strong>stays</strong>, and it is the same fold specialised: a record's <code>Ref</code> is that record's own cell, so summing them is summing the matching values. It differs in exactly one place — a record with no such cell is <em>skipped</em> there and an <strong>error</strong> in the fold. That is deliberate: a bare <code>SUM(Revenue)</code> asks for the values that exist, while <code>SUM(<expr>)</code> asks the expression of each member, and an expression that cannot be evaluated is an error rather than a zero. Silently dropping members is how a routed sum would quietly answer for a subset.</p>
|
||
<p>The asymmetry's <strong>blast radius</strong> is wider than it sounds, and worth stating: gotcha 7 makes a pipe-quoted text cell a first-class <code>.improv</code> value, so <strong>one</strong> text cell anywhere in a member set turns the whole aggregated fitted cell into <code>Error("ref:Cost")</code>, where the bare-ref path would have dropped it and returned a number. That is the loud failure this repo prefers to a quietly wrong sum, but it is a real behaviour difference and not a detail.</p>
|
||
<p>Records come out in <code>DataStore</code> insertion order, deduplicated by an <code>IndexSet</code>, for §8's reason — hash order here would make a fitted cell drift in its last bits between processes.</p>
|
||
<p><strong>Interaction with the fixed point (§3) and <code>formula_cache</code>.</strong> None of this is new state. The fold reads <em>raw</em> data through the same <code>evaluate_depth</code> / <code>aggregate_raw</code> the non-folding path uses, so a formula cell still depends only on raw cells and on other formula cells' cached values; <code>recompute_formulas</code> iterates the same stems to the same fixed point, and <code>formula_cache</code> still holds exactly the aggregated formula cells it held before. The fold does not cache and does not memoise: it is O(members) per evaluation per pass.</p>
|
||
<p><strong>Measured</strong>, because the cost was a blocker and not a footnote (<a class="tid" href="../tickets.html#improvise-9iyv" title="The route-then-aggregate fold's cost, measured">improvise-9iyv</a>; <a href="https://github.com/fiddlerwoaroof/improvise/blob/main/crates/improvise-core/examples/fold_bench.rs"><code>crates/improvise-core/examples/fold_bench.rs</code></a>, five runs per point, best–worst). At 18,050 cells with one routing formula and <code>Tag</code> on <code>Axis::None</code>, <code>recompute_formulas</code> takes <strong>18.1–19.2 ms</strong> release and 132.9–135.1 ms debug, against 7.1–9.0 ms / 71.7–74.6 ms for a linear <code>Rev - Cost</code>. That is <strong>2.3×</strong> a linear formula and <strong>linear in cells</strong>: 400 → 0.4 ms, 2,000 → 2.0 ms, 18,050 → 18.1 ms, so 45× the cells costs 45× the time.</p>
|
||
<p>It was not always linear. Routing the fold through <code>eval_expr_cached</code> — the first version, above — made every member's <code>Ref</code> miss the cache and fall into <code>aggregate_raw</code>, one poorly-narrowed scan <strong>per member</strong>: 51.1–53.3 ms at the same 18,050 cells and superlinear with it (45× the cells cost 85× the time). So the divergence and the stall were <strong>one defect</strong>, and fixing the first fixed the second.</p>
|
||
<p><strong>And "18,050 cells" was not a sufficient description of that workload.</strong> At the same cell count the old code cost 51 ms at a 25×361 shape and 244 ms at 95×95 — a 4.8× spread from group <em>count</em> alone, which is what a per-member scan predicts and a constant factor does not. After the fix the two shapes are 16.9–17.2 ms and 16.4–17.5 ms: indistinguishable. <strong>The shape-dependence being gone is the confirmation of the cause</strong>, more than the absolute number is. Re-measuring this means stating the shape, not just the cells.</p>
|
||
<p>The fixed point runs exactly <strong>2</strong> passes (measured, not assumed), so hoisting the member set out of the loop has a 2× ceiling on the fold's share and was declined: a memo is state, and a memo keyed by anything less than the whole <code>partial</code> is a second answer to "what does this cell depend on".</p>
|
||
<p>One asymmetry not to tidy: the cached path hands <code>eval_expr</code> a fresh <code>MAX_EVAL_DEPTH</code> at each <code>Agg</code>, while the uncached <code>Agg</code> threads its own <code>depth</code> through. The second is what bounds a chain of folded formulas; making them agree by resetting both would unbound it.</p>
|
||
<p><strong><code>Expr::Is(Filter)</code> — a coordinate predicate as a number.</strong> <code>Cat IS Item</code> is <code>1.0</code> when the cell's own <code>Cat</code> coordinate is <code>Item</code> and <code>0.0</code> otherwise. It keeps the evaluator <code>f64</code>-only — no string comparison enters <code>eval_expr</code> — and it reuses the primitive <code>Formula::filter</code> already had, <code>context.get(&cat)</code>, including its <code>unwrap_or(false)</code>: a cell with <strong>no</strong> <code>Cat</code> coordinate is not that item. Under route-then-aggregate that arm is reached only where a record genuinely lacks the coordinate, because the fold hands the expression a record key. Evaluated bare, outside a <code>SUM</code>, it degrades exactly as <code>Formula::filter</code> does — which is why a fitted tree is always emitted wrapped.</p>
|
||
<h2 id="pageaxis-what-constrains-and-what-the-user-may-turn-improvise-p1ym"><a class="anchor" href="#pageaxis-what-constrains-and-what-the-user-may-turn-improvise-p1ym">#</a><code>PageAxis</code> — what constrains, and what the user may turn (<a class="tid" href="../tickets.html#improvise-p1ym" title="Project the page-axis item lists so [ and ] work in a browser">improvise-p1ym</a>)</h2>
|
||
<p><code>GridLayout::page_coords</code> answers two questions that gotcha 27 says must not be confused. The bar at the top of a view names <strong>every</strong> category constraining the visible records, which is <code>View::filtering_cats()</code> — Page ∪ Filter — because a drill fixes its coordinates on <code>Filter</code> and the user should still see them. <code>[</code>/<code>]</code> cycle only what the user may act on, which is <code>categories_on(Axis::Page)</code>.</p>
|
||
<p>Those were two walks before: <code>GridLayout</code> did the first, <code>navigation.rs:: page_cat_data</code> did the second, and each spelled the Page-vs-Filter rule itself. Two spellings of one rule is the shape design-principles §5 calls the disease, and here it had already leaked into the protocol — <code>GridCache::page_coords</code> was a verbatim copy of the layout's list, so a browser handed it would have tried to page a Filter axis.</p>
|
||
<p>So <code>page_coords</code> is now <code>Vec<PageAxis></code>, and <code>items</code> is <code>Some</code> exactly when the axis is navigable. The rule lives in the <em>type</em>: a <code>Filter</code> axis carries no item list, so no caller can compose a cycle for one, and <code>PageAxis::cycle()</code> is the one place that pairs a list with the index its selection sits at. <code>page_cat_data</code> and the browser's <code>page_cycle</code> both read it rather than deriving it.</p>
|
||
<p>Two consequences worth stating. The list is <code>Model::effective_item_names</code>, the same call that already chose the default selection, so the selection and the list it indexes into come from one place by construction — <code>page_cat_data</code> used <code>ordered_item_names</code> before, which for a <code>_Measure</code> page axis meant the default selection could be a formula target <code>[</code>/<code>]</code> could not reach. That incoherence is gone; the cost is that <code>[</code>/<code>]</code> over a <code>_Measure</code> page axis now visits formula targets, which is what the column list shows anyway.</p>
|
||
<p>And the list is <strong>not</strong> filtered to visible items. Hidden items are still cycled, because that is what the TUI has always done, and making the browser differ would be the divergence rather than the fix. Changing it is a change to both, together.</p>
|
||
<h2 id="the-number-format-format-rs-improvise-711n"><a class="anchor" href="#the-number-format-format-rs-improvise-711n">#</a>The number format — <code>format.rs</code> (<a class="tid" href="../tickets.html#improvise-711n" title="parse_number_format silently yields 0 decimals for the ,.2f spelling our own docs show">improvise-711n</a>)</h2>
|
||
<p>A view's <code>number_format</code> is a <strong>printf-style picture, and only that</strong>. A <code>,</code> anywhere turns on thousands grouping; a <code>.</code> introduces the precision, which is the <strong>leading digit run after the last <code>.</code></strong>; anything following those digits — a conversion letter — is ignored. So <code>,.0</code>, <code>,.2</code>, <code>.2</code>, <code>,.2f</code> and <code>%.2f</code> all name what they look like they name.</p>
|
||
<p><strong>Why the trailing <code>f</code> is accepted rather than refused.</strong> It round-trips into documents today and always has. <code>improv.pest</code> spells the line <code>format_line = { "format: " ~ rest_of_line ~ NEWLINE ~ blank_lines }</code> — the grammar takes the format string as an <strong>opaque line</strong>, so <code>format: ,.2f</code> is a document the grammar accepts; <code>:set-number-format</code> stores whatever the user typed verbatim (<code>cmd/effect_cmds.rs</code>), and <code>format_md</code> writes <code>view.number_format</code> back out unchanged. Refusing the spelling at load would therefore reject documents this repo's own grammar admits and its own <code>.improv</code> example spells, to no benefit: by gotcha 8 the format is display-only, so misreading it costs a wrong pixel and can never cost a wrong number. A refusal here buys nothing and costs a document.</p>
|
||
<p><strong>Why printf and not Excel.</strong> The two conventions disagree on exactly the spelling that distinguishes them: <code>.00</code> is <em>precision zero</em> to printf and <em>two places</em> to Excel, so a parser that tried to honour both would have to guess. One is picked, and it is the one every spelling in this repository already uses. Consequences worth stating rather than discovering: <code>0.00</code> reads as <code>(false, 0)</code>, and an Excel picture such as <code>#,##0.00</code> reads as <code>(true, 0)</code> — grouped, no decimals.</p>
|
||
<p><strong>Nothing is validated, deliberately.</strong> A spelling the rule cannot read names no precision and formats with none; there is no error channel and no <code>Result</code>, because the reader is display-only and four call sites would have to thread one.</p>
|
||
<p><strong>What the bug was, and why six tests sat on top of it.</strong> The precision was read as <code>fmt[i+1..].parse::<u8>()</code> over the <em>whole</em> tail, so <code>"2f"</code> failed to parse and fell through to <code>unwrap_or(0)</code> — <code>,.2f</code> displayed <strong>zero</strong> decimals, in the TUI, in the browser projection and in CSV export. <code>",.2f"</code> and its <code>,.3f</code>/<code>,.4f</code> siblings are what this workspace's own fixtures reach for, in about ten tests, and none could see it: they assert the stored <em>string</em> round-trips or use it as an opaque value to dirty a widget, or — in <code>export_csv_quotes_commas_per_rfc4180</code> — they assert the rendered value <code>starts_with("1,200")</code>, which <code>1,200</code> and <code>1,200.00</code> both satisfy. A prefix check over a value whose <em>suffix</em> is the thing in question answers a neighbouring question, which is why that one is now an exact equality.</p>
|
||
<p><strong>Only two of the three consumers are guarded.</strong> Reverting the fix fails <code>format.rs</code>'s own unit tests and <code>export_csv_quotes_commas_per_rfc4180</code>, and nothing else in the workspace: <code>ui/project.rs::grid_of</code> (the browser projection) and <code>ui/app.rs</code> both call <code>parse_number_format</code> and neither has a test that renders a cell and asserts its digits. That gap is named rather than closed — those files are <code>improvise-command</code>, which was another wave's ground when this landed — and it is the more useful half of this entry, because it says where the next regression would go unseen.</p>
|
||
</main>
|
||
</div>
|
||
</body>
|
||
</html>
|