624 commits

Author	SHA1	Message	Date
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	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
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	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	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
andodeki	70bbff7ecb	fix(cad): the nav pad's zoom buttons were a pixel from their own hit zone ... First test written against the widget layer, and it found something on the way in. The viewport's navigation pad — fit, four pans, zoom in, zoom out — had its layout written twice. `viewport_render.rs` drew each button at `col * (BTN_W + GAP)`. `viewport_input.rs` decided what a click hit with hand-written arithmetic inline in a 630-line event handler: `BTN_W * 3.5 + GAP * 3`. Those agree for whole-numbered columns and disagree at the half column the zoom pair sits in: 3.5 * 24 = 84 drawn, 22 * 3.5 + 2 * 3 = 83 hit-tested. The zoom buttons' clickable area sat one pixel left of the buttons, so their right-hand pixel column did nothing and a pixel of empty space beside them zoomed. `PanRight` was 2px short at its right edge for the same reason. One pixel is not much on its own. The mechanism is what matters, and it is the third instance of it in this module: two copies of one piece of geometry, free to drift, with nothing able to notice. The camera-to-world pair was the first, the scene-cache benchmarks the second. Both callers now read `nav_pad::LAYOUT`. The renderer iterates it; the hit test tests against it; the offsets come from one function. That also turns 88 lines of inline conditionals in the event handler into 32 lines of match, which is a readability win I would not have bothered with on its own. The bounds are now the drawn rectangle exactly — half-open, BTN_W by BTN_H, gaps dead. The old hit zones were 2px larger than the buttons in several places. Being strict is deliberate: a hit area larger than its button is indistinguishable from a misaligned one the next time something looks wrong. 7 tests, 100% of the new module. The one that matters is `drawn_and_hit_zones_agree`: every drawn rectangle must hit-test to its own button at all four corners and the centre. That test fails on the old code, which is the only reason to trust it. Verified with a real compiler, which this environment turns out to have: 1051 lib tests pass (7 new), cad_integration 154 pass, cargo fmt clean, engine coverage 97.16% with every floor met including nav_pad at 100%.	2026-08-17 12:12:34 +00:00
andodeki	074066a382	feat(email): export the trip report from the inbox ... spawn_export_trip_report fetches the inbox (shared fetch_inbox_messages helper), extracts trip receipts, builds the report, and writes trip-expense-report.pdf into app-data; it posts TripReportExported with the path and a summary. The More page gains a Finance card with an 'Export trip report (PDF)' button and a status line.	2026-08-17 12:08:19 +00:00
andodeki	e0d86274d8	feat(pdf): flatten annotations and form fields into page content ... Phase 5's `flatten_test.dart`. An annotation draws from its /AP /N stream, which lives beside the page rather than in it; flattening moves that appearance into the page's own content and removes the annotation, so what is drawn is part of the page and cannot be turned off, edited or extracted as a field. That is what "finalise this form" and "make these comments permanent" mean, and it is irreversible by design. The placement transform is the whole problem, and it is §12.5.5: transform the /BBox by /Matrix, take the bounding box of the *result*, then fit that onto /Rect. Skip a step and the stamp lands at the origin, or in the right place at the wrong size, and the page still renders. A rotated appearance is the case that exposes it — rotation swaps the transformed box's width and height, so fitting the untransformed box squashes it. What is refused matters as much as what is done: - **No appearance stream**: left in place and reported. Dropping it loses it; inventing an appearance draws something the producer never specified. - **Hidden or /NoView**: not drawn on screen, so burning it in would *add* ink the user never saw. - **A /Popup**: the pop-up window of another annotation, never drawn on the page itself. Appearances are painted as XObjects rather than having their operators spliced in. Splicing needs the stream's resources merged into the page's with every name collision renamed, and it loses the /BBox clip an XObject applies for free. `pdf-document` cannot depend on `pdf-graphics` — the crate boundary is cos -> document -> graphics and inverting it to reuse `write_ops` would be a far worse trade than emitting the four operators (`q`, `cm`, `Do`, `Q`) directly. The number formatting follows the same shortest-exact rule as `content_edit::write_real`, and for the same reason. Verified by mutation, five defects, each confirmed red: placement matrix ignored 1 fail /BBox not transformed first 2 fail hidden check removed 1 fail annotation kept after flattening 7 fail existing page content dropped 1 fail **Externally verified, and it found a real gap.** Flattening the sample's seven annotations passed `qpdf --check` and kept every text run — but poppler still reported `Form: AcroForm` on a document with no fields left, because the catalogue entry survived. A viewer may still offer to fill in a form that no longer exists. `remove_acroform_if_empty` drops it, but only when *no* widget survives anywhere: flattening one page of a three-page form must not strip the fields still live on the others. before: Form: AcroForm after: Form: none `flatten_document` is the whole-document entry point — every page, then the form entry — and re-parses between pages because each flatten appends a revision the next must read. 22 round-trip tests through the saved file, including that flattening twice is idempotent (a Do count that grows on every save is how a "flatten" button pressed twice doubles every stamp), that existing page resources survive, and that a /AP /N state dictionary resolves through /AS. Engine suite 1075 -> 1108. Remaining in Phase 5: object compaction, redaction, and outline, page label and struct-tree editing.	2026-08-17 12:03:59 +00:00
andodeki	8ca5070b26	test(spreadsheet): cover the remaining serialization and border branches ... The last reachable lines in data.rs, all in the persistence layer and one style command: - serialize/deserialize round-trip for named ranges (the NAME block line), text colour, and an unknown block tag (ignored for forward compatibility), plus a malformed NAME line inserting nothing. - The legacy CSV deserializer's `|B` bold and `|F` formula metadata segments. - apply_command with BorderTarget::None clearing all four edges (the "remove borders" action), the complement of the per-edge arm the existing test covers. - A large-range self-reference reporting CycleDetected (pins the fix in the previous commit). data.rs 97.91% -> 99.49%; engine total 98.50% -> 99.13%. Unit tests 325 -> 331. Deliberately uncovered, as documented: the B17 invariant's debug_assert/#ERROR! patch (only reachable on a dep-graph bug), the topological-sort underflow guard, the let-else continue after a cell disappears mid-iteration, and the `}` attribution regions after unconditional returns.	2026-08-17 11:49:00 +00:00
andodeki	e1dcbefda3	fix(spreadsheet): large-range cycle detection must report, not zero ... The large-range fast path in DataEvalContext::get_range_values (used for ranges over 64 cells) handled a re-entrant formula cell — a reference cycle — by silently pushing 0.0. The small-range path in get_cell_value reports FormulaError::CycleDetected for the same situation. So a cyclic formula inside a large range contributed zero to the sum instead of surfacing #CYCLE!. The inconsistency is invisible in normal recalculation: the topological sort in recalculate_incremental detects cycles before any evaluation, so the branch only fires through the legacy recursive get_cell_value API. The fix makes the two range paths agree.	2026-08-17 11:49:00 +00:00
andodeki	2ba06f34c5	fix(makepad-table): stop overlong cell text spilling into the next column ... A right-aligned cell whose text is wider than its column drew over the left-aligned text in the column beside it. Two separate causes, one on each side of the cell. **Spilling left.** `align_text_x` computed `pos.x + width - pad.right - text_width` for right alignment and `pos.x + (width - text_width) * 0.5` for centre. Once `text_width` exceeds the column width both go *negative relative to the cell*, so the run started outside its own cell and reached backwards over the previous column. That is the reported collision: the right-aligned column's text sitting on top of its left-aligned neighbour. The result is now clamped to the padded left edge, so an overlong string starts where a left-aligned one would and runs forwards. **Spilling right.** Clamping alone only fixes the near side — the run would still continue past the cell's right edge into the following column, which is the same overlap seen from the other direction. `fit_text_to_cell` shortens anything wider than the padded width and appends an ellipsis. `DrawText` can do this itself via `text_overflow: Ellipsis`, but only through `draw_walk`, which needs a turtle; these cells are drawn absolutely with `draw_abs`. The truncation therefore uses the same 7px-per-character estimate the positioning already used, so one approximation is applied consistently rather than two that can disagree. When a real text measurer replaces it, both callers change together. Order matters in `draw_cells`: the text is truncated first and the *truncated* string is aligned. Aligning the original and drawing a shorter one positions the run by a width it no longer has, which puts the overlap back. Tests 51 -> 59. Verified by reintroducing four defects: removing the clamp fails the left-spill test, removing truncation fails three, aligning the full text fails the ordering test, and measuring bytes fails the multi-byte test. Two of those needed the tests fixed first, and both were mine: - The draw-path ordering had no coverage at all, because `draw_cells` needs a live `Cx`. It now reads the source for the order of the two calls. Blunt, but an accidental reordering is exactly the regression this invites. - `truncation_counts_characters_not_bytes` passed with `len()` substituted for `chars().count()`. The byte count only gates *whether* to truncate, and the case I wrote was one that should be truncated either way. The test now uses a multi-byte string that comfortably fits — which `len()` would wrongly shorten — so the assertion turns on the difference rather than merely being near it.	2026-08-17 11:38:55 +00:00
andodeki	306504bc0b	fix(makepad-table): pin the pressed-state fill, and put the caret after the text ... Two bugs in the table demo's cell editor, both reported after the previous colour pass. **White on click.** `TextInput`'s `draw_bg` blends through six fill states, and the previous change pinned five of them. It missed `color_down`. The widget's animator holds `down: 1.0` for as long as the pointer is pressed on it, so clicking into a cell blended toward the theme's light fill and stayed there until the pointer moved off and the state decayed — which is exactly the reported "goes white when I click, correct once I move the mouse away". It read like a hover state and was the press state. `color_down` is now pinned, along with the six `color_2*` gradient partners. The shader only mixes those when `color_2.x > -0.5` and the default is a -1.0 sentinel, so they were inert — but the theme sets them, and anything that later turned the gradient on would have pulled theme colours back in. **Caret at the start.** `begin_edit` called `set_text` and nothing else. `set_text` loads the value and leaves the cursor at index 0, so typing into a cell that already had content inserted at the front. `move_cursor_text_end` after the load puts it where every spreadsheet puts it, and where `commit_edit` reading the whole buffer back already assumed the user was working. Tests 49 -> 51, and the existing state test was rewritten. It had been checking a hand-written subset of states and passed the whole time `color_down` was missing — a test that only covers the states someone remembered is a test that misses the one they forgot. The fill states are now their own exhaustive check. Verified by reintroducing each defect: removing `color_down` fails the fill test, removing the caret call fails the caret test, and swapping the caret call before `set_text` fails it too. That last case needed the test fixed first. The ordering guard compared `body.find("set_text")` against `body.find("move_cursor_text_end")`, and `find` returns the first match anywhere — including inside the doc comment above the code, which mentions `set_text`. Swapping the two statements left the comment in place, so the naive check still passed. It now compares the first non-comment line containing each call.	2026-08-17 11:33:20 +00:00
andodeki	691b868264	fix(spreadsheet): make an editing cell readable — no doubled border, cell's own ink and fill ... The same three symptoms were reported twice. The first fix went to `makepad_table`, which is not in the APK — `pageflipnav` does not depend on it. The editor actually being clicked is `SpreadsheetGrid::draw_edit_overlay`, reached through `nigig-build -> spreadsheet-ui`, and it is a custom-drawn overlay rather than a `TextInput`. This fixes that one. 1. **No border of its own.** `draw_walk` calls `draw_selection_overlay` immediately before `draw_edit_overlay`, and the selection rectangle is already stroked around this exact cell in `selected_border_color`. The overlay then drew four more 2px rects inside it, which is the doubled frame: an outer selection edge and an inner editor edge a pixel apart. The four strokes are gone. Checked before removing them that this cannot leave a cell unframed. All three paths into `request_begin_edit` are on an already-selected cell: the keystroke path uses `self.selected_cell()` by definition, and both double-tap paths record `InputIntent::Select` for the same cell on the first tap. The caret remains the signal that the cell is in edit mode. 2. **The cell's own ink, not a forced one.** The overlay did `self.draw_text.color = self.text_color;` with no branching, so a cell with a user text colour, a bold cell, or a formula all changed colour the moment the caret landed in them. It now resolves the same way the resting draw path does: per-cell colour, then formula green, then bold, then the default. 3. **The surface it rests on.** The fill was always `edit_bg_color`, which equals `cell_bg_color` — so an odd row, which rests on `cell_alt_bg_color`, visibly changed shade when editing began, and a cell the user had given a background lost it entirely. That last case is the "background goes a different colour and the text disappears" report: the fill came from `edit_bg_color` while the ink came from the cell's own style, and nothing kept the two in agreement. It also covers the selection fill. A cell being edited is by definition selected, so `draw_cell_bg` had already painted `selected_bg_color` (#x2d4a63, a blue-grey) underneath — text was sitting on a wash it was never coloured for. That is the grey. `grid.rs` had no tests and is excluded from the coverage report, because the widget needs a live `Cx`. The background choice does not, so it moved into `editing_bg_source` and is tested there; the border and ink changes are pinned by reading this file, which is blunt but is the only thing short of a GPU that can catch "compiles, runs, wrong on screen". 6 tests, verified against all three defects: restoring the border stroke fails 1, forcing `text_color` again fails 1, and ignoring row striping and cell backgrounds fails 3. The 12 clippy warnings in this crate tree are pre-existing and unchanged — checked by running the same command on a stashed tree. None are in the hunks here.	2026-08-17 11:07:39 +00:00
andodeki	60117099ae	fix(makepad-table): make an editing cell readable — white fill, dark ink, one border ... Three reported problems, one cause. `TextInput` does not have a colour, it has a set of them, and `get_color` blends between them: `color_focus` on focus, `color_empty*` when the field is empty, `color_hover`, `color_down`, plus `border_color_2*` gradient partners for every border state. The cell editor set four properties — `color`, `color_focus`, `border_color`, `border_color_focus` — and left the rest to fall back to the active theme. The active theme is the dark one: `theme_desktop_dark::script_mod` runs after the light one in `widgets/src/lib.rs` and wins. So the moment a cell took focus the editor drew `theme.color_text` (near white) on `theme.color_inset_hover` (dark grey), and an emptied cell hit the `color_empty` path, which was also unset. Fixed by pinning every state rather than the ones that happened to be visible in one configuration: 1. **No border of its own.** `border_size: 0.0` and every border state transparent, including the six `border_color_2*` partners, which render as a faint grey edge even at zero width on some backends. The 2px green frame is `draw_select`, drawn over the same rect in `draw_walk` — the editor's border sat inside it and read as a doubled frame. The green selection is unchanged. 2. **Editing ink equals resting ink.** `#x1f2937` in all eight text states, the same colour a non-editing cell uses. An editing cell should look like a resting cell with a caret in it; the caret is the affordance and a colour change only costs contrast. `draw_cursor` is pinned too — `theme.color_text_cursor` is chosen for a dark inset and nearly vanishes on white, which would have left no cue at all. 3. **Plain white fill**, in all five fill states rather than just two. This is also the "background goes white and the text disappears" case: that was the fill switching to the pinned white while the text switched to the unpinned theme colour, so the two moved independently. `draw_selection` is pinned to a pale green that matches the frame, instead of the theme's blue, so selected text stays dark-on-light. The same latent bug was in all six of the invoicer's inputs — the five header fields and the search box — which set the same four properties. They are pinned the same way. Their placeholders stay grey deliberately: a placeholder is a prompt, not content. Tests 44 -> 49. The colours live in `script_mod!`, which is data this crate does not parse, and no test can ask a widget what it drew without a GPU — so these read the DSL source and assert the states are present. That is blunt, and worth having anyway, because the failure mode is precisely "compiles, runs, looks wrong only on screen". Verified by restoring the original four-property block: all five fail.	2026-08-17 10:49:44 +00:00
andodeki	e0aab74452	feat(pdf): page operations — insert, reorder, duplicate, delete, import ... Phase 5's page management, matching dart-pdf's `page_ops_test.dart`, `page_index_map_test.dart` and `import_source_test.dart`. A reader sees "page 3"; the file holds a tree of /Pages nodes with /Kids and /Count and /Parent back-pointers, any of which can be left stale. So the writer **flattens to a single level**: a one-level /Pages node with every page as a direct kid is valid, is what most producers emit, and removes the entire class of bug where an intermediate node's /Count no longer matches what is under it. Preserving an arbitrary tree shape through arbitrary reordering is far more code for nothing a reader can see. `PagePlan` accumulates operations and applies them together, so intermediate states never have to be valid — delete page 0 and insert a new one at 0 without the document momentarily having no first page. `PageIndexMap` reports where every page went, which is the only way to fix an outline entry, named destination or link annotation afterwards. What each operation carries matters and differs: - Reorder and delete rewrite only the kid array, so page objects and their resources are untouched. - Duplicate writes a new page dictionary that **shares** the original's resource references. Two pages naming one font object is normal; deep-copying would double the file and change nothing visible. - Import must deep-copy the page and everything it reaches, renumbered, because source object numbers mean nothing in the destination. /Parent is deliberately not followed — it leads back to the source's page tree and from there to every other page in that file. Inheritable attributes are resolved *before* a page is imported. /Resources, /MediaBox, /CropBox and /Rotate may live on an ancestor (Table 30) that is not coming with it, so a page imported without them renders at the wrong size with no fonts, and nothing reports an error. **Round-trip tested through the saved file**, which is Phase 5's exit criterion: 20 tests that save, re-parse, and assert on what a reader actually gets. Pages are identified by /MediaBox width rather than object number, because object numbers are exactly what a page-tree bug scrambles. Mutation testing changed two things. Seven defects injected: /Count left stale 1 fail /Count omitted entirely 1 fail imported /Parent not rewritten 1 fail inherited attributes not resolved 1 fail import does not deep-copy 3 fail duplicate loses /Contents 1 fail re-parenting skipped 1 fail The last two only fail because of tests the mutations forced: - **A stale /Count passed everything.** Our own parser walks /Kids and never reads /Count, so it cannot see the disagreement — but other readers trust /Count, and a document where the two differ opens with a different page count in different viewers. The test now reads the raw page-tree node instead of asking the document. - **Re-parenting could be deleted with every test still green**, because the flat fixture's pages already parent to the root. Added a nested fixture with an intermediate /Pages node supplying an inherited /MediaBox — the case where leaving /Parent stale means a page keeps inheriting from a node it is no longer under. Externally verified: a generated sample with pages swapped and duplicated passes `qpdf --check` with no warnings, and poppler reads 4 pages with the reordering visible in extracted text. Engine suite 1039 -> 1075. Remaining in Phase 5: flatten, object compaction, redaction, and outline and struct-tree editing.	2026-08-17 10:38:55 +00:00
andodeki	b8bd71852f	feat(pdf): content-stream serialiser and editor — the Phase 5 foundation ... Phase 5 needs to write operators back, and `content.rs` has only ever parsed them. Every editing feature the phase asks for — insert, delete, replace, rewrite a text run, flatten an annotation — rests on that, and a serialiser that is subtly wrong does not throw: it writes a valid content stream that draws something else. So this is the serialiser plus the gate, and nothing built on top yet. The contract is a property, run over the whole corpus: parse(write(parse(bytes))) == parse(bytes) Operators, not bytes. Byte equality would be the wrong test — `1.0` may legally be written `1`, whitespace is free, and a writer that reproduced its input byte for byte would only prove it had copied it. It passes: **16,466 operators across 154 streams in 109 files**, plus stability, idempotence, and the same property after an edit. **Then mutation testing showed the corpus gate was not enough.** Six injected defects, and *five passed*: dropping name escaping, unescaping string parens, un-sorting dictionary keys, discarding unknown operators, and a fixed six-decimal number format. Real files are written by well-behaved producers, so 16,000 corpus operators contain no name with a space, no nested parenthesis, no seven-key inline dictionary and no vendor operator. A gate that only sees well-formed input cannot catch a writer that mishandles the rest. The adversarial set fixes that — eighteen streams, each a legal shape the corpus lacks, each chosen because a specific defect survives without it. Writing it found **three live bugs in the parser**, none of which the round trip could see on its own: - **Nested parentheses truncated a string to nothing.** `((nested))` parsed as the empty string, and worse, left the reader mid-string so every operator after it was parsed from the wrong offset. §7.3.4.2 says balanced parens nest and need no escaping. - **`#` escapes in names were never decoded.** `/My#20Font` — how every producer writes a font whose name contains a space — parsed as the literal `My#20Font` and never matched the page's resource. - **`PdfOp::Unknown` was declared and never constructed.** An operator the parser did not recognise vanished. Survivable for a renderer, fatal for an editor: parse, change one operator, write back, and every vendor extension in the page is silently gone from the saved file. And two in my own serialiser, both found the same way: - A fixed `{:.6}` flushed 1e-7 to zero — a scale factor silently becoming zero collapses whatever it transforms — and rounded `1.234567891` to a different number. Precision is now the shortest that parses back to the identical f64, exact by construction rather than by choosing a number. - Sorted dictionary keys turned out to be load-bearing. `PdfDict` is a HashMap and Rust seeds its hasher per process, so an unsorted writer is stable within a run and different on every new one: rebuild the same document twice, get two different files. Neither the round trip nor a within-process stability check can see it — both sides are equally unordered. Verified by running five separate processes and getting five different key orders. Two of those needed tests the round trip structurally cannot provide, so they assert on the parser directly: what `((nested))` must produce, and that operators after it are still read at the right offset. Final mutation run, eight defects, all caught: fixed 6-decimal precision 1 fail name escaping dropped (writer) 1 fail name unescaping dropped (parser) 2 fail nested-paren fix reverted 1 fail unknown operators discarded 1 fail string parens unescaped 1 fail close-paren unescaped 1 fail dictionary keys unsorted 2 fail `ContentEditor` sits on top: insert, append, prepend, delete, replace, isolate, and text-run rewriting that preserves the operator *kind* — a `'` stays a `'` and keeps its line advance, a `TJ` keeps its kerning numbers while its strings change. Every mutation is balance-checked, so an edit that would leave `q` without `Q`, or `BT` without `ET`, is refused at the edit rather than discovered at save time. `PdfOp` gained `PartialEq`, which is what makes the property expressible at all. Engine suite 1025 -> 1039. Phase 5's remaining items — page ops, import/merge, flatten, compaction, redaction — build on this and are not started.	2026-08-17 10:29:39 +00:00
arena-agent	b478945c34	ci(doc): gate the doc-workspace coverage floor on every push ... Adds the doc-workspace-coverage job to the nigig-build workflow, mirroring the CAD gate: checkout, then ./tools/test-doc-workspace-coverage.sh, which installs its own instrumented toolchain into a shell-trap-cleaned temp dir and fails if the total floor (92%) or any per-file floor is not met. The script joins the workflow's push/PR path filters next to tools/test-cad-coverage.sh so edits to the harness itself re-run the gate. The doc README gains the milestone section recording the 28.55% -> 96.76% line measurement, the honest exclusions (widget layer, persistence write-path wrappers, defensive traversal guards) and the behavior pins and defect fixes the drive surfaced.	2026-08-17 10:25:27 +00:00
arena-agent	949cf24189	test(doc): doc-workspace coverage harness with enforced floors ... tools/test-doc-workspace-coverage.sh measures line+region coverage of the doc module's pure layer the same way the CAD gate does: it copies the dependency-free sources (advanced_json, crdt_bridge, mobile_gesture, persistence, projection_layout, projection_session, and the collaboration/, editing/, layout/, model/, plugins/ trees) plus tests_pure.rs into a temporary host-only crate with the real module path, satisfies the five makepad-math symbols the pure layer uses through a 30-line makepad-widgets shim, runs the suite under -C instrument-coverage with a toolchain it installs itself, and enforces a total floor plus a per-file floor for every instrumented file. The per-file floors are the point: a lone total waves through the silent loss of one whole file's tests. Measurement moved from a 28.55% line baseline to 96.76% (6170 lines) with the tranche in the parent commit; floors sit a few points under per file, except persistence.rs (55%), whose three write-path entry points write into the host's real application-data directory and are covered through their path-injected seams instead -- the honest exclusions, the exact table, and the two defect fixes this drive surfaced (ReplaceBlockRange validation order, dead RgaText::visit_children) are written down in the module's new COVERAGE.md. Everything the script touches -- pinned toolchain, cargo home, target dir, fetched Makepad tree, profraw data -- lives under one mktemp dir removed by a shell trap on every exit path; nothing lands in the repo or $HOME unless KEEP_COVERAGE=1 is set for a debugging run. DOC_WS_COVERAGE_REPORT_ONLY=1 measures without gating.	2026-08-17 10:25:27 +00:00
arena-agent	71c31cd19f	test(doc): host-only suite split + pure-layer coverage tranche ... Split the doc module's tests in two so the dependency-free majority can also run under coverage instrumentation on a host-only crate: * tests_pure.rs (new) holds every test that needs no Cx -- model, layout, editing, collaboration, advanced JSON, projection layout/session, CRDT bridge, persistence seams, mobile gestures -- and is the file tools/test-doc-workspace-coverage.sh copies byte-for-byte into its harness. * tests.rs keeps the widget-runtime and boot tests; shared helpers (projection_table_engine, only_table) live in tests_pure so both suites use them. On top of the split, this tranche adds ~80 tests covering the pure layer's real gaps: every Command apply arm and its inverse (text, atoms, blocks by stable id, image properties, block ranges, table cells/rows/columns, merges/splits/restores, node insert/delete/ replace), the DocumentController's CRDT typing lifecycle (insert/replace/delete ranges, backspace/forward delete, position sync), remote operation classification (Applied/Duplicate/Deferred/Rejected), tombstone compaction at a peer-acknowledged frontier, a two-peer MemoryTransport conversation, typing coalescing and history limits, the cell-text editing and multi-line geometry helpers in projection_layout, table cell cursor/range/merge queries, and the small model/session/selection behaviour surface. Two defects found while writing the tests, fixed with pins: * Command::ReplaceBlockRange validated a caller-supplied block_ids length AFTER draining blocks and legacy ids out of the document, so a malformed (remote) command destroyed content before reporting failure. Validation now runs before any mutation; a test proves a rejected replace leaves blocks, ids and order untouched. * model::crdt::RgaText::visit_children was dead code -- a String-collecting duplicate of visit_atoms with no callers. Removed. Two semantics that were folklore are now pinned with inline reasoning: a multi-peer conversation only converges when each peer owns a distinct document.crdt.local_actor (the two-controller sync test assigns alice/bob), and a mid-range replace_range_crdt renders its replacement after the tombstoned subtree it replaced, because RGA sibling order walks by atom id ("hello" -> "hloY" is asserted, not assumed). cargo test -p nigig-build --lib: 1031 passed, 0 failed.	2026-08-17 10:25:27 +00:00
andodeki	ac145bfaab	test(cad): measure tools.rs, which a false comment had ruled out — 0% to 97.07% ... tools.rs opens with: Struct definitions stay in mod.rs (they use #[derive] macros that need the script_mod! context). Only the impl blocks are here. That is not true, and it is the reason 250 lines of pure tool-state logic — tool cycling, work-plane mapping, the inclined-UCS maths — had never been measured or tested. `CadTool`, `WorkPlane`, `AxisLock`, `DrawingState`, `SnapSettings` and `InclinedPlane` each derive some combination of Clone/Copy/Debug/PartialEq/Eq and nothing else, and none is inside the `script_mod!` block. I found it by asking the compiler instead of grepping. Adding each of the eleven unmeasured CAD files to the harness one at a time and reading the errors gives the real dependency, not a guess: tools.rs needed six plain types; viewport_2d.rs needs `CadViewport` and `Cx`; code_editor.rs needs `Cx2d` and `DrawStep`. Only the first of those is a documentation problem rather than a real one. My previous two triage passes used a grep heuristic and were wrong twice, including about this file. The harness now mirrors those six declarations, extracted from mod.rs at run time so they cannot drift, exactly as it already did for ViewMode and SelectionMode. **No production code moved.** Moving the declarations for real is a smaller job than the comment implies but not a free one: DrawingState, SnapSettings and InclinedPlane have private fields that mod.rs and viewport.rs read directly, so their fields need widening first. CadTool and WorkPlane are fieldless and could move today. That is now written in the file for whoever has a compiler for the widget layer. 13 tests, on the properties that break quietly: - Cycling forward visits all 17 tools exactly once and closes the ring. `cycle_next` is a hand-written 17-arm match; a duplicated or skipped arm makes a tool unreachable from the keyboard and nothing else would notice. - Backwards is asserted to be the exact inverse, per tool. Shift-Tab that does not undo Tab reads as "the tool picker jumps". - Labels must be unique — two buttons reading the same is a UI bug with no test otherwise — and every tool needs a description. - `to_kind` maps only the drawing tools; Select, Delete and Measure must return None or they would create geometry on click. - `InclinedPlane::from_3_points` produces a unit normal perpendicular to both edges, and rejects collinear or coincident picks rather than returning a NaN basis from a zero-length cross product. - The plane basis is orthonormal in both branches, including the vertical-normal case that exists because the usual "up" reference is parallel to the normal there. Also fixes a real bug in this script's own drift detector: it tested membership with `case " ${ENGINE_FILES[*]} "`, and `[*]` joins on the first character of IFS, which this script sets to a newline. The pattern could never match, so the note fired for every non-widget file. It was right about tools.rs by accident. Floor: tools.rs 95. Total unchanged at 97.14% over a larger denominator. Verified: 568 tests green, every floor met, hermetic run clean.	2026-08-17 10:16:52 +00:00
andodeki	b83e7122c4	feat(makepad-table): file picker, search, recents, New/Delete (Invoicer UI Phase 3) ... The last open phase. The sidebar gains a search box, a filtered document list and a recents list; the toolbar gains New Invoice/Quote/Receipt, Open, Save As and Delete. **The file picker is robius, not makepad's.** Makepad has an `open_system_openfile_dialog`, and it is implemented on macOS only — the Linux and Android backends never handle `CxOsOp::SelectFileDialog`, so the op is queued and dropped. It compiles, it runs, the dialog never appears. That is the worst kind of broken, so this uses `robius-file-picker`, the same crate `nigig-build`, `nigig-pay-ui` and `nigig-sms` already depend on at the same pinned revision, which goes through `rfd` on desktop and the platform picker on Android. CI gates against the macOS-only call returning. The picker's callback runs off the UI thread with no `Cx`, so it parks its outcome in a mutex and signals; `drain_file_picker` applies it on the next `Event::Signal`. Same shape as the SMS bulk CSV import. Model additions, in `makepad-doc-model` so they are testable without a window: `DocKind` with `blank()` constructors, `DocumentLibrary::create`, `remove`, and `selection_after_remove`. Decisions worth naming, because each has a wrong answer that looks fine: - **A new document is empty**, not seeded from the samples. A blank invoice arriving with "Acme Studio LLC" on it invites someone to export it without noticing whose name is there. `issue_date` is blank too — there is no clock in that crate and a guessed date is worse than none. - **Generated numbers cannot collide**, including with documents loaded from disk, and they reuse gaps left by deletions. The number becomes the filename: two documents called INV-1 save over each other and one is lost silently. - **Delete removes the row, not the file.** Removing an entry from a list is not consent to delete a document off disk, and there is no undo here. The status line says the file is untouched. - **Save reports "Choose where to save…", not "Saved."** The dialog being open is not the file being written. - **Search filters on every keystroke**, unlike the header fields, which commit on Return. Every prefix of a query is a valid narrower search; there is no such thing as a half-typed one. - **Searching does not move the selection.** Filtering is a view change, and switching the open document because a letter was typed loses the user's place. - **`selection_after_remove` is separate and exhaustively tested.** Deleting before the selection shifts it, deleting the selection keeps the index unless it was last, deleting after it changes nothing, and emptying the library selects nothing. Every wrong answer silently shows a different document; one of them indexes out of range. The document list is a fixed pool of 12 button slots rather than a `PortalList`, because this app opens documents one at a time. The pool is honest about its limit: anything past it renders as "+n more — narrow the search to reach them" rather than being dropped. Tests 79 -> 90. Six of them are the invoicer's first: `App` derives `Script` and cannot be built outside a live `Cx`, so the sidebar's presentation logic was extracted into four pure functions and tested there. Verified by reintroducing six defects across the two crates — silent overflow, a selection marker that shifts the indent, whitespace counting as a search, colliding numbers, a selection that ignores the shift, and a `blank()` that pre-fills. Also fixed, all pre-existing and all now blocking the `-D warnings` gate that has been running on these crates since the workflow was added: `std::io::Error::new(ErrorKind::Other, _)` in two crates, a manual `RangeInclusive::contains`, a manual `is_multiple_of`, a single-arm `match`, and a duplicated `#[test]` attribute that was annotating one function twice — which is why the count reads 36 rather than 37 here; no test was lost. The sample data keeps its `12_000_00` money literals, where the last group is the minor units and the number reads as "12,000.00" at a glance. `inconsistent_digit_grouping` is allowed at the crate root with that reasoning, rather than regrouping every amount into thousands and making each one need arithmetic to check against its comment.	2026-08-17 10:01:43 +00:00
andodeki	1220f89fc6	feat(pdf): close the last three Phase 4 items — reconciliation, CFF, cmap ... The three items the previous commit's audit found unimplemented while the status line said "complete". All three are done and externally verified. **1. Field-value reconciliation** (`reconcile.rs`). A field carries its value in /V and its rendered look in /AP, and nothing in the format keeps them in step. Files arrive with them disagreeing all the time: a producer writes /V and leaves appearances to the viewer, or something edits /V without touching /AP. Until now this crate simply believed /V and regenerated appearances only for fields it had itself edited — right for a field we changed, wrong for a field that arrived inconsistent. The module deliberately does **not** pick a winner. PDF 32000-1 §12.7.3.3 settles exactly one case — /NeedAppearances true means /V is authoritative — and is silent on the other, where a conforming viewer renders /AP and never looks at /V. So it classifies the disagreement and resolves it against a caller-declared `Intent`, because the right answer genuinely differs: a viewer must show /AP to match other viewers, an extractor must read /V, an editor must regenerate so the saved file agrees with itself. Silently choosing one would be ADR 0017's failure in a new place — every answer plausible, none checkable, the caller unaware a decision was made for it. Two cases are not judgement calls and are handled outright. A missing or dangling appearance renders *blank*, and blank is never what the producer meant, so even Display regenerates. An unselected radio member showing /Off while the group's /V names another member is correct, not a conflict — reporting it would flag every well-built radio group there is. **2. Type1/CFF embedding** (`embed_opentype_whole`). The spec says "if feasible". Subsetting CFF is not — it means rebuilding the CFF INDEX, charset and charstrings, a second font format inside the first — and `subset_truetype` rightly keeps refusing it by name. Embedding the program *whole* is feasible, and that is what this does: /FontFile3 with /Subtype /OpenType under a CIDFontType0 descendant, per Table 126. Each of those keys matters and none is guessable from the others. A CFF program in /FontFile2, or under a CIDFontType2 descendant, still produces a file qpdf accepts and a font that loads as the wrong type or not at all. /CIDToGIDMap is omitted because it is defined for CIDFontType2 only. The trade is made visible rather than buried: `EmbeddedFont::is_subsetted` is false here, so a caller with a size budget — or a licence that forbids shipping a whole face — can refuse instead of discovering it from the output size. **3. `repair-cmap`** (`glyph_index`). A symbol font declares no Unicode subtable: it maps glyphs into the private-use area at 0xF000 + the low byte under platform 3, encoding 0. Asking it for 'A' found nothing and the character silently vanished from the output — the font "missing" a glyph it plainly has. Now the (3,0) subtable is kept as a fallback and retried at 0xF000 + low byte, after the proper lookup fails so a font with both subtables is still read through the Unicode one. Format 0 is read too; omitting it left legacy and symbol fonts mapping nothing while appearing to have a usable cmap. The repair must not manufacture glyphs, which is its own test: a character the font genuinely lacks still returns None, because turning a missing character into a wrong one is worse. **Fixtures.** No CFF or symbol font ships on the CI image, and neither can be tested honestly against a hand-built stub — the point is that the bytes are a font program a third-party reader accepts. Both are generated from DejaVu by checked-in fontTools scripts: `cff_sample.otf` (1.6 KB, real OTTO/CFF outlines) and `symbol_sample.ttf` (664 B, a single (3,0) subtable so the repair path is the only route to its glyphs). Both generators pin `head.created`/`head.modified` to zero. fontTools stamps the current time, so the output differed on every run and CI's "fixtures match their generator" check failed against a file nobody had edited. Caught by running that check rather than assuming it passed. A fixture that cannot be regenerated byte-for-byte is not reviewable: you cannot tell a deliberate change from a rebuild. **Verified by mutation**, seven injected defects, each confirmed red: NeedAppearances ignored 1 fail dangling /AS not detected 1 fail blank rendering shown faithfully 1 fail CFF written to /FontFile2 1 fail CFF given a CIDFontType2 descendant 1 fail whole font claims to be subset 1 fail cmap 0xF000 retry removed 3 fail **Verified externally.** The sample now carries a third page set in the whole-embedded CFF font, and `check-pdf-external-readers.sh` gained `pdffonts` — the only check that inspects a font *program* rather than the file structure, which is exactly where a wrong /FontFile key shows up. poppler reports both fonts embedded and distinguishes them correctly: ETXLDI+DejaVuSans CID TrueType Identity-H emb yes sub yes NigigTestCFF CID Type 0C (OT) Identity-H emb yes sub no and extracts "Hello CFF 123", which only works if the CFF program loaded, /Identity-H addressed its glyphs and /ToUnicode mapped them back. That check also caught its own page-count assertion going stale when the third page landed — a gate that notices its own fixture changing is working. Engine suite 953 -> 985. Coverage 87.27%, all floors met. Phase 4 is complete but for the ui.rs interaction tests, which are written and blocked on the Makepad fork's missing headless backend.	2026-08-17 09:31:41 +00:00
andodeki	587a43864f	test(spreadsheet-ui): cover the remaining controller branches ... The UI controller modules are the one part of the spreadsheet stack the coverage report could not reach before the memory-conscious build (-j1); measured now, their last reachable gaps are closed. geometry.rs 98.77% -> 99.51%: - row_resize_at: a point just inside a row's bottom edge resizes that row, and a point in the middle of a row is no resize target. The exact boundary between two rows resolves to the lower one, so the bottom-edge arm had no other path to it. render_cache.rs 98.21% -> 98.68%: - get_or_insert_with on a missing cell runs the builder (the entry API's insert arm, the complement of the hit arm), and a later read sees the cached state without rebuilding. model.rs 90.99% -> 98.40%: - load_saved() and save() were only exercised by a #[ignore]d test. The test now runs by default: it serialises on a module-local lock and restores whatever was in the shared generated/ directory before it ran, the same save/restore convention the engine's own persistence tests use. ui-controllers total 96.99% -> 99.11% (floor 96). UI lib tests 52 -> 55, all pass, none ignored. Deliberately uncovered, as before: the panic-arm canaries in event_router's positive tests, the frozen-pane `return None` guards in col_at_x/row_at_y (the frozen loop spans exactly the frozen interval, so they cannot fire), the existing unreachable-builder closure in render_cache's hit test, and the environment-dependent save/restore cleanup branch in model's disk test.	2026-08-17 09:26:55 +00:00
andodeki	e46b2c504a	fix(cad): the scene-cache benchmarks were measuring a function that cannot cache ... `REVIEWS/PAY_CAD_IMPLEMENTATION_STATUS.md` has carried the same CAD entry through ten tranches — "unchanged", blocked on "no Cargo toolchain is available in this execution environment to run the required tests/profiles". `profile_benchmarks.rs` imports nothing from Makepad but the math types, so it builds in the same host-only harness the coverage run already uses. `CAD_BENCH=1 ./tools/test-cad-coverage.sh` runs all 16, release, no desktop, self-cleaning. Running them found the drift they exist to catch. `bench_scene_cache_hit_vs_rebuild` reported **1.2×** against the **488×** recorded in BENCH_BASELINE.md, and `bench_scene_cache_scaling` reported 1× at every part count with the warm read scaling linearly — 3.2 µs at 10 parts to 64 µs at 500. That reads as a catastrophic cache regression. It was not. Both called `SceneCache::scene(&[CadNode])`, which is documented as always rebuilding: it takes a bare slice, so it has no generation to compare against and cannot cache. The editor's caching entry point is `scene_for(&PartsStore)`. When the generation-tracked store landed in Phase 5.1 these two benchmarks were not moved with it, so their "warm" sample was a second full rebuild and the printed speedup was allocator noise. Nobody saw it because the benchmarks had not been runnable since. Repointed at `scene_for`, they reproduce the checked-in baseline on different hardware: cold 24.6 µs / warm **48 ns**, **512×** against the recorded 488×, and the warm read is flat at ~55 ns from 10 parts to 500. `bench_scene_cache_hit_vs_rebuild` now asserts `Arc::ptr_eq` across its two samples, so it fails loudly instead of quietly timing two rebuilds if it is ever pointed at a non-caching path again. `SceneCache::scene()` itself is untouched. I started to delete it as a "cacheless method on a cache" and stopped: its docstring says exactly what it does and why, and nine tests use it for precisely that case. The benchmarks were wrong, not the API. Verified: the 16 benchmarks run and reproduce the baseline; the default coverage mode is unchanged at 97.14% with every floor met.	2026-08-17 09:23:32 +00:00
andodeki	33ef24cee5	refactor(cad): one screen-to-world path, not two ... viewport.rs carried two independent implementations of the same geometry. `screen_to_ray_3d` and `screen_to_world_3d` derive the camera basis from yaw, pitch, distance and a 42-degree fov; `camera_eye` and `unproject_point` did the same job by inverting `last_view` and `last_proj`. The matrix pair was dead — nothing in the workspace called either of them, and no script binding registers them by name. Two implementations of one piece of geometry, one of them never executed, is exactly how axis conventions drift apart. The evidence is in the deleted code: `camera_eye` carried its own spherical "fallback" that was a copy of `compute_eye`, complete with a "FIXED: was +cp*cy (must match makepad XR convention)" note about a convention the live copy had already been corrected for. A second copy of a convention is a second place to forget to fix it. So the dead pair goes, and a breadcrumb comment in its place says where screen-to-world actually lives — the question someone will have when they find `mat4_inverse` and wonder why nothing calls it. `math::mat4_inverse` stays. It is correct and covered now, and using the matrices the renderer actually drew with is the better way to unproject than re-deriving the camera basis from Euler angles — that is a real improvement for whoever wants it, and they should start from a version that works. Its doc comment no longer claims callers it does not have. VERIFICATION, stated plainly: `cargo check -p nigig-build` needs the Makepad desktop stack and does not run in the environment this was written in. What did run: rustfmt parses both files (a syntax error would be a parse failure, not a diff); a brace/paren delta count over the deletion (10 opens, 10 closes; 31 parens each way); a repo-wide grep for both names across every file type, which finds only comments; and the engine coverage suite, unchanged at 97.14% with every floor met. The compile is gated by full-crate-check in CI, which is where a missed reference would surface — loudly, and immediately.	2026-08-17 09:12:03 +00:00
andodeki	89ca5186c6	docs(pdf): Phase 4 is not 100% — audit it, and verify the half that is ... Asked whether Phase 4 was complete, I checked the tree instead of my own commit message, and the commit message was wrong. Three items named in the Phase 4 spec are **not** implemented, and the status line said "complete" over them: - **Field-value reconciliation** (`form_reconcile_test.dart`). Setting a value writes /V, marks the field dirty and regenerates /AP — that all works. What is missing is the reconciliation case: a file opened with /V and /AP *already disagreeing*, where the right answer depends on /NeedAppearances. Nothing decides that today. - **Type1/CFF embedding.** The spec hedges with "if feasible", so this is a legitimate deferral rather than an oversight — but "complete" did not say so. `sfnt.rs` detects CFF outlines and `font.rs` reads an existing /FontFile3; nothing writes one. Creation is TrueType-only. - **`repair-cmap`.** No equivalent exists. `text_box_appearance_test.dart` *is* covered, by appearance.rs:235 — it just does not carry that filename, which is why a grep for the dart test names is a starting point and not an answer. The other half of the exit criterion — "generated PDFs open cleanly in external viewers" — had never been checked at all. The sample generator's own doc comment admits no test in this repository can assert it. So I ran it through implementations we share no code with, and **it passes**: qpdf --check no syntax or stream encoding errors pdfinfo title, author, subject, keywords, 2 pages, Form: AcroForm pdftotext all text, including the embedded DejaVu subset and its em-dash qpdf --list-attachments readme.txt, extracted by name with description catalogue /Outlines /Names /EmbeddedFiles /PageLabels /Dests /PageMode /ViewerPreferences /AcroForm `tools/check-pdf-external-readers.sh` makes that repeatable, and pdf.yml runs it. It treats a qpdf *warning* as failure, not just an error: qpdf warns where it had to reconstruct, and reconstructing is exactly what a stricter viewer will refuse to do. Negative-tested twice — removing the attachment fails 3 checks, and corrupting the startxref offset makes qpdf report "file is damaged". Two defects that audit found: - **The sample never exercised XMP**, so the Phase 4 feature most likely to be silently missing was also the one nothing looked at. Probed separately: `set_xmp_metadata` works, pdfinfo reports `Metadata Stream: yes`. - **A `Banner` naming an unregistered font produces a structurally valid PDF that renders no text.** qpdf --check passes; poppler says `Unknown font tag 'F1'` and draws nothing. `stamp.rs` cannot register the font itself — fonts belong to the document, and a banner does not know which document it will be drawn into — so this is now documented on `Banner` with a worked example, and pinned by `a_banner_font_must_be_registered_or_the_page_lacks_the_resource`, which asserts on the page's /Font resources because that is the thing actually missing and the thing a caller can check. The plan now records that it was wrong once, rather than quietly correcting itself. A status line that has been overstated should show its working. Engine suite 952 -> 953. Phase 4's engine half is verified end to end against third-party readers; the ui.rs interaction half is written and still blocked on the Makepad headless backend.	2026-08-17 09:11:03 +00:00
andodeki	6b04b07e14	test(spreadsheet): cover the last reachable engine branches ... Three files, each the weakest reachable logic left after the workbook_api tranche. autofill.rs 92.71% -> 96.26%: - A single chrono value is still a series (delta defaults to 1). - A chrono sequence whose delta changes mid-run is not a series. - A repeated chrono value has delta 0 and is rejected rather than producing an infinite fill. - A backward chrono sequence wraps the delta modulo the list length (Feb, Jan -> delta 11). - get_series_value on Series::None returns None. - match_case with an empty value returns an empty string. undo.rs 98.34% -> 100%: - Redo of a deletion (a Change::SetCell whose `new` is None, the shape recorded by set_cell("") and remove_cell) removes the cell and its dependency-graph edges; undoing it restores both. This is the mirror of the existing create/undo test and was the one uncovered arm in Change::apply_redo. workbook.rs 99.35% -> 99.68%: - detect_workbook_version on a legacy `#MP_SHEET_V2` payload reports version 1. Engine total 98.24% -> 98.50% (floor 96). Unit tests 318 -> 325. Deliberately uncovered, as before: panic-arm canaries in positive tests, the `}` after an unconditional return in detect_series, and the test-harness save/restore cleanup branches in persistence.rs (their inverse runs depending on pre-existing state — the CI-normal path).	2026-08-17 06:31:46 +00:00
arena-agent	dd8cc17b75	fix(doc): re-float the clipboard menu when a selection-handle drag ends ... On mobile the native menu floated at long-press word-select (or select-all) but dragging either handle afterwards re-anchored nothing, leaving the platform toolbar over the previous word — or already dismissed — once the selection had moved. The gesture router's end() only distinguishes PendingLongPress, so the Stop arm now samples the router state first: when the ended gesture was AdjustingStartHandle or AdjustingEndHandle and the session is in Edit mode, the menu re-floats on lift-off through the same cx.show_clipboard_actions request the long-press arm sends, with rect = the adjusted selection's handle union (the existing clipboard_menu_rect). Mid-drag stays quiet (the TextInput cadence DEVICE_VERIFICATION 3.3 documents) and View mode keeps handle drags as pure highlight/merge surface. Tests (2 new): a runtime drive of long-press 'hello' -> end-handle drag onto 'w' in 'world' -> lift-off asserts no request mid-drag, a fresh request on Stop whose rect covers more than the stale word rect, and the focus atom on the dragged-to glyph; the View-mode twin asserts the span adjusts while clipboard_menu stays empty. DEVICE_VERIFICATION gains row 3.6 for the hardware pass.	2026-08-17 06:28:44 +00:00
andodeki	89437838b7	test(spreadsheet): cover workbook_api.rs and util.rs remaining branches ... The two weakest files left after the formula2 tranche. workbook_api.rs 88.80% -> 99.58%: - with_sheets_active with an empty sheet list falls back to a fresh single-sheet workbook. - set_active_sheet with an out-of-range index is a no-op. - remove_sheet: removing a sheet before the active one shifts the index down; removing the active one (or one after it) clamps to the new end. - apply(): the style arms the earlier tests did not touch — italic, underline, number format, background — and both SetBorders shapes (per-edge target writes each requested edge, BorderTarget::None clears all four). - Workbook::default() is a fresh single-sheet workbook. - save()/load() round-trip through the default path, a legacy payload still loads as a one-sheet workbook, and a non-workbook payload yields None. The disk test takes a new crate::test_disk lock so it cannot race the persistence module's disk test under parallel `cargo test`; that pre-existing test now takes the same lock. util.rs 90.91% -> 100%: - write_col_letters matches col_letters across one-, two- and three- letter columns and reuses the caller's buffer without stale tails. - adjust_formula_refs: a digit-bearing function name (LOG10) is not a cell ref; an 8-letter column overflowing u32 and a 30-digit row overflowing i64 are copied verbatim through the overflow fallbacks. Engine total 96.54% -> 98.24% (floor 96). Unit tests 308 -> 318; 9 integration tests unchanged. Deliberately uncovered: the mutex-poison recovery closure in the test_disk lock (unreachable unless a test panics while holding it), and the disk test's restore-to-clean branch (runs only when no save file pre-exists — the CI-normal path; its inverse is the branch that runs otherwise). Both match the existing persistence test's save/restore convention.	2026-08-17 05:37:38 +00:00
andodeki	4a6409f37b	chore(spreadsheet): drop Workbook::load's dead legacy-migrate branch ... `Workbook::load()` carried a fallback — if the saved payload was not a V2 workbook but "looked legacy", migrate it — plus the two private helpers behind it, `is_legacy_workbook_payload` and `migrate_legacy_workbook`. The branch is unreachable. `deserialize_workbook` already treats both legacy V1 formats (single-sheet `#MP_SHEET_V2` and the older CSV) as `WorkbookFormat::V1Legacy` and returns a migrated one-sheet workbook, so by the time `load()` sees a legacy payload the `if let` above the fallback has already returned `Some`. The two conditions are exact complements — any string `deserialize_workbook` rejects is also rejected by `is_legacy_workbook_payload` — so the migrate branch and both helpers are dead code, confirmed by direct probe against `detect_workbook_format`. Behavior is unchanged: the legacy migration still happens, inside `deserialize_workbook`, which the existing `legacy_v1_migration_clears_undo_history` test already pins.	2026-08-17 05:37:38 +00:00
andodeki	624d6b846f	feat(makepad-table): document library for Phase 3, and correct every stale note ... Two things: the model layer Invoicer UI Phase 3 needs, and a sweep of the documentation, which was still describing the crate as it was six phases ago. `DocumentLibrary` in `makepad-doc-model` — open, save, recents and search, in the model rather than the app so it is testable without a window. The UI over it is not built; the README says so rather than claiming the phase. Three things it gets right that are easy to get wrong: - A document number becomes a filename and is user-controlled text. `safe_file_stem` replaces anything outside `[A-Za-z0-9._-]`, so `../../etc/passwd` cannot steer a write out of its directory. Leading dots go too, and a fully-stripped name falls back to `untitled`. - An empty query matches everything, so clearing the search box restores the list instead of emptying it. All terms must match, so each word narrows. - Recents de-duplicate and move to the front. Without that, re-opening one file fills the list with it and evicts everything else. doc-model tests 22 -> 31. Verified by reintroducing four defects: dropping the filename sanitising fails 3, removing the recents de-duplication fails 1, and switching the search from all-terms to any-term fails 2. The empty-query guard is honestly untested and marked as such below. Two test expectations I wrote were wrong and the code was right, which is worth recording because both look like search bugs and are not. Searching "globex" returns the invoice *and* the receipt — both are addressed to Globex, and finding every document for a client is the point. Searching "inv-2024-001" also returns both, because the receipt's line item reads "Invoice INV-2024-001 — Brand identity + website": it is the payment for that invoice, and surfacing it is the useful answer. Documentation, all of which had drifted: - `src/table.rs` called itself a "Phase 1 + 2 scaffold" with "Phase 3+ (SCAFFOLD ONLY — emits actions, no UI yet)". All six phases are implemented; the header now summarises what each one does. - `src/lib.rs` said Phase 3+ was "scaffolded via TableAction emissions but not yet implemented", and did not export the Phase 6 types at all. `parse_solid_spec`, `wireframe_edges`, `project_isometric`, `SolidSpec`, `SolidSpecError` and `Point3` were public but unreachable from the crate root. - `TableAction::RowMenuRequested` / `ColMenuRequested` were documented as "Phase 3 will open a PopupMenu". The widget opens the menu itself; these are notifications, not requests. - Both demos logged "Phase 3 will open PopupMenu" and neither handled `ColumnMoved`, so a Phase 4 drag produced no output in either. - The invoicer's header described a toolbar of six buttons that does not exist and a context menu as pending. - The README's caveats section listed three "if the compiler complains" predictions from before the crate had ever been built. All three are settled — `KeyCode::Tab` is right, `TextInput` needs no `ComponentRef`, pdf-writer 0.15 compiles as written — so it now lists the five real remaining limitations instead. - The README's workspace tree omitted `examples/table_demo` entirely and described `table.rs` as 1145 lines; it is 3260. The "drop into makepad" instructions were quietly wrong after Phase 5 and are now corrected with verified line numbers. They say to register `Table` next to `chart`, which at the pinned revision is line 611 — but `MathView` registers at 617, and `Table`'s DSL body names `mod.widgets.MathView`. Following the old advice literally would register the widget six lines before the type it depends on.	2026-08-17 05:30:24 +00:00
andodeki	d7fcd4c73d	refactor(spreadsheet-ui): extract grid geometry so it can be measured ... The UI coverage exclusion was hiding real logic, and the exclusion note said it was not. `grid.rs`, `ui.rs` and `workspace.rs` are excluded from coverage on the grounds that they carry the `script_mod!` DSL and cannot be constructed without a `ScriptVm`. That is true of the files. It was not true of most of their contents: `grid.rs` is 2,702 lines of which roughly the last 30 are DSL, and of its 59 functions **36 take no `cx`, no `Event` and no `Scope`**. Hit testing, cell rectangles, frozen-pane placement, scroll offsets, resize borders, autofill handle bounds — all arithmetic over plain numbers, none of it reachable by the report, and it carried **zero tests**. Confirmed rather than assumed: a probe test constructing `SpreadsheetGrid::default()` fails to compile, because the `Script` derive provides `script_default(vm)` and not `Default`. So the file genuinely cannot be unit-tested — which is exactly why the logic had to leave it rather than stay behind the exclusion. `geometry.rs` holds that arithmetic now as `GridMetrics`, a plain struct with no Makepad dependency. `grid.rs` keeps no second copy: `metrics()` snapshots the widget's live fields and `col_at_x`, `row_at_y`, `cell_abs_rect`, `range_abs_rect` and `handle_rect` all delegate. A parallel implementation would drift from its own tests, which is the failure this is meant to end, not repeat. Behaviour is unchanged and the semantics were read out of the original before being moved — including the ones that look like bugs and are not: a point left of the row header returns `None` rather than column 0, the frozen pane is searched before the scrolling area, and a fractional scroll offsets by a fraction of the *default* width rather than the overridden one, matching the scrollbar's model. Two review items are addressed on the way. SPREADSHEET REVIEW item 7 names `col_at_x`/`row_at_y`/`cell_abs_rect` as O(N) scans run per frame and per pointer event; item 10 names the geometry tangled through `handle_event`. The maths is now in one place with a stated coordinate convention, which is the precondition for replacing the scans with prefix sums — that is a separate change, deliberately, because this one must not alter a single pixel. 29 tests. They assert relationships rather than constants where the relationship is the contract: every cell origin hit-tests back to its own cell over an 8x6 grid, cell boundaries are half-open so there is no dead pixel between columns, frozen cells stay put under a scroll, and a reversed selection drag normalises instead of producing a negative-sized rect. The fixture grid uses non-uniform sizes on purpose — with every column 100 wide, an off-by-one column index and a 100-pixel offset error are indistinguishable, and so are a width and a height. Verified by mutation, six injected defects, each confirmed red: frozen columns scroll with the grid 1 fail range_rect stops normalising corners 1 fail cell boundary becomes inclusive 1 fail fractional scroll ignored 1 fail handle touch-target floor removed 1 fail resize ignores the header-strip check 1 fail UI controllers 95.70% -> 97.00%, floor 95 -> 96; geometry.rs at 98.78%. UI tests 23 -> 52. The gain is not the percentage — it is 409 lines of logic that were previously invisible to it. The exclusion note now says to audit the list before widening it. An exclusion that quietly grows to cover real logic is worse than no exclusion, because the number stays green while the coverage goes away.	2026-08-17 05:12:59 +00:00
andodeki	c301c7a48f	test(spreadsheet): cover formula2.rs tokenizer/evaluator branches ... The branches the corrected report named, now that every test binary is measured: - Tokenizer: a lone `!` is a parse error; string escapes (`\n`, `\t`, `\\`); a double-dot number (`1.2.3`) is rejected after the tokenizer breaks on the second dot. - FormulaError: `InvalidRef` and `RuntimeError` display strings; the `Display` delegation; `from_display` round-trip for `#REF!...`. - BinOp precedence table, pinned so a shared precedence cannot pass silently. - Value conversions: `to_f64` / `to_bool` / `to_display_string` / `is_error` propagate and render `Value::Error`. - Evaluator: boolean literals; unary `+` (built directly — the parser collapses `+x` to `x`, so the `UnaryOp::Pos` arm only runs on a hand-built AST); single-cell named range; single- and multi-cell range expressions; `SUM(Undefined)` -> UnknownName through `resolve_arg`; `values_equal` text (case-insensitive), boolean, error and mixed-type arms. - evaluate_call: `SQRT(-4)` -> `#DIV/0!`; `LOG10`, `SIN`, `COS`, `TAN`. - evaluate_if: wrong argument count -> `#VALUE:`. formula2.rs 90.11% -> 99.05% lines; engine total 94.38% -> 96.54%. Unit tests 289 -> 308; 9 integration tests unchanged. Deliberately left uncovered: the `invalid number` map_err closure in the tokenizer (an f64 parse of a digit/dot string cannot fail) and the `_ => panic!(...)` arms of existing positive match tests.	2026-08-17 05:04:49 +00:00
andodeki	8776a961ee	chore(spreadsheet): remove dead CellRef::parse_inner ... `parse_inner` is a byte-for-byte duplicate of the earlier iteration of `CellRef::parse` that carried `#[allow(dead_code)]` since a refactor, and no caller in the repository references it. It never ran, so its 60 lines could only ever drag the coverage report down without guarding anything. `parse` remains the single entry point for cell-reference grammar.	2026-08-17 05:04:49 +00:00
andodeki	69f6fb224b	fix(map): NigigMapView packed vertex shader for new DrawVector; fix pay sheet visible ... - NigigMapView shader still used pre-packed fields (u,v,color_r/g/b/a,shape_id,param0..3,clip_radius,cr) which no longer exist on VectorVertexPacked (now uv,color,p0s,p12,p3c,param4/5,stroke_dist). Device log showed dozens of "field u not found on Pod" / "shape_id not found" errors when opening mobility_nav and stack overflow of shader Tables. - Replace vertex: fn() with upstream packed preamble (unpack2f16/unpack4u8, g_uv/g_color/g_p0s/g_p12/g_p3c, expanded/surface_decal, terrain lift, view_rot/tilt, icon_zoom gating, expand_slack clip) as in makepad widgets/src/map/view.rs 2026-08-16. Add missing uniforms (tile_fade,width_correction,face_correction,icon_zoom,height_grow,view_rot,rot_pivot,tilt_params,terrain_tex/org/span/uvfit/fill_lift, shiny_gates/sun) so shader compiles on new draw_vector. - Pay sheet: Button does not have DSL property visible; removed visible:false from pin_eye_btn := Button (the widget is hidden/shown via set_visible(cx,true/false) in code already). Fixes "property visible not defined on type" spam every navigation (4 hits per frame). Verified: previous build succeeded, runtime mobility_nav no longer spams shader Pod errors; map renders via packed path.	2026-08-17 04:51:07 +00:00
andodeki	8325805e22	test(spreadsheet): measure every test binary, and cover what that exposed ... The coverage report was reading one object file. Cargo builds each integration test into its own executable, so measuring only the lib-test binary discarded everything `tests/` exercised. That is not a rounding error. `persistence.rs` reported 41.77% with 14 of its 17 functions apparently never called, while `tests/sync_flow.rs` was calling `save_spreadsheet_state` and `load_saved_spreadsheet_state` on every run and passing. The functions were covered; the report was reading the wrong object. Fixing it alone moved persistence.rs to 72.15% and the engine total 90.09% -> 90.57% without a single new test. This is the third defect of its kind in this script — the ignore regex that excluded the sources being measured, the awk matcher that never fired, and now the single-object report. All three had the same signature: a confident number that was measuring less than it claimed. `--all-targets` for the UI crate too, so a future `tests/` file is measured the day it is added rather than silently skipped. Doing that immediately surfaced `spreadsheet-ui/tests/ui.rs`, which had **never compiled**: the crate did not enable `makepad-widgets`' `test` feature, so `makepad_widgets::makepad_test` did not resolve. `cargo test --lib` never built it and nothing reported the breakage. The manifest now enables the feature, matching `pdf-makepad`, and the two tests are `#[ignore]`d with the same documented reason as `pdf-makepad`'s — the fork has no headless Linux backend. Compiled on every run, so they cannot rot further while appearing to be coverage. Then the branches the corrected report named: - `undo.rs` 86.11% -> 98.34%. Resize undo/redo, both directions. The `None` arms are the substance: a column with no width override must have its key *removed* on undo, not have a default written into it. Writing a default looks identical until the default changes, at which point every previously-resized-then-undone column stops following it. - `persistence.rs` -> 93.70%. The legacy `current.sheet.csv` fallback, including that a whitespace-only current file must not shadow a real legacy one; `save_spreadsheet_state_as` writing where it says it does; and a rejected filename writing nothing at all. - `style.rs` 92.19% -> 100%. Format and alignment codes round-trip, and the codes are distinct — a shared code passes a round-trip test while making two formats indistinguishable on disk. - `model.rs` 84.87% -> 90.99%. Undo/redo intents, from_parts/into_parts, the active-sheet accessors agreeing with each other, and the disk round trip (`#[ignore]`d: it writes the shared generated/ file). Verified by mutation, seven injected defects, each confirmed red: undo None-arm writes a default 6 fail two number formats share a code 1 fail save_as ignores its validation 1 fail legacy fallback removed 2 fail Undo intent wired to redo() 1 fail from_parts drops the active index 1 fail active_sheet_data_mut hits sheet 0 1 fail Two of those changed the tests rather than merely passing: - `save_as ignores its validation` really does write `../escape.tsv` into the crate root, and the file survives the failing run — so every later run failed on the previous run's debris rather than on the current code. The test now removes any leftover before asserting. - `undo_and_redo_intents_reach_the_workbook` failed on first run because `apply()` does not call `begin_recording` and `apply_batch()` does, so there was nothing to undo. That asymmetry is the trap pinned by the engine's `only_set_cell_records_its_own_undo_step`; it now has a test on the model side too, since a caller reaching for `apply` and then offering an undo button gets a button that does nothing. Also fixed a pre-existing clippy **error** in `util.rs` — `approx_constant` on a literal `3.14` in a test that has nothing to do with PI. Confirmed pre-existing by reproducing on a stashed tree. It denies the whole crate, so no clippy gate could be added while it stood. Engine 269 -> 280 tests, 90.09% -> 91.58%; floor 90 -> 91. UI 17 -> 23 tests, 94.55% -> 95.70%; floor 94 -> 95.	2026-08-17 04:42:28 +00:00
andodeki	4ea1224e49	test(cad): actually exercise the GLB parallel path, 94.83% -> 98.65% ... Correcting my own test from two commits ago. `build_glb` only reaches rayon at 32 or more geometric nodes -- below that it runs an ordinary iterator. Every test in the file used one or two nodes, so: - the `par_iter()` arm, the reason rayon is a dependency at all, had never executed; and - the test named "the sequential fallback matches the parallel builder" was comparing `build_glb`'s SEQUENTIAL branch against `build_glb_sequential`. Two sequential paths. It would have passed with the parallel arm deleted. The coverage report is what showed it: those lines stayed red after a commit whose message claimed to cover them. Three tests: - 40 nodes, crossing the threshold, asserted byte-identical to the sequential builder and with mesh names still in order. `par_iter().filter_map().collect()` preserves order; `par_bridge` or a collect into a map would not, and the symptom is a model whose parts are labelled with each other's names. - The sequential builder walks a SceneVisitor whose per-variant arms are separate code from the parallel path's `collect_node`. Seven variants through it, asserting one mesh each. - Nodes that are geometric but mesh to nothing (two empty CSG results) hit the third error arm, on both paths. Without it the exporter writes a GLB with an empty buffer, which a viewer opens as a blank stage and the user reads as a successful export. Floor: arch_gltf 92 -> 97. Verified with: ./tools/test-cad-coverage.sh (555 tests green, total 97.14%)	2026-08-17 04:41:52 +00:00
andodeki	c4b646c1fa	feat(makepad-table): editable header, currency and tax, doc switcher (Invoicer UI Phase 2) ... The invoicer displayed the document number, issue date and due date in three `TextInput`s that were all `is_read_only: true`, because there was nowhere to write an edit back to. This is the write half, plus the currency and tax entry and the sidebar switcher the phase called for. Model (`makepad-doc-model`): - Setters for number, both dates, currency, default tax, issuer and recipient, each trimming its input. A trailing space in a document number becomes a trailing space in the exported filename, and a leading one makes two identical-looking documents sort apart. - `secondary_date`, `set_secondary_date` and `secondary_date_label`, because the second date is a due date on an invoice and a valid-until on a quote, and a receipt has neither. - `Currency::presets()` and `from_code()`. An unknown code is refused rather than turned into an `Other` with a guessed symbol and decimal count, which would format amounts confidently and wrongly. - `TaxRate::parse_percent`. - `switcher_label()`. **A defect this uncovered.** `Document::default_tax()` returned `None` for a receipt, even though `Receipt` carries a `default_tax` field like the other two and its `tax_total_minor()` bills from it. The accessor was the only thing claiming a receipt has no default rate. `document_to_table_data` trusted it, substituted `TaxRate::zero()`, and printed 0% in the Tax column for every un-overridden line while the totals underneath were computed from the real rate — the table and the total disagreeing on the same screen. It never showed because the shipped sample receipt is 0%-rated, so the wrong answer and the right one coincided. It separates as soon as a rate is set, which is exactly what the tax field added here now lets a user do. Fixed at the accessor, so the table builder is corrected without touching it. **A second one.** `TaxRate::percent` casts `f64 -> u32`, and that cast saturates: `percent(-5.0, ..)` is 0%, and so is `percent(f64::NAN, ..)`. Neither refuses, so a user typing nonsense into the new field would have got a plausible-looking rate they did not ask for. `parse_percent` validates first — finite, 0 to 100 — and returns `None` otherwise. Rejected input is reported in the status line and the field is reset to the stored value, so the box never keeps text the document did not accept. UI: - The three header inputs are editable, with a white background and a focus border rather than the read-only grey. - The due-date field used to render "2024-05-01 (valid until)" for a quote — the label baked into the value, so it could not be edited without deleting the annotation. The label is now on the label. - Currency and default-tax fields. - Three sidebar buttons switch document, labelled from the documents themselves, with the current one named below. - Header edits commit on Return or focus loss, not per keystroke: re-reading the model on each character fights the caret, and a half-typed date is not a date. Also fixed, all pre-existing: - `examples/table_demo` did not compile. It used `action.cast::<T>()`, which makepad's Action API no longer has. That package was in no workspace and had no CI until the previous commits, so it never failed loudly — it was simply never built. This is the second defect found purely by putting it somewhere a compiler would look. - Both demos imported `makepad_widgets` alongside `makepad_table`, which re-exports it wholesale, making every widget name ambiguous. - A dead `refresh_totals_display` no-op stub. - Stale "Phase 3 will open PopupMenu" status strings; the menus exist. doc-model tests 12 -> 22, and all five crates now pass `clippy --all-targets -D warnings`. Verified by reintroducing three defects: restoring `None` for a receipt's default tax fails the tax-reporting test, letting `parse_percent` fall through to `percent` fails the validation test, and dropping the trim fails the whitespace test. Invoicer UI Phase 3 (file browser, recent documents, search) remains open.	2026-08-17 04:41:08 +00:00