769 commits

This branch

This branch

All branches

Author	SHA1	Message	Date
andodeki	728fbc3ad0	fix(pdf): mesh shadings — three bugs in code that had no fixture ... ADR 0028 shipped types 4-7 and said honestly that they were unproven: the uncovered lines of `shading.rs` were exactly `parse_mesh`, "the position `image.rs` was in before ADR 0016 found the JPEG decoder was a stub". Writing the fixtures found three real bugs. - Type 5 has no per-vertex flag; `/VerticesPerRow` delimits it. Reading 8 phantom bits shifted every vertex after the first, decoding plausible coordinates that were entirely wrong. - Types 6 and 7 are patches: 12 or 16 control points carrying no colour, then four corner colours. The old loop read a colour per point, consumed three times too many components, ran off the stream, and the None-on-truncation path swallowed it as "the mesh ended". - A flag-0 triangle is three vertices whose second and third flags are ignored (§8.7.4.5.5). Acting on them cleared the strip every time and produced no triangles at all. Caught in new code, before it shipped. And one omission: `color_at_point` returned None for a mesh, so a mesh that parsed perfectly still painted nothing — indistinguishable from one that failed. `MeshTriangle::color_at` now interpolates the corner colours by barycentric coordinates, None outside, because black is a colour a mesh can legitimately produce. Shared-edge patches (flags 1-3) inherit the previous patch's edge rather than being read as fresh patches, which desynchronised the rest of the stream. Five corpus fixtures, generated from named coordinates and colours so every expected value in the tests is one the generator wrote deliberately. Eight tests, five mutations, all killed. Coons flattening is still an approximation and still reports `is_approximate`. ADR 0029.	2026-08-18 19:10:49 +00:00
andodeki	c9474e2c9f	feat(spreadsheet): dynamic-array spill model — FILTER, GROUPBY, PIVOTBY, A1# ... The engine stored every formula result as one display string per cell. This adds the dynamic-array architecture: a formula can return a grid, which "spills" into the cells below/right of its anchor. Value model: - `Value::Array(Vec<Vec<Value>>)` — a grid result. Scalars coerce to one-cell grids where the shape matters; `to_f64`/`to_bool` refuse an array, `to_display_string` shows the top-left, and `resolve_arg` flattens an array argument so `SUM(FILTER(...))` aggregates its cells. - Element-wise broadcasting: `resolve_array2d` maps a binary operator over a grid when one side is a scalar (or both grids share a shape), so `FILTER(A1:A4, C1:C4 > 15)` builds the boolean include the way Excel does. The binary-op logic was factored into `apply_binop`. Parser and AST: - `#` is now the spill operator: `A1#` and `Sheet2!A1#` parse as `SpillRef` / `SheetSpillRef`, evaluate to the anchored array (so `=A1#` re-spills), flatten in aggregates, and track their anchor in the dependency graph (intra- and cross-sheet). Functions (dynamic arrays): - FILTER(array, include, [if_empty]) — keep rows (column include) or columns (row include); `#N/A` on no match unless `if_empty`. - GROUPBY(row_fields, values, function, [field_headers]) — group rows by field tuples and aggregate each value column (SUM/AVERAGE/COUNT/ MAX/MIN/MEDIAN by name, ETA-reduced-LAMBDA form). - PIVOTBY(row_fields, col_fields, values, function) — a 2D pivot with the aggregate name in the top-left corner. Spill storage (data.rs): - `SpillRange` + `spills` map on `SpreadsheetData`: derived cells read back through `get_display_value`/`get_raw`/`get_edit_value`, are not blank, and are read-only — `set_cell`/`put_cell`/`remove_cell`/ `mutate_cell` refuse to touch them (the UI blocks via `is_spilled`). - Recalc builds the spill from the `Array` result (`apply_formula_result`) and drops stale spills when a formula becomes scalar, is removed, or cycles. Spills are derived state, never serialized — the anchor formula persists and re-derives on load. - Cross-sheet spills read through `get_sheet_spill_values`. Tests: 21 new units (FILTER/GROUPBY/PIVOTBY shapes, broadcasting, Value::Array methods, spill parsing) + 5 end-to-end tests (spill display, read-only cells, `A1#` aggregation and re-spill, stale-spill clearing, cross-sheet spill). Engine unit tests 405 -> 427; UI lib tests still pass. Engine coverage 96.66% (floor 96).	2026-08-18 18:34:22 +00:00
andodeki	a2b05c56c9	feat(makepad-table): opt-in capabilities feature, and raise the matrix_client defect ... The two caveats from the dependency investigation. ## The capabilities feature Camera and location attachments are now available behind `features = ["capabilities"]`, which pulls `nigig-uikit` and supplies `UikitAttachmentProvider`. Measured: 89 crates by default, 275 with the feature on. That cost is real and it is inherent, not packaging waste. `camera_widget` imports `send_geocode_request` and `request_map_tile` from `nigig-core`, both of which call `spawn_async` — the shared Tokio runtime — and the first makes an HTTPS call to Nominatim. A camera that geocodes needs an async runtime and an HTTP client; there is no lighter honest version. It is affordable because it is opt-in, and because any app enabling it already depends on `nigig-core`, so that app's own tree grows by nothing. Everything touching `nigig-uikit` is in one module, so the boundary is a file rather than `#[cfg]` scattered through the widget. The provider holds no widgets of its own: the host owns the `CameraWidget` already in its tree and this asks it to open, because a provider that instantiated a second camera would fight the first for the device. A second request while one is outstanding is refused rather than overwriting. The table turns that refusal into `AttachmentUnavailable`, so the user is told the camera is busy instead of watching their first request vanish. File picking is deliberately declined here — `robius-file-picker` already ships unconditionally and costs nothing, and two paths for one job is one too many. Two CI gates, both verified to fail when they should: the opt-in build must keep compiling, and the default build must pull none of `tokio`, `reqwest`, `hyper`, `clap`, `csv`, `image`, `nigig-uikit` or `nigig-core`. The second checks the resolved `cargo tree` rather than the manifest, because feature unification can switch an optional dependency on from a sibling crate. Tests 99 default, 105 with the feature. Both clippy-clean. ## The matrix_client defect Raised in REVIEWS/MATRIX_CLIENT_FEATURE_GATE.md rather than fixed. It is not my crate, nothing depends on the broken combination, and a blind fix could change behaviour someone relies on. `matrix_client` declares `native = ["dep:tokio", "dep:reqwest", "dep:rusqlite"]` but its source gates on `#[cfg(not(target_arch = "wasm32"))]`. Two switches for the same modules, so on a native target with the feature off the modules compile and their dependencies do not — 19 errors, 26 ungated uses across 7 files. There is no CI job for the crate, which is why it rotted unnoticed. The note corrects an overstatement I made while arguing for the trait hook. I said fixing this would unblock wasm. It would not: `matrix_client` already builds clean for wasm32 with `--no-default-features`, and `nigig-core` has 8 wasm errors of its own (`crate::platform::spawn` missing) that have nothing to do with it. The only broken combination is native-target-with-feature-off, which nothing builds. I also said earlier that `matrix_client` was heavy — it is a 7-dependency local crate, not matrix-sdk. That was wrong and it inflated the case for the trait hook; the note records the measured numbers instead.	2026-08-18 18:03:26 +00:00
andodeki	45efc74106	feat(spreadsheet): pivot and chart aggregates — IFS family, statistics, SUMPRODUCT, LARGE/SMALL/RANK ... Full GROUPBY/PIVOTBY need a dynamic-array "spill" model (a formula returning a grid), which this single-cell engine deliberately does not have. These are the single-cell building blocks that do the same work. Multi-criteria conditional aggregates (the "filter then aggregate" pivot core), resolved positionally so criteria ranges stay aligned: - SUMIFS(sum_range, criteria_range1, criteria1, ...) - AVERAGEIFS(avg_range, criteria_range1, criteria1, ...) (#DIV/0! on no match) - COUNTIFS(criteria_range1, criteria1, ...) - MAXIFS / MINIFS (0 on no match, like Excel) Mismatched range sizes are #VALUE!, not a silent misalignment. Chart statistics (over the flattened numeric arguments): - MEDIAN, MODE (ties keep the smallest value), and the sample/population STDEV / STDEVP / VAR / VARP. Sample forms divide by n-1 (#DIV/0! for a single value), population by n. Pivot/ranking helpers: - SUMPRODUCT(array1, [array2], ...) — the element-wise dot product; text counts as zero, errors propagate, mismatched sizes are #VALUE!. - LARGE / SMALL(array, k) — k-th largest/smallest; k out of range is the new #NUM! error. - RANK(value, array, [order]) — descending by default, ascending on any nonzero order, tied values share a rank (RANK.EQ). `FormulaError` gains `NumError` (`#NUM!`) for out-of-domain numeric arguments, rounding out the error surface after `Na` in the lookup tranche; it round-trips through display/parse and propagates from cached values. Tests: 10 unit tests (multi-criteria aggregation, statistics, dot products, top-N/ranking, argument/size errors) + an end-to-end test proving the dependency graph tracks every range and recalculates the pivot formulas when a source cell is edited. Engine unit tests 395 -> 405. Engine coverage 97.34% (floor 96).	2026-08-18 18:03:00 +00:00
andodeki	c1d1e67f3a	feat(pdf): shadings — the sh operator was parsed and thrown away ... ADR 0028, the first of Phase 7's eight bullets. content.rs contained `PdfOp::Shading(_name) => {}`. The operator was lexed, given its own variant, matched during interpretation, and discarded. A page whose background is a gradient rendered as nothing. Nothing caught it for the usual reason: a blank region is a legal thing for a page to contain, so "drew nothing" and "drew what was asked" are indistinguishable without an assertion naming the expected colour. The golden corpus had no shading page, so there was nothing to be wrong. Two of the three pieces already existed — function.rs evaluates the colour function and colorspace.rs converts it to RGB. What was missing was the geometry between them. Sampling rather than a gradient primitive: a PDF shading is defined by an arbitrary function, possibly a sampled table or a PostScript program, and neither reduces to a stop list without loss. A device with a native gradient can still recognise the two-stop case from the samples. "No colour here" is None, not black. Black is a colour a shading can legitimately produce, so returning it for "outside an unextended shading" would paint a rectangle the author never asked for and the caller could not tell the two apart. Types 1-5 exact. Coons and tensor patches are flattened to their corners, which loses the curvature, and is_approximate says so rather than leaving a caller to assume fidelity. An unknown type is refused by number: a mesh drawn as a flat fill is a plausible-looking wrong answer. paint_shading is a new trait method, so the compiler found every implementor. The Makepad renderer records the request in pending_shadings, mirroring pending_xobjects — it cannot resolve a /Shading resource because it does not own the page dictionary, and recording the request is what stops the operator vanishing a second time. That holds even for types we refuse, so a host can warn the user. Four mutations, all killed. The first — discarding sh again — fails three tests. Stated plainly and left unticked: the mesh path is written but NOT exercised by any real stream. shading.rs is at 68% and the uncovered part is exactly parse_mesh and triangulate. Mesh support should be treated as unproven, not working: the code runs and produces triangles, and nothing yet demonstrates they are the right triangles. That is the position image.rs was in before ADR 0016 found the JPEG decoder was a stub. The Phase 7 status line is a table from the start this time — one row per spec bullet, seven of them saying "not started". Per ADR 0021, written before the work rather than after it. pdf: 1321 passed (was 1291). pdf-ui: 1366. Coverage 87.60%, floors met.	2026-08-18 17:30:39 +00:00
andodeki	cb8912f762	test(pdf): close the two real coverage gaps in the signing module ... Asked to verify Phase 6 was complete *with test coverage*, I measured sign.rs per function rather than trusting the file-level 82%. Most of the apparent gap is error arms inside covered functions — llvm-cov attributes each `map_err` closure separately — but two things were genuinely untested, and one of them was not code that should exist. algorithm_name() was dead. It returned a &'static str describing the algorithm and nothing called it: `algorithm()` supersedes it, returns a type rather than a string, and is what the CMS writer actually uses. Deleted rather than tested, because a test would have preserved code whose only caller was the test. SigningError's Display impl was never exercised. These strings reach a user through a host application. ContentsTooSmall in particular must carry both numbers — a caller cannot raise the reservation without knowing by how much — and that is now driven through the real signing path with a 32-byte reservation rather than by constructing the error. sign.rs 82.07% -> 83.79%. pdf: 1291 passed. Coverage 88.03%, floors met.	2026-08-18 17:08:11 +00:00
andodeki	7737096858	refactor(makepad-table): adopt Robrix's image decode path ... Replaces the hand-rolled try-PNG-then-JPEG with `pageflipnav/src/utils.rs::load_png_or_jpg`, the pattern the Robrix-derived app in this repo already uses. Two things it does better: - It sniffs the header with `imghdr` and calls the matching loader directly, so a JPEG does not decode-and-fail as a PNG first on every cold cache. - It still falls back to trying both when the sniff names something unexpected or nothing at all. `imghdr` is not perfect, and a mislabelled file is more useful decoded than refused. `imghdr` has no transitive dependencies — it reads a header and names a format. It is already a dependency of `pageflipnav` at the same version. The upstream version logs the failure and dumps the bad bytes to disk. That is right for a chat client receiving untrusted media and wrong here: this runs from the draw path for every attached cell, so a broken file would log once per frame. The caller already caches the failure and draws a labelled chip naming the file, which tells the user more than a log line would. Tests 94 -> 99. Verified by removing the sniff and by removing the fallback; each fails the ordering test. `TextOrImage`, the other candidate for reuse, is referenced in `room_screen.rs` but not defined anywhere in this checkout — it is upstream Robrix only, so there was no baseline here to adopt.	2026-08-18 16:51:47 +00:00
andodeki	01ebeeb7fe	feat(makepad-table): real image rendering with a resize anchor, and per-row heights ... Two things: attached images are decoded and drawn rather than shown as a placeholder chip, and row heights become genuinely per-row. **Image rendering.** Decoding follows `pageflipnav/src/utils.rs`, the Robrix-derived app in this repo: try PNG, then JPEG, because a header sniff is not reliable enough to choose on its own. The decoded texture is cached per cell, and a cache entry of `None` records a file that could not be read so a broken path is attempted once rather than every frame — `draw_walk` runs at 60Hz and re-decoding a photo there would be the slowest thing in the widget by a wide margin. One `Image` widget repositioned per cell, matching `cell_editor` and `math_cell`, with the texture swapped from the cache. A pool would let several textures live at once but needs runtime template instantiation and a reuse policy; this is the same number of GPU uploads with far less machinery. On first successful decode the real pixel size is written back to the attachment, so the row is sized from the true aspect ratio instead of the placeholder guess. **The resize anchor.** A grab square at the image's bottom-right corner. Dragging it writes `ImageSizing::Fixed`, which pins the height so a later relayout cannot overrule what the user chose, and the row follows because `row_height_for` reads the same value. The floor is asserted at compile time against the anchor size: an image dragged smaller than its own grab handle could not be grabbed again, and the user would have to delete the attachment to recover it. The anchor is hit-tested before the cell, or dragging it would open the editor instead. Sizing lives on the attachment rather than in widget state, so it survives a column reorder along with the image. **Per-row heights.** `TableRow::height` holds a dragged override and `row_height_for` honours it. Item 5's row resize previously assigned `self.row_height`, which is table-wide — dragging one row's handle resized every row at once. `RowGeometry`, added with the attachments, now carries the consequence: rows below a resized one shift down. Tests 86 -> 94 (140 across the tree), all five crates clippy-clean. Verified by reintroducing four defects. One of those guards did not work first time and the gap was mine. Deleting the per-row override branch from `row_height_for` left the whole suite green: the tests checked that `TableRow::height` could be *stored*, and nothing checked it was ever *read*. Storing a value no one consults is exactly the shape of "the handle does nothing". `the_row_override_is_actually_consulted` now asserts the connection at both ends — that `row_height_for` reads `row.height`, and that the resize writes it rather than the table-wide field.	2026-08-18 15:19:36 +00:00
andodeki	3c751f18bd	feat(makepad-table): cell attachments and per-row heights (item 6) ... The last of the nine. Items 1-5, 7, 8 and 9 shipped in 1887b54 and bbdfe82. **Per-row heights.** Row positions were `index * row_height`, which is only correct while every row is the same. A cell holding an image is not, so `RowGeometry` now accumulates boundaries the way `ColumnGeometry` does, and hit-testing scans them instead of dividing — with unequal heights there is no divisor. Ten call sites moved over. **Attachments.** A long press on a cell opens its menu: copy, paste, clear, add image or PDF, take a photo, add location, remove. Cells stay `String`. An attachment is a side-table on `TableData` keyed by `(row, col)`, so a table without them costs nothing and still round-trips as text — every existing caller builds `TableData` from strings and none of them change. Those keys are positional, which is the part that bites: inserting a row or reordering a column has to move them too, or the data moves and the image stays behind. `shift_attachments_for_row_insert`, `_row_remove` and `_col_move` handle it, and `move_column` swaps rather than shifts because it is a swap. This is the class of bug that only appears once there is real content in the table, so it is tested directly rather than left to review. **Why a provider trait.** Camera and location live in `nigig-uikit`, which pulls `nigig-core` and with it tokio, reqwest, matrix-sdk, clap, image and csv. For a widget whose only dependency is `makepad-widgets`, that is the wrong trade. `CellAttachmentProvider` names the three capabilities and a host that already has them supplies them; the default implementation declines everything, and declining emits `AttachmentUnavailable` rather than leaving a menu entry that silently does nothing. File picking ships with the widget, because `robius-file-picker` is already a dependency of three crates here, works through `rfd` on desktop and the platform picker on Android, and carries none of that weight. Its callback runs off the UI thread, so results are parked in `PENDING_CELL_FILE` and drained on `Event::Signal`, the same shape the invoicer and the SMS bulk import already use. **What is drawn.** A labelled chip, not the image. Decoding a photo or rasterising a PDF page per frame belongs in a texture cache the host owns, and a rasteriser is not something this widget should carry. The chip reports what is attached and takes the space the row grew for it; a host wanting a thumbnail draws over the same rect. Tests 72 -> 86 (132 across the tree), all five crates clippy-clean. Verified by reintroducing four defects: attachments not following a column reorder, not following a row insert, row lookup dividing by a uniform height, and an image ignoring its aspect ratio.	2026-08-18 13:52:41 +00:00
andodeki	bbdfe823f8	feat(makepad-table): row gutter, header select/resize, long-press menus, one input model ... Items 1, 2, 3, 4, 5, 7 and 9 of the nine reported. Item 8 shipped separately in 1887b54; item 6 (cell attachments) is next and needs the trait hook agreed for camera and location. **One input model for mouse and touch (item 9).** The touch path was a parallel implementation with its own tracker, its own long-press timer and its own movement threshold, and the two had already drifted: a long press opened a menu under a finger and did nothing under a mouse. Both paths now share `PressTracker`, `handle_press_timers` and `dispatch_target_click`. Only gesture recognition differs, because a finger reports Start/Move/Stop with no tap count and no hover — double-tap is derived from the interval between taps, the way the CAD viewport derives its gestures from raw touches. A held press schedules its own frame with `new_next_frame`. Neither a mouse nor a finger emits events while held still, so a long press would otherwise only fire if the user happened to move. **Double-click to edit (item 3).** A single click now selects; the second click of a double-click edits. `DOUBLE_TAP_WINDOW` is deliberately shorter than `LONG_PRESS`, so a slow double tap cannot also register as a long press and both edit the cell and open the menu. **Long press opens header menus (item 4).** Previously a single click did, which left no gesture free for selection. **Header selection and resizing (item 5).** A click on a header outlines it and shows a grab handle — right edge for a column, bottom edge for a row. Dragging the handle resizes, with a full-length guide while the drag is live. The size is floored at `MIN_SIZE`, which is not cosmetic: a header dragged below the handle size cannot be grabbed again, so the column would be unrecoverable by dragging. **Row gutter (items 1 and 2).** A leading column numbers the rows and acts as the row header. It is not a data column — no entry in `columns`, so it cannot be reordered or dropped onto — and the numbers are derived from the index each frame rather than stored, so an insert or reorder cannot leave them stale. This replaces the old 16px handle strip, which sat inside the first data cell and stole clicks from it. **Every grid position is drawn (item 1).** The draw loop iterated each row's own `cells`, so a row shorter than the column list simply stopped: the remaining columns had no background, no border and nothing to click. It now iterates the column count and treats a missing entry as empty. The rightmost vertical divider is also drawn; the old range stopped one short and left the last column open. **Header renaming (item 7).** Reached from the header menu rather than bound directly to the long press, so one gesture does not mean two things. It reuses the same `cell_editor` as the cells, which means the colour pinning, caret placement and focus deferral fixed earlier all apply to it for free. Row headers are deliberately not renameable: a row's label is its position, and changing it would mean introducing a stored row title the data model does not have. Tests 63 -> 72 (118 across the tree), all five crates clippy-clean. Verified by reintroducing four defects: removing the resize floor, removing the self-scheduled frame, widening the double-tap window past the long press, and re-forking the touch threshold.	2026-08-18 13:18:31 +00:00
andodeki	1887b54efa	fix(makepad-table): repaint when the cell editor takes focus (item 8) ... The editing cell's text did not appear until the pointer moved. `begin_edit` cannot focus the editor directly. `set_key_focus` takes an `Area`, and the editor only has one once it has been drawn, so focus is deferred: `begin_edit` sets `needs_editor_focus` and the next `handle_event` calls `set_key_focus` on the now-valid area. That deferred step changed how the editor draws — the caret starts blinking and the focused colour states apply — but it never asked for another frame. The editor therefore kept painting its unfocused appearance until something unrelated triggered a redraw. Moving the mouse was that something, which is why the text appeared only after moving the pointer away. One `self.redraw(cx)` after focus is taken. Tests 62 -> 63; verified by removing the call, which fails the new test. This is item 8 of nine reported together. The other eight are new capability — a row-number gutter, header editing, resize handles, a cell context menu with attachments, and a touch path — and are being scoped separately rather than bundled into a bug fix.	2026-08-18 13:04:21 +00:00
andodeki	374af5ccad	feat(pdf): the five Phase 6 bullets the status line omitted ... ADR 0027. Asked whether Phase 6 was 100% complete, I checked the plan's bullets against the code instead of answering from the status line. Five were not implemented and the status line named none of them: detached/ATTACHED signatures /SubFilter hardcoded to adbe.pkcs7.detached PAdES basics ETSI.CAdES.detached was a string in a match external_signing_test.dart absent; SigningIdentity needs an in-memory key OCSP/CRL lookup CRL only; OCSP counted, never parsed Fulcio identity absent (optional in the plan) This is the second time. ADR 0021 recorded the same failure in Phase 4 and wrote the rule meant to prevent it — enumerate criteria from the plan text first, then mark each done or explicitly deferred. I wrote that rule and then produced another prose summary of what I had built. A summary written from the work cannot show what the work omitted. PAdES is a real profile, not a label. CAdES signs a set of signed attributes, one carrying the document digest, and the signature is over those attributes re-tagged as a SET (RFC 5652 5.4) rather than over the [0] IMPLICIT SEQUENCE they are carried in. Verification checks the messageDigest attribute against the document as well as verifying the attribute signature; without that, a signature over somebody else's digest would be accepted. /SubFilter now comes from the profile, so a document cannot claim CAdES while carrying plain PKCS#7. ExternalSigner is a trait: bytes in, signature out. A smartcard or KMS never hands out its key, so SigningIdentity could not represent one. SigningIdentity implements the trait rather than sitting beside it, so there is one signing path — a second path for hardware keys would be a second place the byte range could be computed differently. OCSP is decoded with the der crate already present rather than adding the ocsp crate for two fields. Revoked from any response beats Good from any other. Attached signatures are REFUSED, not deferred. Both attached profiles (adbe.pkcs7.sha1, adbe.x509.rsa_sha1) are SHA-1 based, and SHA-1 is broken for signatures. They are parsed so such documents can be read; they cannot be written, enforced by the absence of a SignatureProfile variant. Same decision as RC4 in ADR 0024. Recorded as refused rather than not-done, because "not done" invites someone to finish it. Four mutations, all killed first attempt: messageDigest not compared, CAdES verified against the wrong bytes, /SubFilter hardcoded again, OCSP revoked read as good. The status line is now the plan's own bullets in a table, one row per spec item, not prose. Two wrong status lines in the same direction is a pattern, and the fix is structural: a missing row is visible, a missing sentence is not. Four rows are left unticked — Fulcio, independent review, Acrobat interoperability, and signing a document that already has an AcroForm. qpdf accepts documents under both profiles. pdf: 1289 passed (was 1276). Coverage 87.96%.	2026-08-18 12:06:59 +00:00
andodeki	780e323674	refactor(spreadsheet-ui): headless formula-bar state machine, wired to the workspace ... The formula bar's state — formula text, target cell, dirty flag, and the "ignore the next change" latch — lived as five scattered fields plus inline logic inside workspace.rs, a `script_mod!` DSL file that no unit test can construct. That made the formula bar the one piece of the UI whose behaviour was untestable headlessly. Extract a `formula_bar` controller module (the same pattern as the earlier geometry.rs extraction) and wire the workspace to it: - `FormulaBar` owns text/target/dirty/latch with `sync_to_cell`, `on_user_input`, `insert_reference`, `take_commit`, and `sync_after_commit`. The workspace now only bridges it to the Makepad text input; the change/return/focus-loss/undo-redo handlers route through the controller unchanged in behaviour. - Point-and-click formula building (new, Excel-style): while the formula bar is focused, selecting a cell appends its reference instead of replacing the formula being edited. `reference_for(sheet, row, col)` and `quote_sheet_name` build `A1`, `Sheet2!A1`, or `'My Sheet'!A1` with Excel's quoting rules (bare identifier unquoted; spaces, punctuation or a leading digit quoted; embedded quotes doubled) — the machinery that makes the cross-sheet reference syntax buildable from the UI. - The coverage script now measures formula_bar.rs alongside the other controller modules. Tests: 11 new headless unit tests (quoting, reference construction, sync/latch/commit round trip, and insertion into empty/value/formula bars). UI lib tests 62 -> 73, all pass; ui-controllers coverage 98.74% (floor 96) with formula_bar.rs at ~98%.	2026-08-18 11:40:49 +00:00
andodeki	d0e7ede0c1	feat(spreadsheet): cross-sheet named ranges (Data!Total) ... Completes the cross-sheet reference feature with `Sheet!Name`, the one shape the previous tranche explicitly left out. Parser and AST: - `Sheet2!Total` and `'My Sheet'!Sales` parse as a new `Expr::SheetNamedRange { sheet, name }` node. In `parse_sheet_qualified_ref`, an identifier after `!` that is not a valid cell reference is treated as a named range; a cell reference wins when both readings are possible (`Sheet2!A1`), matching Excel. Evaluation: - `EvalContext` gains `get_sheet_named_range` (default `None`). `evaluate` resolves a `SheetNamedRange` to its range and returns the first cell in expression context, while `resolve_arg` expands it to every cell for aggregates — so `Data!Total` reads one cell and `SUM(Data!Total)` sums the whole range. An unknown name on a real sheet is `#NAME?`, and `DataEvalContext` looks the range up on the sibling sheet case-insensitively (or on the sheet itself, through the intra-sheet path). Dependencies: - A named-range reference is a coarse sheet-level dependency, because its bounds live on the target sheet. `SpreadsheetData` tracks `cross_sheet_named_refs` (cell -> sheet names), rebuilt by `rebuild_dependency_graphs` / `update_dependency_graph` alongside the cell-level `cross_sheet_refs`, and the workbook folds both into its sheet-index dependency map. Editing a cell inside the named range therefore recalculates the reading sheet. Tests: parser/evaluator units (incl. quoted names, cell-vs-named precedence, dependency extraction) and workbook end-to-end (SUM over a cross-sheet named range with propagation on edit, quoted names, and the #NAME? case). Engine unit tests 390 -> 395; UI lib tests still pass. Engine coverage 97.56% (floor 96).	2026-08-18 11:07:57 +00:00
andodeki	593cd8fee8	feat(spreadsheet): cross-sheet references (Sheet2!A1) ... The engine's dependency graph is per-sheet by design, so this was the structural change: a formula can now read a cell on another sheet, and edits to the source sheet recalculate the readers. Parser and AST: - New tokens: `Bang` (`!`) and `SheetName` (`'My Sheet'`, with `''` as an escaped quote). `!=` still tokenizes as not-equal. - New AST nodes: `SheetCellRef { sheet, cell }` and `SheetRange { sheet, range }`, parsed from `Sheet2!A1`, `Sheet2!A1:B2`, `'My Sheet'!B3`, before the cell-ref fallback (a bare `Sheet2` would otherwise parse as a bogus cell reference). - `evaluate` and `resolve_arg` resolve them through two new `EvalContext` hooks, `get_sheet_cell_value` / `get_sheet_range_values`, which default to `#REF!` in contexts without sibling sheets. Evaluation: - `DataEvalContext` gains a sibling-sheet view plus a shared cross-sheet cycle guard keyed by `(sheet, row, col)`. Reading a sibling routes to a child context pointed at that sibling; re-entering the same cell on the same sheet mid-evaluation reports `#CYCLE!`, and a reference back to the sheet being recalculated is caught through the guard. - `recalculate_all_with` / `evaluate_formula_with` accept the sibling view; the plain per-sheet entry points are unchanged. Recalculation and dependencies: - `SpreadsheetData` tracks `cross_sheet_refs` per cell (rebuilt by `rebuild_dependency_graphs` / `update_dependency_graph`), and the workbook flattens it into sheet-index edges. Editing a sheet recalculates — with sibling access — the transitive closure of sheets that read it, plus the sheet itself (its own formulas may read other sheets). Single-sheet edits with no cross-sheet references keep the fast incremental path. - `evaluate_all` clears computed values across sheets first, then runs one extra pass per sheet, so acyclic chains propagate and mutual cross-sheet cycles terminate with `#CYCLE!`. - Cross-sheet formulas survive save/load: deserialization runs a sibling-aware pass, and a missing sheet renders `#REF!`. Tests: parser/evaluator units (incl. quoted names, dependency extraction) and workbook end-to-end (read + propagation, a three-sheet chain, quoted names, missing sheet, save/load round trip, cycle termination). Engine unit tests 382 -> 390; UI lib tests still pass. Engine coverage 97.55% (floor 96).	2026-08-18 10:53:08 +00:00
andodeki	99aebc202a	fix(pdf): security review of the signing code — a forgery verified as valid ... ADR 0026. Both ADR 0024 and ADR 0025 said this code needed a security review before shipping. This is that review, done adversarially: for each way a signature could be defeated, a test that attempts it. It found a critical vulnerability in the code as shipped last turn. FINDING 1, critical, exploitable with no special access. Verification recovered the certificate and the signature by *scanning* the blob for DER-shaped bytes rather than decoding it. The signature was checked against certificates[0]; trust was checked against ANY certificate present. Two questions, two different certificates. So: the attacker signs a forgery with their own key the attacker appends the victim's trusted certificate to the blob signature_valid = true (their signature over their own content is real) chain_trusted = true (the victim's certificate is present) is_valid() = true Demonstrated before the fix, with the message "I hereby transfer everything to the attacker" verifying as valid. Fixed by decoding the ContentInfo/SignedData structure and finding the certificate the SignerInfo actually names, by issuer AND serial, then evaluating both the signature and the trust path against that one certificate. Trailing data now fails the decode instead of being ignored. The scanning functions are deleted, not left unused: dead code that once returned the wrong answer is an invitation to call it again. FINDING 2, moderate. signer_certificate() returned chain[0] unconditionally, so a chain whose first entry was not the signing key's certificate made the SignerInfo name the wrong one. Not a forgery route — the signature fails — but a UI showing "signed by <somebody trustworthy>" beside a failed check is its own kind of dangerous. Now it finds the entry whose public key matches the key doing the signing. FINDING 3, informational. digest_matches was hardcoded true under a comment claiming it was computed. Not exploitable, because is_valid() also requires signature_valid and the signature covers the bytes — but a field asserting an unperformed check is ADR 0017's pattern exactly. The four items ADR 0025 left unticked are closed: PKIX chain building, with each link's issuer signature verified. A name match alone is not a chain; anyone can put any name in a certificate. Pinning still short-circuits first. Stapled revocation from /DSS, offline only. Unknown is the default and a first-class answer: treating "no information" as "not revoked" is a claim a verifier cannot support. Signature appearances, with the claimed time labelled "Time claimed" because a self-declared /M carries no authority. One-call sign_document. Three things were wrong first: the /ByteRange placeholder was too narrow for real offsets so patching them moved every later byte; /Contents must be a hex string because a literal full of NULs needs escaping and changes length; and a signature dictionary nothing points at is invisible — the first version wrote one and the reader reported zero signatures over a correctly signed document. Four mutations, all killed — two only after strengthening the tests. My first smuggling test put the attacker's certificate first, where certificates[0] finds it anyway, so it passed with or without the issuer/serial match. Putting the TRUSTED certificate first is what distinguishes them, and writing that test is what exposed Finding 2. qpdf --check accepts the signed documents. pdf: 1276 passed. Coverage 87.98%. Left unticked, deliberately: an independent review by someone who did not write the code. This is a self-review; it found two real vulnerabilities, which is evidence the method works and not evidence that nothing remains. Also untested against Acrobat, which is stricter than the spec, and sign_document replaces rather than merges an existing AcroForm.	2026-08-18 10:39:20 +00:00
andodeki	9989043a37	ci: gate the coverage that was already measured and unenforced (Phase 0) ... Phase 0 of REVIEWS/REPO_COVERAGE_100_PLAN.md, and the reason it is Phase 0: no new tests, no new measurement, just ratchets on numbers that were already good and already decaying-capable. **spreadsheet** — `tools/test-spreadsheet-coverage.sh` has had a 96 floor for the engine and another for the UI controllers, and no CI job has ever run it. New `.forgejo/workflows/spreadsheet.yml`, two jobs: engine-coverage 98.83% of lines (floor 96) ui-controller-coverage 98.85% of lines (floor 96) Split in two because the halves cost very differently. The engine is pure Rust and finishes in about three minutes; the UI half has to build Makepad's Linux backend to link a test binary. One job would hide an engine regression behind a ten-minute build. **CAD widget layer** — `cad-widget-coverage` in nigig-build.yml, deliberately REPORT-ONLY. It sits at 13.25% of 10,637 lines with six files at exactly zero, and a floor there would read as a blessing rather than a debt. What the job buys is that the number is printed on every push instead of being rediscovered in six months. The first real input test should set a floor behind it. Also corrects the plan. It claimed the doc workspace module was ungated; it is not — nigig-build.yml has run doc-workspace-coverage since before the plan was written. I had surveyed by grepping workflow files for the word "coverage" and attributed nigig-build's coverage jobs to CAD alone. I nearly committed a duplicate workflow on the strength of it. The census table was right; the prose under it was not, and the correction is in the file. One thing checked and deliberately NOT changed: the spreadsheet script appears to skip its UI half when the native packages are absent. It does not. `makepad-native-libs.sh --check` returns 1, the script runs under `set -e`, and it aborts. What misled me was reading `$?` after piping the script into `tail` — which reports tail's status, not the script's. The same class of mistake this repository's CI comments warn about; no fix was needed and none was made. Verified by running each job's exact command line: COVERAGE_TARGET=engine ./tools/test-spreadsheet-coverage.sh rc=0 COVERAGE_TARGET=ui ./tools/test-spreadsheet-coverage.sh floors met ./tools/test-cad-widget-coverage.sh 13.25%, rc=0	2026-08-18 10:20:22 +00:00
andodeki	2d7b48b72c	feat(spreadsheet): lookup functions — MATCH, INDEX, VLOOKUP, HLOOKUP, XLOOKUP ... The formula engine could compute over ranges but could not look a value up in one. This adds the Excel lookup family, plus the #N/A error they need to report "not found". New `FormulaError::Na`: - Renders as `#N/A`, parses back in `from_display`, and — a bonus fix — the recalc fast path now propagates a stored `#N/A` as an error instead of turning it into text. Functions (5): - MATCH(lookup_value, lookup_array, [match_type]) — 1-based position. Type 0 exact (case-insensitive text), 1 largest ≤ lookup, -1 smallest ≥ lookup. - INDEX(array, row_num, [col_num]) — cell at a 1-based position; a single index walks the array flat in row-major order. Out of range is #REF!. - VLOOKUP / HLOOKUP — exact or approximate (approximate takes the largest first-column/first-row value ≤ lookup, the sorted-table convention), col/row index out of range is #REF!. - XLOOKUP(lookup_value, lookup_array, return_array, [if_not_found], [match_mode], [search_mode]) — match modes 0 exact, -1 next-smaller, 1 next-larger, 2 wildcard (`*`/`?`, case-insensitive); search modes 1 first-to-last and -1 last-to-first; the `if_not_found` fallback is evaluated lazily, only when nothing matches. Binary search modes are rejected with a clear error rather than silently mishandled. Lookups index ranges by position, so they keep their arguments as AST nodes (a new `range_from_arg` resolves Range and NamedRange expressions) instead of flattening through the aggregate path. Tests: 9 new unit tests (exact/approximate/wildcard/search-direction/ error arms) + an end-to-end test proving the dependency graph tracks the lookup table and recalculates dependents when a table cell is edited. Engine unit tests 373 -> 382.	2026-08-18 10:13:51 +00:00
andodeki	3928063392	ci(email): raise the domain floor; record the finance-email path ... email.yml: FLOOR 225 -> 230. The review doc records the email-to-finance sharing and notes the chat/Matrix path remains unbuilt (matrix_client has login+sync only).	2026-08-18 10:07:54 +00:00
andodeki	383533da36	feat(email): email the trip report to the finance department ... build_report_email (pure, tested) attaches the report PDF as application/pdf in a multipart message to a comma-separated finance recipient list. spawn_email_trip_report fetches the inbox, extracts trip receipts, builds the report, and emails it via the signed-in SMTP account; the proxy backend reports 'attachments unsupported' honestly rather than failing silently. The Finance card gains a recipients field and an 'Email report to finance' button. An end-to-end test sends the attached PDF through the SMTP sink and asserts the recipient, application/pdf type and filename land in DATA. Domain tests 234 -> 237.	2026-08-18 10:07:54 +00:00
andodeki	26f6d431fb	feat(spreadsheet): date and time functions, with a date display format ... The formula engine had no notion of dates. This adds an Excel-style date/time model and the functions to work with it. Date model (new `dates` module): - A date/time is a single f64: the integer part is a day serial, the fraction is the time of day. Serial 25569 = 1970-01-01, so every modern date agrees with Excel exactly. The calendar is the proleptic Gregorian (Howard Hinnant's days_from_civil/civil_from_days), so Excel's phantom 1900-02-29 (serial 60) reads back as 1900-02-28 and serial 61 = 1900-03-01 — documented rather than reproduced. - Serial <-> calendar conversion, day-of-week, leap-year and days-in-month helpers, date/time string parsing, EDATE/EOMONTH, DATEDIF(Y/M/D), and ISO formatting. Functions (17): - DATE, DATEVALUE, YEAR, MONTH, DAY, HOUR, MINUTE, SECOND, TIME, TIMEVALUE, WEEKDAY (return types 1/2/3), DAYS, EDATE, EOMONTH, DATEDIF — with Excel's month/day rollover in DATE, day clamping in EDATE, and #VALUE! for malformed strings and unknown units. - TODAY and NOW read a wall clock. The engine stays deterministic: `EvalContext` gains a default-none `now_serial` hook, and `SpreadsheetData` carries an optional `now_serial` (never serialized). Without a clock they report "requires a wall clock"; `SpreadsheetData::set_system_now` / `Workbook::set_system_now` supply the system clock, and the UI workspace model wires it on creation. Display: - `NumberFormat::Date` and `NumberFormat::DateTime` render a serial as `YYYY-MM-DD` / `YYYY-MM-DD HH:MM:SS` through write_display_value, with codes that round-trip through the existing serialization. Tests: 18 new unit tests (calendar round-trips across centuries, known serials, leap-year rules, parsing, weekday schemes, EDATE/EOMONTH clamping, DATEDIF, ISO formatting) + 6 evaluator tests + an end-to-end test covering format rendering and recalc through the dependency graph. Engine unit tests 352 -> 373. UI lib tests still 62 pass with the new clock wiring.	2026-08-18 10:00:53 +00:00
andodeki	9a5ce9c0e6	feat(pdf): signing and verification — Valid becomes reachable, with a policy ... ADR 0025, completing Phase 6's functional core. This partially reverses ADR 0010, which refused signing and cryptographic verification outright, and it reverses only the half whose justification expired. ADR 0010 gave two reasons. The first — a parsing library has no business signing — stopped being true when Phase 4 began creating documents and Phase 5 editing them. The second is still true and is preserved intact: deciding which certificate authorities to trust is a policy decision that belongs to the host, not to a parsing library So VerificationStatus::Valid is still not reachable by default. Verification returns three independent booleans and is_valid() needs all three; the third, chain_trusted, can only become true through a caller-supplied TrustAnchors. There is no TrustAnchors::system(), no bundled root store, no Default that trusts anything. A caller with no policy is told "cryptographically intact, signed by somebody you have not said you trust" — a different fact from "forged", and a host that cannot tell them apart shows the wrong thing to a user. RSA PKCS#1 v1.5, ECDSA P-256 and Ed25519, all with SHA-256. PSS is stronger and not universally accepted by PDF verifiers, so v1.5 is what is written. Ed25519 carries an interoperability caveat in the doc comment on the variant itself, because that is where someone choosing it will read it: ISO 32000-2 does not list it and most desktop viewers will reject it. No network. Revocation is not implemented rather than smuggled in: the engine crates are CI-gated against reaching outward, and that gate is a rule about layering, not an obstacle to work around. Every test generates a real key and a real certificate at run time. Nothing asserts against a checked-in blob — a fixed expectation only proves the code still does what it did, which is the wrong question for a signature. The tampering tests assert the signature verifies FIRST, then flip a bit; without that half they could pass by never verifying anything. Four mutations, all killed. The one that matters is the first: making an empty anchor set confer trust is exactly the regression that would turn this back into the thing ADR 0010 refused, and it fails immediately. Two bugs the tests found: UTCTime cannot encode a year past 2049 (RFC 5280 4.1.2.5.1). The first fixture used a 2096 expiry and every certificate failed to encode. The certificate scanner assumed a two-byte DER length. RSA certificates are large enough to use that form, so RSA and P-256 passed while Ed25519 found no certificate at all — its certificate is small enough for the short form. A scanner tested only against the largest input fails silently on the smallest. 72 dependency packages pulled in, zero non-compliant licences, no C. Stated plainly and left unticked in the ADR: chain_trusted is anchor identity matching, not PKIX path building. Correct for certificate pinning, a false negative for a real CA hierarchy. Also outstanding: revocation, signature appearance generation, and one-call incremental signing. pdf: 1247 passed. Coverage 88.08%, floors met.	2026-08-18 09:45:01 +00:00
nigig-ci	6e784fffee	fix(ci): the sample_thread gate was inverted under pipefail ... The 'Development sample data must not reach the UI' gate ran count=$(grep ... | wc -l) under set -euo pipefail. When sample_thread is correctly ABSENT from the UI, grep returns exit 1 (no matches), pipefail propagates it, and set -e kills the script -- so the gate reported FAIL in the GOOD state and would have passed in the BAD state. Add || true so a zero-match result is counted as 0 and the gate passes, as intended. Verified: all 11 gates now pass, and sample_thread is confirmed gone from the UI crate (only in email_store.rs, definition + tests).	2026-08-18 09:36:35 +00:00
andodeki	b5ff5dcf40	feat(spreadsheet): text manipulation and IS* info functions ... The formula engine could count, sum, compare and test conditions, but had almost no way to work with text (only CONCAT/LEN/UPPER/LOWER) and no way to ask about a value's type. This adds both. Text functions (operate on the display form of their arguments): - TRIM — collapse internal space runs and drop leading/trailing spaces - LEFT / RIGHT — first/last n characters (default 1) - MID — n characters from a 1-based start - SUBSTITUTE — replace all occurrences, or just the nth instance - FIND / SEARCH — 1-based position, case-sensitive vs case-insensitive, #VALUE! when not found or start is out of range - REPT — repeat n times, capped at Excel's 32767-character result - PROPER — title-case each word Info functions (never propagate their argument's error, like Excel — ISERROR reports it, the others treat it as FALSE): - ISNUMBER / ISTEXT / ISNONTEXT / ISLOGICAL / ISERROR - ISBLANK — TRUE only for an absent cell. This needs the distinction between "missing" and "holds 0", so EvalContext gains a `cell_is_blank` method; DataEvalContext overrides it with a direct cells lookup (an absent cell reads as Number(0.0) through get_cell_value, but is blank, whereas a real 0 is not). Dependencies flow through the existing AST walk, so a TRIM/LEFT/etc. reference is tracked and recalculates when its source cell is edited — pinned by an end-to-end test through SpreadsheetData. Engine unit tests 345 -> 352.	2026-08-18 09:35:19 +00:00
andodeki	b93dc485b9	test(spreadsheet): cover public workbook serialization round trip	2026-08-18 08:31:17 +00:00
andodeki	b7eacb2a94	test(spreadsheet): cover public multi-sheet evaluation	2026-08-18 08:27:13 +00:00
andodeki	4deefabd0a	test(spreadsheet): cover public style commands	2026-08-18 08:24:09 +00:00
andodeki	a099ab9980	test(spreadsheet): cover public workbook lifecycle API	2026-08-18 08:02:17 +00:00
andodeki	2824b49f0e	test(spreadsheet): cover public workbook deserialization API	2026-08-18 07:44:23 +00:00
andodeki	1629994de7	test(spreadsheet): add public persistence integration test	2026-08-18 07:30:43 +00:00
andodeki	d4e3e9a443	feat(pdf): encryption on save — AES-128 and AES-256 (Phase 6, part one) ... ADR 0024. This reverses ADR 0005's "never write encryption", and the reason it is safe to reverse is that the facts changed underneath it. A crate that only reads cannot produce weak ciphertext, so refusing to write any was free. Now that Phase 4 creates documents and Phase 5 edits them, the refusal does something worse than protect nobody: open a password-protected file, change one annotation, save, and the output is plaintext. No error, no warning — the protection is silently dropped. That is this project's recurring failure mode in the one place where the consequence is a breach. The principle survives in a narrower form: no hand-rolled crypto, and no weak cipher offered as an option. RC4 stays readable because files use it and is not writable — EncryptionAlgorithm has no RC4 variant, so the refusal is a type, not a runtime check someone can route around. The encryptor is the literal inverse of the decryptor and imports its primitives rather than restating them; two implementations of one algorithm drift, and here they drift towards "decrypts to garbage". Every unit test round-trips through the existing Decryptor. Encryption sits at one choke point: PdfWriter holds the Encryptor and write_object_at encrypts everything passing through. Not per call site — there are twenty-two of those in PdfDocBuilder, and one stream written in the clear inside an encrypted document is not a partial failure, it is a leak that no reader will report because the file is otherwise valid. The /Encrypt dictionary is the single deliberate exemption: it holds the salts a reader needs before it has a key, so encrypting it bricks the file. Verified against implementations we share no code with, now gated in CI: ok qpdf opens it with the password ok it really is AES-256 ok the wrong password is refused ok poppler decrypts the content ok no plaintext in the encrypted file Four mutations, all killed — two only after the tests were strengthened, and both misses are the interesting part: A fixed IV survived two_saves_of_one_document_are_not_byte_identical, because the AES-256 file key is fresh per save and that alone makes the output differ. The property actually needed is narrower: one encryptor, identical plaintext, different bytes. In CBC a repeated IV under one key leaks that two plaintexts are equal. A wrong /Length survived because our own reader recovers by scanning for endstream — a robustness fix from ADR 0023. An independent reader that trusts /Length reads a truncated stream and decrypts garbage. A lenient reader hides a broken writer, which is why the external gate exists. The /Length test itself had a bug first: it searched a from_utf8_lossy view and reported a stream declaring 80 bytes holding 156. Ciphertext is not UTF-8; the replacement characters shifted every offset. Unencrypted output stays byte-reproducible; encrypted output cannot be, and a test asserts that loss rather than leaving it implicit. pdf: 1220 passed (was 1187). pdf-ui: green. Coverage 88.21%, encrypt_write.rs at 96.5%. Signing is NOT started. It needs the trust-anchor decision ADR 0010 deferred: VerificationStatus::Valid is unreachable by construction, and making sign -> verify pass is a policy change, not an implementation detail. The plan's Phase 6 status now says so.	2026-08-18 07:27:45 +00:00
andodeki	118fbefe9a	test(spreadsheet): move format detection test to integration suite	2026-08-18 07:07:12 +00:00
nigig-ci	ee8d10d978	fix(pay): the M-Pesa PIN field has been visible by default since 69f6fb2 ... The first thing the newly registered runner found. tools/check-no-pin-capture.sh fails on main: FAIL: pin_input is not hidden by default in the DSL (review item 0.3: hiding must survive a DSL reload) Reproduced locally, so this is a real defect and not runner flakiness. 69f6fb2 -- "fix(map): NigigMapView packed vertex shader for new DrawVector; fix pay sheet visible" -- deleted TWO lines from shared_pay_sheet.rs. Its message says it removed `visible: false` from `pin_eye_btn := Button` because "Button does not have DSL property visible". Whatever the merit of that for the Button, the same commit silently took the line off `pin_input` as well, and pin_input is a RobrixTextInput, which does support the property -- 6f05c47 had added it there deliberately eight days earlier for exactly this reason. Consequence: both M-Pesa PIN controls have rendered by default in every non-demo build since. The runtime code only ever calls set_visible(cx, true), and only under #[cfg(feature = "demo")] -- nothing hides them -- so the DSL was the entire mechanism. The comment three lines above still read "The DSL hides the PIN controls by default so a reload cannot surface them", which had become false. That is review item 0.3 / defect B11: the hiding must survive a DSL reload, because a hot-reloaded or re-instantiated sheet re-applies the definition and a runtime-only call does not run again. Restored on both controls. On the Button: the stated reason for removing it does not hold in this tree -- nigig-build's cad/mod.rs has three Buttons carrying `visible: false` in the DSL (workspace_split_axis_btn, desktop_workspace_sync_toggle_btn, ai_cancel_button) and they do not warn. Verified `cargo check -p nigig-pay-ui --lib` is clean with both lines back. Also hid the parent `pin_visibility_row`, so the container and its children agree and a future edit to one child cannot re-expose the field on its own. tools/check-no-pin-capture.sh now passes end to end: PIN controls are hidden by default in the UI definition no PIN capture in a packaging build; the default retains it deliberately Worth stating plainly: this shipped because a map shader commit touched a payments file and no CI ran. The gate that catches it has existed the whole time.	2026-08-18 07:05:04 +00:00
andodeki	ad3fe19b90	docs(pdf): the four missing ADRs — codecs, Phase 4 completion, editing, redaction ... Ten PDF feature commits landed without an ADR, covering three whole phases. Every feature gets one; these are the four that were owed. Written against the code as it stands and re-verified by running it, not transcribed from the commit messages. 0020 CCITT, JBIG2 and JPEG 2000 — the codecs ADR 0015 refused by name 0021 Phase 4 completion — stamping, reconciliation, CFF, cmap, and the audit that corrected a false "complete" in ADR 0019 0022 Editing — content_edit, page_ops, flatten, catalog_edit 0023 Redaction and compaction, and the three reader defects they found Verified rather than assumed, on the tree at f75c1cc: ccitt 30, jbig2 29, jpx 44 unit tests; jpx_roundtrip's 7 compare against OpenJPEG-generated pixels, exactly, because a JPEG 2000 bug produces a plausible image rather than an error. Redaction re-checked with a test written from outside the module. The middle line is the one worth keeping: after redact, secret present = true <- earlier revision after compact, secret present = false public text retained = true Redaction alone leaves the secret in the bytes. That is why the two shipped together, and why the report carries earlier_revisions_retain_content. Determinism re-checked by generating the same document from four separate processes: byte-identical. The HashMap key-order defect really is fixed. This ADR contains a correction to its own first draft. 0022 initially listed text-run rewriting and annotation sub-types as deferred, on the strength of a grep for "rewrite" finding nothing. Both are implemented — the rewriter is ContentEditor::set_text, which preserves the operator kind so a ' keeps its line advance and a TJ keeps its kerning; and "callout" and "comment" are dart-pdf's names for a FreeText with a /CL and a Text annotation, not distinct /Subtype values. Checking the plan's claim against the code was right; concluding from a failed grep was not. One criterion across the four is left unticked, and it is real: the ui.rs interaction tests, blocked on the Makepad headless backend since Phase 1. 0021 also records a process note. ADR 0019's merge criteria were derived from what had been built, so all of them ticked while three unbuilt items stayed invisible. Criteria should come from the plan text first, then be marked done or explicitly deferred — a deferral stated is fine, a deferral unstated is a false claim. pdf: 1187 passed. No code changed.	2026-08-18 05:53:36 +00:00
andodeki	676b47087a	refactor(spreadsheet-ui): centralize dirty region state	2026-08-18 05:49:55 +00:00
andodeki	f75c1cc966	feat(spreadsheet-ui): model dirty render regions	2026-08-18 05:41:42 +00:00
andodeki	2ba1837055	fix(pdf): main was red — three clippy errors broke the engine build ... The PDF engine did not compile under `-D warnings`, which is what CI's engine job runs, so that job could not have passed on any of the last eleven PDF commits. The tests themselves were fine (1187 passing); the build was not. Two lints in the new jpx.rs, one in tests/filters.rs. One root cause each: jpx.rs:1279,1290 needless_range_loop on the inverse component transform. The lint's suggestion does not work here: each pass reads and writes three component planes at the same index, and `components.iter_mut()` cannot express three simultaneous mutable borrows of one Vec. Allowed locally with the reason written down, rather than restructuring correct code to satisfy a lint that has misread it. filters.rs:383 vec_init_then_push, where the lint is simply right. No behaviour change. Verified after the fix, on a clean checkout of df3c650: TEST_TARGET=pdf 1187 passed TEST_TARGET=pdf-ui 1232 passed coverage 87.98%, all floors met corpus matches generate.py; golden expectations unchanged fuzz manifest gate, no-makepad and no-process gates all pass	2026-08-18 05:41:22 +00:00
nigig-ci	632479c964	fix(ci): email.yml has been invalid YAML for six commits ... `python3 -c "yaml.safe_load(open('.forgejo/workflows/email.yml'))"` fails: mapping values are not allowed here in ".forgejo/workflows/email.yml", line 455, column 35 A workflow that does not parse does not fail -- it does not RUN. So every gate in this file has been silently absent: the S2 password checks, the multi-recipient regression check, the TLS check, the coverage floors. All of them. The file has looked like protection while providing none. Cause: the "Coverage floors" step was rewritten to call tools/test-email-coverage.sh, and ten lines of the previous inline implementation were left behind underneath the new `run:` scalar. YAML reads the first `echo "$out" | grep -E '^test result:'` as a new mapping key and gives up. Broken by 3dab4a1 and still broken at 6bf138d -- six commits, every one of which believed it was adding or tightening a gate. Verified the removal is safe: tools/test-email-coverage.sh exists, is executable, and enforces the floors itself (TOTAL_FLOOR=88 plus per-file floors), so the orphaned lines were duplicating work the script already does. Nothing was lost. The repo's own repo-hygiene.yml WOULD have caught this -- it has an "Every workflow file must be valid YAML" step, added precisely because commit 8c9ccb9 once broke nigig-build.yml the same way and silently disabled the CAD gates. I ran that step by hand against this tree and it fails, correctly. It did not catch it because no runner is registered, so repo-hygiene has never executed on these commits. That is the actual lesson here and it is not about YAML: a gate that has never run is indistinguishable from a gate that does not exist. This is the third time in this crate's history that a check existed, looked right, and was doing nothing. After the fix: YAML valid, 4 jobs / 32 steps, all 12 source gates pass, cargo check --all-targets clean on both crates, 213 email domain tests pass, fmt clean.	2026-08-18 05:33:18 +00:00
andodeki	77255965fa	test(spreadsheet-ui): measure headless controller coverage	2026-08-18 05:30:48 +00:00
andodeki	df3c650c3e	fix(coverage): invoke native preflight portably	2026-08-18 05:25:29 +00:00
andodeki	ee81d292ed	test(spreadsheet): cover literal and comparison formula edges	2026-08-18 05:22:48 +00:00
andodeki	0fcb2d3fae	test(spreadsheet): cover sum boolean text error branches	2026-08-17 19:44:27 +00:00
andodeki	4a955c5c89	docs: repo-wide coverage plan — census first, then phases ... The CAD plan covered one module. This is the level above it: all 58 workspace members, 310,287 lines. The census is the point of the document: A gated 5 crates 45,190 lines B measured, not gated 2 crates 15,373 C tested, unmeasured 27 crates 225,149 D zero tests 5 crates 4,160 E templated shells 19 crates 20,415 Three things the census turned up that were not visible from inside any one crate. Tier B is free money. spreadsheet and the doc workspace both have coverage scripts with per-file floors already written, and no CI job runs either. Same for the CAD widget layer. Fifteen thousand measured lines with nothing stopping them decaying. pageflipnav is 24,117 lines with 20 tests, no coverage tooling, and no mention in any review document in this repository. It is the worst ratio here by a distance and nobody has looked at it. The nineteen "app" crates are one program. Diff any two main.rs files after normalising the name and you get a background colour and a root screen identifier. Covering them as nineteen crates is nineteen times the work for one crate of risk; the plan asks for a decision rather than quietly doing it. On the goal itself: the only honest cost estimate available is the measured one. The CAD engine's 13,752 lines took about six sessions to reach 97%, on the easiest half of one module. Tier C is 225,149 lines, so straight-line extrapolation is ~98 sessions and the extrapolation is optimistic. That is not an argument against 100% — it is an argument that sequence matters more than destination, because the first fifth of the effort can cover most of the risk if pointed at the right code. So the phases are ordered by risk, not size: gate what is measured, measure everything else, then payments and SMS before anything larger. Phase 4 (GUI) is explicitly gated on the CAD makepad-test spike, so nobody commits three months to an approach that may not work here. Also recorded: #[coverage(off)] is unstable on the pinned 1.97.1 toolchain, verified with E0658, so unreachable code cannot be annotated away. It has to be covered, moved to an excluded file, or subtracted in the open.	2026-08-17 12:44:56 +00:00
andodeki	211ce31e9f	docs(cad): a phased plan to 100% coverage, written after measuring ... Asked for a plan to 100%. Four things had to be established first, because each one changes the plan's shape, and three of them contradict what I would have assumed. **There are already 148 widget tests and they have never run.** tests/ui.rs is 1,889 lines of #[makepad_test] tests driving the real app through TestApp/Selector. Line 1 imports a crate that is not a dependency, so the file has never compiled, and no CI job names it. Same defect the spreadsheet-ui suite documents about itself; same class as the nigig-email binary that had never been built. **They compile with a one-line manifest change, and they run.** Adding makepad-test as a dev-dependency produces a binary; under xvfb-run all 148 execute in 59 seconds without hanging. **All 148 fail at a known point.** The harness's child build of the app exits non-zero before startup: code 127 with no cargo on the child PATH, code 101 after fixing that — while `cargo check -p nigig-build --bins` passes. So the blocker is in how the harness invokes the child, not in the app, and spreadsheet-ui already documents the workaround. **#[coverage(off)] is unstable on 1.97.1.** There is no way to annotate a line as legitimately unreachable, so anything genuinely uncoverable has to be covered, moved to an excluded file, or subtracted openly. The plan puts unblocking those 148 tests first, because it is the cheapest large prize and because its outcome resizes everything after it. Engine cleanup runs in parallel since it is independent. Extraction work is explicitly held until Phase 0 reports, so nobody extracts logic the app-level tests already cover. It also argues against 100% as a target for the widget half. The 148 tests are mostly wait_visible(); they will move the number a long way while proving that widgets exist. Of the four real defects this work has found, three came from reading uncovered regions and asking why they were unreachable, not from driving a percentage. The plan targets 100% of what is worth executing and names the ~48 subtracted lines.	2026-08-17 12:40:35 +00:00
andodeki	dffa140123	fix(makepad-table): measure text with the layouter instead of estimating it ... Right-aligned text still overflowed its column after the previous fix, because that fix kept the 7px-per-character estimate and only clamped the result. Real glyphs at this size average wider than 7px, so the estimate came out short, `pos.x + width - pad - estimated_width` placed the run too far right, and it ran past the cell edge. The clamp only guards the left side. No fixed per-character figure can work here: under-measuring overflows and over-measuring leaves a visible gap. `draw_cells` now measures with the same layouter that draws the run — `DrawText::layout(..).size_in_lpxs.width`, the pattern `glass_panel.rs` uses — and both the truncation and the alignment use that measurement, so the two cannot disagree. `align_text_x` takes a width rather than a string. `fit_text_to_cell` becomes `fit_text_measured`, taking a measuring closure and bisecting for the longest prefix that fits; laying out a string is not free, and a long value in a wide column would otherwise be measured once per character every frame. Tests 59 -> 62, and the existing ones were rewritten around an injected measurer. `varied()` gives different characters different widths, as a real font does, so the suite now fails on anything that assumes a constant width. Verified against four defects. Three of the guards did not work on the first attempt, and all three failures were mine: - **Estimating the alignment width — the actual reported bug — passed.** The source check asserted `size_in_lpxs` appeared *somewhere* in `draw_cells`, and the fitting call still mentioned it after the aligning call had been replaced with an estimate. It now counts both measurements and rejects any `chars().count() as f64 *` in the draw path. - **Removing the clamp passed.** Every test reached `align_text_x` through `fit_text_measured`, and once text has been shortened to fit, the clamp never fires. `alignment_clamps_a_run_wider_than_its_cell` calls it directly with an over-wide value. The clamp still earns its place: the fitted width and the aligned width are separate measurements of separate strings, and any disagreement is what it catches. - **An off-by-one in the bisection hung instead of failing.** `lo = mid - 1` stops the interval shrinking and the loop spins forever — a frozen frame, not a wrong pixel, and no assertion catches it without a timeout. The loop is now a bounded `for` capped at the number of steps a correct bisection can need, so the same mistake produces a wrong answer that a test can see. `the_fit_search_terminates_on_hostile_input` covers degenerate measurers.	2026-08-17 12:31:43 +00:00
andodeki	b870dc4c69	feat(pdf): outline, page label and struct-tree editing — Phase 5 complete ... The last functional item. `catalog.rs` read all three; nothing could write them into an existing document. `PdfDocBuilder` can emit an outline when *creating* a file, but a document already on disk could not have its bookmarks changed. An outline is a doubly-linked tree — `/First`, `/Last`, `/Next`, `/Prev`, `/Parent` and a signed `/Count` — and every pointer has to agree. A viewer walking `/Next` and one walking `/First`..`/Last` must see the same list, or bookmarks vanish in one reader and not another with no error anywhere. Object numbers are reserved before any dictionary is built, because each item names its parent, its siblings and its children. `/Count` is signed and that matters: positive means open and counts *visible* descendants, negative means closed. A closed child contributes itself but hides its own children. Writing the total unconditionally makes every node render expanded. Page labels are a number tree, so the keys are sorted before writing and two rules starting on the same page are refused — that page's label would be undefined, and picking one arbitrarily is worse than saying so. Struct-tree editing is deliberately **removal only**. Editing the tree in place means rewriting `/K` arrays whose entries are marked-content ids inside page content streams; the tree and the content must stay in step, and changing one without the other produces a document whose accessibility information describes content that is no longer there. Removal is honest — the document stops claiming to be tagged — and `/MarkInfo` goes with it, because `/Marked true` with no tree tells a screen reader there is structure to find. Verified by mutation, seven defects, all caught: /Prev never written 1 fail /Next never written 5 fail /Count always positive 1 fail page validation skipped 2 fail /MarkInfo left behind 1 fail children not linked via /First 2 fail label rules not sorted 1 fail The last one needed a new test. Our reader walks `/Nums` linearly, so it tolerates any order and the round-trip passed unsorted — but a conforming reader binary-searches it and would label pages arbitrarily. Only reading the raw array catches that, which is the same lesson as the stale `/Count` in the page-ops tranche: our parser's tolerance hides defects that harm other readers. Engine suite 1158 -> 1187. External readers still pass. **Phase 5 is complete** but for the `ui.rs` interaction tests, blocked on the same missing Makepad headless backend as Phase 4's. The plan records the item-by-item status and, separately, the six defects the round-trip tests found in code that already existed — an unordered dictionary writer that made every generated PDF differ run to run, a short /Length that silently truncated streams, two readers disagreeing by a byte, a nested paren that truncated a string and desynchronised the stream, undecoded # escapes in names, and unknown operators being dropped outright.	2026-08-17 12:26:29 +00:00
andodeki	41c43df0e5	feat(pdf): redaction and object compaction — and three reader defects ... Phase 5's last two functional items. Together they are what makes a redaction real, which is why they are one commit: redaction removes the content, compaction removes the revision that still holds it. **Redaction removes operators; it does not draw rectangles.** The famous failure is painting black over text and shipping it — the text is still there, and `pdftotext` prints it. This module draws nothing. It removes the text-showing operators whose position falls inside a rectangle, and the test that matters is that the text can no longer be extracted. Positioning needs the text matrix, so the module tracks `Tm`/`Td`/`TD`/`T*` and the CTM through `q`/`Q`/`cm`. It cannot reach the graphics layer — the crate boundary again — so it treats a showing operator's origin as its position and removes the whole run. That is coarse in the *safe* direction: removing more than asked loses content the user can see is missing; removing less leaves the secret in the file. What it refuses to claim is as important. Images are removed entirely rather than cropped. Metadata and attachments are untouched. And an incremental redaction leaves the original text in the earlier revision — the report says so via `earlier_revisions_retain_content` rather than implying the job is done. **Compaction finishes it.** The output is built from the object graph reachable from `/Root`, so dead objects, superseded revisions and the bytes behind a redaction are not copied — they are simply never written. A signed document is refused unless `allow_signed` is set, because compaction destroys the revision a signature covers and would leave every signature unverifiable with no warning. The end-to-end test is the point: redact, compact, then search the output bytes for the secret. It is gone. **Three reader defects, all found by writing the tests.** - **`PdfWriter` wrote dictionary keys unordered.** `PdfDict` is a HashMap and Rust seeds its hasher per process, so *every generated PDF differed run to run*. Found by compaction's idempotence test — compacting an already-compact file produced the same objects at the same offsets with their keys shuffled. Verified fixed by running four separate processes and getting a byte-identical file. Same defect as the one fixed in `content_edit::write_dict`; this one affected every file this codebase has ever written. - **A short `/Length` silently truncated a stream.** The reader guarded against a `/Length` running past the buffer but trusted one that was too small, cutting the stream at the wrong place and losing the rest with no error. Short lengths are common in hand-edited files. `endstream` is now the authority when the two disagree — but only when it is *further* on, so binary data containing the word `endstream` is still bounded by its declared length. - **Two stream readers disagreed by one byte.** `read_object_at` did not trim the EOL before `endstream` while `find_endstream` did, so a write-read-write cycle grew every stream by a newline. A test fixture had encoded the bug: it declared `/Length 9` for eight bytes of content and asserted the newline came back as data. Both corrected — the newline is syntax (§7.3.8.1), not content. Verified by mutation. Ten defects across the two modules, all caught: redaction covers instead of removes 16 fail CTM ignored 1 fail Q does not restore the CTM 1 fail operands kept when operator removed 12 fail revision warning always false 1 fail signature guard removed 1 fail reachability keeps everything 2 fail dropped reference left dangling 1 fail unresolvable object kept as reachable 1 fail writer dictionary order unsorted 1 fail short-/Length fix reverted 1 fail /Length not rewritten on compaction 1 fail One mutation survived and deleted code rather than adding a test: a `continue` skipping `/Length` in the compaction loop was dead, because the `set` after the loop overwrites it either way. Removed rather than left as untested defence with a reassuring comment — the same call ADR 0017 made about the visited-set guard. A second mutation moved a test rather than a fixture: a stale `/Length` can no longer reach `renumber` through a file, because the reader now repairs it first, so that branch is tested directly instead. Engine suite 1108 -> 1158. External readers still pass. Phase 5 remaining: outline, page label and struct-tree editing.	2026-08-17 12:20:25 +00:00
andodeki	01d889c90b	docs(cad): what the first widget extraction actually bought ... 13.15% -> 13.25%, and viewport_input.rs is still at 0.00%. Worth writing down rather than quietly celebrating a fix. Extraction makes logic testable by moving it out of the handler, so the handler shrinks instead of getting covered: nav_pad.rs took 64 lines to 100%, and viewport_input.rs went from 736 lines uncovered to 672 lines uncovered. At ~60 lines an extraction the remaining input handler is ten more of these. The pixel bug found on the first one suggests the yield is real, so it is a reasonable way to spend effort — but anyone expecting the widget number to climb fast should know it will not, and that actually covering the handlers means driving them with an event loop through makepad-test, which tests/cad_ui.rs and spreadsheet-ui already use.	2026-08-17 12:20:21 +00:00
andodeki	b7eb271a0d	feat(spreadsheet): logical and conditional formula functions ... The formula engine could sum, average and do single-cell math, but had no way to combine conditions or aggregate selectively — no `=IF(AND(...))`, no `=SUMIF(...)`. This closes that gap with seven Excel-compatible functions. Logical (evaluated lazily, like IF, so they short-circuit): - `AND(a, b, ...)` — TRUE iff every argument coerces to TRUE; ranges flatten to their cells. Stops at the first FALSE, so `AND(FALSE, 1/0)` is FALSE rather than `#DIV/0!`. - `OR(a, b, ...)` — TRUE iff any argument is TRUE, short-circuiting on the first TRUE. - `NOT(x)` — logical negation of the single argument. - `IFERROR(value, fallback)` — `value` unless it errors, in which case the fallback is evaluated and returned (lazily). Conditional aggregates (criteria-matched by position): - `COUNTIF(range, criteria)` — count of matching cells. - `SUMIF(range, criteria[, sum_range])` — sum of `sum_range` (or `range`) cells whose position matches; text in the summed region is skipped, errors propagate. - `AVERAGEIF(range, criteria[, average_range])` — mean of the matched cells, `#DIV/0!` when nothing matches. Criteria accept numbers, comparison operators (`>5`, `>=5`, `<5`, `<=5`, `<>5`, `=5`), case-insensitive text (`"apple"`, `=apple`, `<>apple`), and cell references holding any of those. Wildcards are not supported. Dependencies flow through the existing AST walk, so a SUMIF's range and criteria cell are tracked by the dependency graph and the formula recalculates when an input is edited — pinned by an end-to-end test through SpreadsheetData. Engine unit tests 331 -> 343.	2026-08-17 12:19:57 +00:00
andodeki	e1d1346b27	fix(mpesa): parse SMS from in-memory messages, not serde-skipped cache ... OfflineSmsMessage.body has #[serde(skip)] so it round-trips as empty. scan_sms was upserting to the offline store then loading back and parsing from the empty body, which always fails. Parse directly from the in-memory list_messages() result instead.	2026-08-17 15:13:33 +03:00