792 commits

This branch

This branch

All branches

Author	SHA1	Message	Date
andodeki	98460026d3	docs(cad): close the review entry that has said "unchanged" for ten tranches ... The CAD blocker in this document was recorded once, in tranche 1, and then carried forward nine more times as "**CAD** — unchanged" without anyone re-testing it. Both halves of it are false. "No benchmark output is checked in" was already false when written: BENCH_BASELINE.md sits at the repository root with a full table. "No Cargo toolchain is available in this execution environment" was true of CAD and false of the repository — this document's own "Required next commands" section praises `tools/test-rust-clean.sh`, an isolated runner that installs the pinned toolchain and deletes itself on exit, and uses it to validate the two pure Pay crates. The technique was three directories away from the problem for ten tranches. Applying it to CAD is `tools/test-cad-coverage.sh`. Both listed CAD commands are resolved: `send_sync_audit` runs on every push at 100%, and `profile_benchmarks` runs host-only under CAD_BENCH=1 and reproduces the baseline. Replaces the assertion with a table of measurements, and — the part that matters more — keeps three things explicitly *not* claimed: the widget layer still has no coverage number, the host-only harness runs the same sources but not the same target as `cargo test -p nigig-build`, and the two cost_estimator test targets still do not compile. A review that repeats a stale blocker is worse than one that says nothing, because it is what people read to decide what to work on.	2026-08-17 09:24:06 +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	de255a617c	docs(cad): correct an overstated impact claim about the mat4_inverse fix ... I wrote that the broken matrix inverse meant "3-D picking resolved clicks to the wrong world point, about one unit off at a 35 degree yaw". That is not true, and I should have checked before writing it. `mat4_inverse` has exactly two callers -- `CadViewport::camera_eye` and `CadViewport::unproject_point` -- and grepping the workspace shows nothing calls either of them. Live 3-D picking runs through `screen_to_ray_3d`, which builds the ray from the camera's yaw, pitch, distance and field of view and never inverts a matrix. So the defect was real and latent, not real and live: a wrong answer waiting for its first caller. The fix and its tests stand; the severity claim does not, and an overstated bug report is its own kind of defect in a file people read to decide what to work on next. It does surface something worth fixing on its own terms, now recorded here: viewport.rs carries two independent camera-to-world implementations -- the dead matrix pair, and the live spherical one that duplicates the 42-degree fov and the axis conventions. Two implementations of the same geometry, one never exercised, is how conventions drift apart.	2026-08-17 05:26:45 +00:00
andodeki	60e8c0510c	test(spreadsheet): raise the engine floor to 96, where the merge landed it ... c301c7a's formula2 tokenizer/evaluator tests took the engine to 96.70%. A floor of 94 under that protects nothing: two and a half points could regress silently. Same reasoning as ac062ec.	2026-08-17 05:12:59 +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	fc0b1f287f	ci(email): run the conversation-kit tests; mark Phase E complete ... email.yml: the domain test floor ratchets 190 -> 205, and the nigig-email job runs cargo test -p nigig-uikit --lib -- conversation so an email-driven regression in the shared kit cannot silently surface in SMS. The review doc marks E1-E6 done and records the honest correction E5 surfaced: lettre's timeout bounds only the TCP connect, not the greeting/command reads — the send path now bounds the whole operation.	2026-08-17 05:09:15 +00:00
andodeki	1262e71f9a	test(uikit): pin the shared conversation click guard (E6) ... The conversation preview row is consumed by BOTH SMS and email, so a regression there moves two features. Extract should_emit_clicked as a pure function and test it: an empty address suppresses the Clicked action, and so does a scroll in progress. The Clicked payload and props binding are also pinned. nigig-uikit gains its first tests.	2026-08-17 05:09:15 +00:00
andodeki	c51d448ba0	test(email): SMTP sink integration test, and bound the whole send (E5) ... Extract build_email_message (the pure message construction) and send_bounded (the whole send wrapped in platform::timeout). A hand-rolled SMTP sink on 127.0.0.1 now receives a real send and asserts the envelope, every recipient, and the DATA payload — the first time the SMTP conversation has been executed in this repo. This surfaced a real defect: lettre's .timeout() only bounds the TCP connect, not the greeting/command reads, so a server that accepts and never greets hangs the send indefinitely (the review's A6/P4 '60s default' claim was wrong for the read path). send_bounded closes that gap, and a_send_to_a_silent_server_errors_instead_of_hanging pins it.	2026-08-17 05:09:15 +00:00
andodeki	d65f0cd3b8	test(email): property-test the parsers (E2) ... proptest dev-dependency (the same one the SMS crate uses) and a new email_properties module pinning 'never panic + structural invariants' for the untrusted-input parsers: looks_like_email, looks_like_hostname, parse_recipients, is_plausible_address, preview_line (the A3 byte-offset class of bug) and parse_imap_date. Arbitrary input must not panic, and an accepted verdict must satisfy the structural checks it exists to enforce.	2026-08-17 05:09:15 +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	ac062ec3c0	test(spreadsheet): raise the engine floor to 94, where the merge landed it ... Rebasing onto a859053 and a88fb68 put the engine at 94.55%: their new data.rs dependency-graph tests, measured through the every-binary fix in the previous commit. A floor of 91 under a 94.55% measurement protects nothing — three and a half points of regression would pass silently.	2026-08-17 04:45:17 +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	f9f4bda2d8	docs(cad): final coverage table, and the list of what is deliberately not covered ... 96.53% -> 97.14%. Two things worth having in the file rather than only in commit messages. First, the list of what is NOT covered and why: the printpdf emitter (152 lines, checked by opening the file), the save-dialog branch (needs a windowing system), the two persistence wrappers (they write to the real user data directory), the `panic!` arms inside tests, and the one `#[ignore]`d test that pins release-only behaviour. Every one of those is a decision, and a reader who does not know that will either try to "fix" them or quietly lower a floor. Second, a finding about the measurement rather than the code. `build_glb` only reaches rayon at 32+ geometric nodes; every test used one or two, so the parallel arm had never run and the test named "the sequential fallback matches the parallel builder" was comparing two sequential paths. It would have passed with the parallel arm deleted. The only thing that showed it was those lines staying red after a commit whose message claimed to cover them — which is the argument for reading the per-file report rather than watching the total.	2026-08-17 04:42:27 +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
andodeki	a859053bc6	test(spreadsheet): cover remaining data.rs dependency-graph branches ... Targets the uncovered branches the coverage report named, around the formula dependency graph and incremental recalculation: - Formula replacement/removal edge cases: set_cell("") removes the cell and its edges, formula->value and formula->formula rewiring drop stale dependency edges. - No-op removal paths: set_cell("") on a missing cell records nothing. - Dependency-graph cleanup after formula deletion: remove_cell on a formula cell, and update_dependency_graph's defensive path when a dependent has no dependents entry. - Affected-cell recalculation error paths: non-formula cells inside a cycle-affected set keep their raw value; the ="" empty-result regression; the recursive eval slow path (cached AST, parse fallback, and CycleDetected); parse_cell_computed_value arms; parse errors propagating through a dependent and through a large range. Also covers named-range unary expansion (=-Total), the >64-cell range fast path, non-numeric number-format fallbacks, and the demo_q3 / demo_roi constructors. Engine line coverage: data.rs 91.17% -> 97.90%; engine total 90.09% -> 93.24%. Unit tests 260 -> 279 (19 new); 9 integration tests unchanged.	2026-08-17 04:40:28 +00:00
andodeki	a88fb68eab	fix(spreadsheet): B17 recalc invariant must not panic on empty-string results ... `recalculate_incremental` treated "empty computed_value on a changed formula cell" as proof that the topological sort dropped the cell, and debug_asserted on it. But a formula may legitimately evaluate to the empty string (`=""`, `=IF(FALSE, "x", "")`), leaving computed_value empty through no fault of the dep-graph walk. In a debug build that edit panicked the engine. The invariant now checks what it actually meant to check: whether the topological sort *visited* every changed formula cell, using the `visited` set built from `sorted`. Emptiness is no longer used as the error signal, so legitimate empty-string results flow through (the display falls back to the raw formula text, as already documented for empty computed values).	2026-08-17 04:40:28 +00:00
andodeki	bd97e68af0	test(cad): opt-in reuse knobs so the coverage loop is usable while writing tests ... A cold run spends about 90 seconds installing a toolchain and another minute compiling printpdf before it measures anything. That is correct for CI and hostile to the person actually writing the tests, who runs it twenty times in an afternoon — and the workaround is to hand-roll a private copy of the harness, which then drifts from the committed one. Both of the last two coverage pushes were done that way. Better to support it. Three opt-in variables, none set by CI: CAD_COV_TOOLCHAIN_HOME reuse RUSTUP_HOME + CARGO_HOME CAD_COV_TARGET_DIR reuse the build cache CAD_COV_MAKEPAD reuse a Makepad checkout (already existed) With all three: 20 seconds instead of three minutes, measured. The default is unchanged and stays the only reproducible mode: everything under one mktemp directory, removed by the trap. A reused directory is deliberately NOT deleted — it lives outside $WORK by definition, and silently removing a path the caller named would be a nasty surprise the first time someone points it at the wrong thing. The toolchain check is now "is there a cargo binary here", and a reused home that was installed without llvm-tools-preview gets a message naming the component and the rustup line to fix it, rather than a "no such file" on llvm-profdata three steps later. Verified both paths against this commit: hermetic cold run and fully-reused run both report 96.86% and meet every floor.	2026-08-17 04:40:16 +00:00
andodeki	723fe019d0	test(cad): the store's generation contract, the STL trait impl, and two NaN guards ... Mop-up of three files whose remaining gaps were small but not empty. scene_holder 96.16 -> 100.00%, arch_stl 96.83 -> 98.88%, construction_geometry 95.08 -> 95.88%. scene_holder — the generation counter is the whole reason PartsStore exists: it moves on its own so `SceneCache::scene_for` can tell whether its cached scene is stale, instead of trusting every caller to remember `mark_dirty()`. That contract is per-method and nothing checked it: - Reads must NOT bump. Eight of them (len, as_slice, iter, get, find_by_raw_id, index_of_raw_id, is_empty) asserted against one generation snapshot. A read that bumps rebuilds the scene every frame -- slow, and invisible. - `get_mut` bumps only on a hit. Bumping on a miss invalidates the cache for a lookup that changed nothing. - `iter_mut` bumps unconditionally, before it knows whether the caller writes. That is the deliberate conservative choice that replaced the `as_mut_vec()` escape hatch, and it is now pinned so nobody "optimises" it into a lie. - The pairing itself: an unchanged store returns the same Arc, a bumped one rebuilds and the rebuilt scene carries the edit. - `PartIdAllocator::default()` must agree with `new(1)`. Defaulting to 0 would hand out an id that reads as "no node". arch_stl — only `build_stl` was covered, so the `Exporter` impl (the path the export buttons and the async worker take) had never run. Both arms now write the same bytes, both report a failed write, and a group node contributes nothing an empty scene would not: meshing it would add an empty solid and shift every later vertex index. construction_geometry — the two non-finite guards in `snap_to_polar_angle` and `normalize_angle_signed`. `rem_euclid` on an infinity is a NaN, so without them an infinite drag delta becomes a NaN heading and every vertex after it is NaN. Also the documented wrap-round contract at the boundary: 370 degrees behaves as 10, -30 snaps to -45 rather than 315, and pi stays pi because the range is (-pi, pi]. Its remaining 20 uncovered lines are `other => panic!(...)` arms inside existing tests. Those only execute when a test fails, so they are uncoverable by construction rather than untested. Floors: construction_geometry 92 -> 95, arch_stl 94 -> 98, scene_holder 93 -> 99, total 95 -> 96. Verified with: ./tools/test-cad-coverage.sh (552 tests green, total 96.86%)	2026-08-17 04:38:04 +00:00
arena-agent	228bc2c81f	ci(doc-engine): gate the engine coverage, and note it in the doc README ... New coverage job runs tools/test-doc-engine-coverage.sh on changes to crates/apps/doc/**, the script itself, or the workflow. A coverage number nobody gates goes down; the floors (total plus per-file) are the enforcement. The doc workspace README records the milestone and the two CRDT-tolerance behaviors the new tests pin.	2026-08-17 04:33:08 +00:00
arena-agent	9d37874453	test(doc-engine): isolated source-coverage harness with floors ... Mirror of the CAD engine harness for the doc crate, minus the shim gymnastics (doc-engine depends only on serde/serde_json, so it instruments directly): an isolated toolchain + cargo + target dir under one mktemp directory, removed by a shell trap on every exit path; nothing enters the host, the workspace target/, or $HOME. Runs the unit tests plus tests/materialize.rs under -C instrument-coverage, enforces a 96% total-lines floor against a 99.00% baseline plus per-file floors (losing one module's tests must not hide in the total), and with KEEP_COVERAGE=1 writes the uncovered-line listing that makes adding branch tests directed rather than guesswork. COVERAGE.md records the baseline, the exclusions, and the arms that are deliberately left uncovered (defensive CRDT merge arms, one unreachable!, and the Compensation::inverse arms unreachable through the public API).	2026-08-17 04:33:08 +00:00
arena-agent	1269811b44	test(doc-engine): cover the branches the first coverage report named ... A first instrumented run (99.00% -> this branch set is what got it there) showed the gaps precisely; these tests close the reachable ones: - insert_text_at_offset / delete_text_at_offset: mid-block splices, prepend at 0, append at end, backward/forward deletion and every out-of-range guard (both fns were 0% covered). - set_block_alignment materialization, including the CRDT-tolerance arm for a block whose op has not arrived. - set_table_cells: multi-cell batch undoes and redoes as ONE group; empty write lists are rejected. - CrdtDocument::to_json/from_json wire round trip (the format every workspace save rides on) was never exercised end to end. - toggle_text_style_at_offsets rejects unknown fields and missing blocks; a style patch on an EMPTY block is retained rather than dropped. Writing them inverted two expectations and the tests pin the actual -- and correct -- CRDT semantics instead: inserts/cell writes addressed to anchors that have not arrived yet are ACCEPTED into the op log (they must be, to merge when the anchor lands) while conjuring nothing into the rendered document. 96 integration tests pass; clippy stays at -D warnings clean.	2026-08-17 04:33:08 +00:00
andodeki	ede451136b	docs(cad): refresh the coverage table, 88.75% -> 96.53% ... Six tranches since the table was written. It also gains the second finding, which is quieter than the matrix bug and the same shape as it: three of the four exporters had an arm per CadSolid variant and only one variant had ever been walked through them. SVG had boxes, GLB had boxes, the PDF projector had walls. That failure mode is worth writing down rather than leaving in commit messages. A part whose arm is wrong does not fail anything -- the export succeeds, the file opens, and the column is not in it. All four exporters are now driven over every variant they claim to support.	2026-08-17 04:32:52 +00:00
andodeki	c5eefaaea4	feat(makepad-table): 3D cells (Phase 6) ... The last of the README's table phases. `CellKind::Solid3d` reads a short textual description of a solid from the cell and draws an isometric wireframe of it: cube 10 20 30 / cube 10 / sphere 5 / cylinder 3 12 Separators may be spaces or commas, names are case-insensitive, `box` and `cyl` are aliases. Text in, geometry out — the cell stays a `String`, so a 3D column saves, loads and round-trips exactly like every other column, and the kind travels with the column through a drag-reorder. A wireframe rather than a shaded render, deliberately. Shading needs a 3D pass with its own camera, depth buffer and lighting shader; the CAD viewport elsewhere in this repo spends about 2,500 lines on precisely that. A cell forty pixels tall gains nothing from it. Edges projected isometrically and stroked with `DrawVector` need no pass of their own, and isometric has no camera to configure and cannot degenerate. Curved solids are drawn as rings, not their full triangulation: a 40px cell cannot resolve hundreds of triangles and stroking them would cost more than the rest of the table. The projection scales to fit and centres, so a 1-unit and a 1000-unit cube are drawn identically — without that a cell shows either a dot or nothing. Degenerate inputs (no edges, an inset larger than the cell, a zero-size cell, geometry that collapses to a point) return nothing rather than dividing by a zero span, because `DrawVector` silently drops a path containing NaN and the cell would just look empty. Dimensions must be finite and positive, and a rejected spec draws its reason in the cell — `[unknown shape: torus]`, `[cube wants 3 args, got 2]`. Same principle as the LaTeX path: an empty cell and a broken one must not look identical, or a typo reads as a rendering fault. Tests 26 -> 44. Parsing: each shape, uniform and three-dimension boxes, aliases, case, comma separators, and every rejection path including NaN and infinity. Wireframes: a cube has twelve edges and eight corners, extents are centred, sphere vertices lie on the radius. Projection: fits inside the cell, is scale-invariant, is centred, stays finite under extreme aspect ratios, and returns nothing when degenerate. Verified by reintroducing three defects: dropping dimension validation fails 1 test, a fixed scale instead of scale-to-fit fails 2, and removing the re-centring fails 1. That last one is the interesting case, because on the first attempt it failed *nothing*. Every primitive is built centred on the origin, so the midpoint of its projection is already zero and subtracting it is a no-op — the centring tests could not distinguish "centres the drawing" from "happens to be centred". `an_off_centre_solid_is_still_centred_in_its_cell` translates a cube well away from the origin first, and that one does fail. A guard that cannot fail is decoration, and this one could not until it was checked.	2026-08-17 04:32:32 +00:00
andodeki	aad2a20d43	test(cad): cover the PDF plan projection, 75.92% -> 88.99% ... The last of the four exporters with the same gap: the arch projector classifies each node by LAYER NAME and then by geometry, and only the wall path had ever run. Columns, beams, spheres, generic blocks, polygons, the 2-D primitives and CSG results all reached their own `make_*` and none of them were executed, along with both public entry points. 10 tests: - The plan projection negates Z so north is up. One line, and a sign error there mirrors the entire drawing. - Cylinders become Columns and spheres become Spheres, with centre, radius and height asserted, and the label prefix checked -- the labels carry a per-type counter that the drawing schedule reads. - A box on an unregistered layer falls back to a generic Block rather than vanishing. That fallback is what keeps an unclassified part on the drawing. - A beam keeps length on size.x, plan width on size.z and thickness on size.y. Swapping any two produces a plausible-looking beam of the wrong shape. - Polygons and extruded polygons are drawn as their bounding box centred on the polygon's own centre, not the node origin -- the node is at (2, 3) and the triangle's centre is offset from it, so the test would pass either way if it only checked the size. - An empty vertex list emits nothing, rather than a zero-by-zero block at the plan origin. - `export_scene_to_pdf` writes a real `%PDF-` file into a directory it had to create, and reports a path it cannot create. The tolerances in this module are 1e-6 rather than 1e-9 on purpose: every dimension crosses f32 to f64 on the way in, and 0.3f32 as f64 is 0.30000001192092896. The first draft used 1e-9 and failed on the beam. Floors: arch_pdf 72 -> 86, total 94 -> 95. The remaining 152 lines are the printpdf emitter itself -- page furniture, dimension strings and title-block layout, whose output is only meaningfully checked by opening the file. Verified with: ./tools/test-cad-coverage.sh (540 tests green, total 96.53%)	2026-08-17 04:31:44 +00:00
andodeki	2a74c6cac4	ci(email): gate the keystore feature, cover email_bulk ... email.yml: the feature-compile check now covers imap,keystore together. test-email-coverage.sh instruments email_bulk.rs (91.9% line) alongside the rest of the domain; total 89.84%, floors enforced. The review doc records C6/C7/C1f as fully closed, with the honest caveats unchanged (network sockets and the OS vault are compile-checked, not runtime-verified).	2026-08-17 04:29:30 +00:00
andodeki	595ad6ad24	feat(email): pull-to-refresh (C7) and a real OS keystore (C1f) ... C7: pull-to-refresh on the inbox, mirroring the SMS/M-Pesa transaction lists (scrolled + scroll_position over a threshold, throttled to 1.2s and guarded by the in-flight flag). The Refresh button remains for platforms without a gesture. C1f: a real KeyringCredentialStore behind the keystore feature -- the OS credential vault (Linux Secret Service, Windows Credential Manager, macOS Keychain) via the keyring crate, so IMAP credentials can survive a restart. Native only; without the feature active_store() stays fail-closed. The runtime vault is not host-verified (no secret service in CI), which is the same honest caveat as the IMAP transport.	2026-08-17 04:29:30 +00:00
andodeki	765e178737	feat(email): paced bulk send — batch and pace large recipient lists (C6) ... The Bulk tab was capped at MAX_RECIPIENTS (100): a 500-recipient list was refused with TooManyRecipients, not paced. That is a capped single send, not bulk. email_bulk.rs: bulk_send_plan splits a list into provider-sized batches with a pacing schedule (pure, tested), and run_bulk_send executes the plan — gap between batches, rate-limiter backstop, abandon check between every step, per-batch progress. Tested against a mock send (batching, delays, failed-batch counting, abandon). email_send.rs: validate_bulk_message accepts a list over the cap (the caller batches it) while still enforcing subject/body limits. The Bulk page now sends <=100 recipients as one message and anything over as paced batches, posting BulkSendProgress after each batch and at the end. Domain tests 195 -> 206.	2026-08-17 04:29:30 +00:00
andodeki	5e864498d5	test(cad): cover the GLB export entry points and every solid it collects, 75.48% -> 94.83% ... Same shape of gap as the SVG exporter, one file over. The mesh collector had an arm per solid and only boxes were ever walked through it; `scene_to_glb`, `export_scene_to_glb`, `export_scene_to_glb_with_cache` and `build_glb_sequential` -- every public entry point except the one the trait impl uses -- were at zero, along with both `From` conversions on the error type. 8 tests: - Nine solids exported one at a time, each asserted to produce a structurally valid GLB: the "glTF" magic, version 2, and a declared length that matches the file. A viewer rejects the file outright if any of those disagree, so checking "some bytes came back" would not have been worth writing. - The sequential fallback is asserted byte-identical to the parallel builder. It is documented as the path for environments without rayon; if it drifts, that fallback silently exports something else and only those environments see it. - An empty scene and a groups-only scene are both refused, with the two distinct messages. A GLB that opens to an empty stage is worse than a refusal, because the user reads it as "the export worked". - `export_scene_to_glb` creates the directory it was pointed at (the user picks the path, its parent may not exist), the shared-cache variant writes identical bytes, and both report a path they cannot create instead of dropping the export. - The error type's Display, plus its io and serde_json `From` conversions -- those exist so `?` works inside the export path, and an unexercised conversion is a `?` that fails to compile the day someone needs it. Floors: arch_gltf 72 -> 92, total 93 -> 94. Verified with: ./tools/test-cad-coverage.sh (530 tests green, total 95.42%)	2026-08-17 04:29:19 +00:00
andodeki	e16a6da5f5	test(cad): cover the real command context and the undo-stack housekeeping, 87.63% -> 96.23% ... The undo/redo tests all ran against a mock. The mock keeps its own Vec, so `CadCommandCtx` -- the implementation the editor actually uses -- had two methods that no test had ever called: `update_node`, the in-place property edit, and `insert_node_at`, the undo of a delete. `UndoRedoStack::clear`, its Debug impl, and the describe/as_any pair on half the command types were also at zero. 12 tests against the real context, with a real PartsStore: - `update_node` edits in place, bumps the store generation, and does NOT reorder the list. Order is what the layer panel and the draw order read; the same class of reordering defect is already called out in the mock's own comment. - `update_node` on a missing id reports NodeNotFound and does not run the edit closure -- otherwise a stale selection edits whatever node happens to be in that slot. - `insert_node_at` puts a deleted node back at its recorded index, not on the end, and clamps an out-of-range index instead of panicking. The index is captured before the delete and other commands may have shortened the list since. - DeleteNode and CreateNode are round-tripped through the real context, including DeleteNode's no-recorded-index arm (appends) and CreateNode's fallback from `assigned_id` to the snapshot id. Plus the trait and stack housekeeping: - The `Command` defaults: the generic "command" label, and `can_merge` returning false. A default of true would silently collapse unrelated undo steps. - `clear()` empties both stacks. The editor calls it when a document is closed; an entry surviving into the next document applies an edit to the wrong model. - The Debug impl prints depths and asserts the command list is NOT dumped -- a derived Debug over two stacks of boxed trait objects would put the whole edit history in a log line. - Every command type's describe/as_any, including that two commands with identical field shapes do not downcast into each other. That downcast is what `can_merge` runs on; a wrong one turns a drag into one undo entry per frame. Floors: commands 85 -> 94, total 92 -> 93. Verified with: ./tools/test-cad-coverage.sh (523 tests green, total 94.31%)	2026-08-17 04:27:54 +00:00
andodeki	15ef8a0447	feat(makepad-table): LaTeX cells (Phase 5) ... A column can now be declared as maths rather than text: TableColumn { kind: CellKind::Latex, ..Default::default() } Rendering goes through makepad's own `MathView`, which is already registered with the script VM and already owns a glyph cache and the `makepad-latex-math` parse/layout path. Reimplementing that inside the table would have been a second copy of the same logic with none of the same testing. The kind lives on the column, not the cell. Cells stay `String`, so a `TableData` built before this existed keeps working and a table still round-trips through plain text. A column of formulae is also the realistic case — a spreadsheet does not mix prose and LaTeX down one column — and it means the decision is made once per column rather than re-derived per cell per frame. `CellKind` defaults to `Text`, so nothing changes for existing callers except that the struct gained a field. One `MathView` is repositioned over each maths cell in turn, the same pattern `cell_editor` already uses. A widget per cell would allocate a glyph cache per cell. The walk is `Size::Fit` rather than fixed to the cell, because stretching a glyph run to fill a cell distorts the maths. An expression that does not parse is not blanked. `MathView` draws its own `[reason]` marker, so a mistyped formula is visible in the cell rather than silently erasing the content — the failure mode that makes a formula column untrustworthy. Also extracted `align_text_x`, which was inline in `draw_cells`. It counts characters rather than bytes; a multi-byte string measured with `len()` is pushed off the cell entirely. The 7px-per-character approximation is unchanged and still crude — correcting it needs a real text measurer, not a different guess — but it is now in one place and under test, so replacing it will be a visible diff rather than a silent shift. Tests 21 -> 26. New: kinds default to Text, a Latex column keeps its kind through a drag-reorder, left alignment ignores content, centre and right alignment against the stated approximation, and character-vs-byte counting. Verified by reintroducing two defects: measuring bytes fails the multi-byte test, and resetting kind during a reorder fails the kind-preservation test. The invoicer's six columns gained an explicit `kind`. That break was caught by the `Invoicer and demo compile` step added with the workflow in the previous commit, which is what it is there for.	2026-08-17 04:27:12 +00:00
andodeki	44bf7da725	test(cad): cover the shapes the SVG exporter never drew in a test, 82.48% -> 99.02% ... Only boxes were exercised. Cylinders, spheres, circles, arcs, polygons, extruded polygons and CSG results all had their own arm in the SVG visitor and not one of them was executed — 110 uncovered lines, in the exporter that produces the construction drawing. That is the worst shape for an exporter bug: a part whose arm is wrong does not fail anything, it just is not in the drawing. Nothing is red, the file opens, and the column is missing. 13 tests: - Every drawable variant is exported on its own and must produce exactly one path. Nine variants, nine assertions. - A round outline has one point per segment, and a sphere is drawn from segments_u, not segments_v. Both show up visually as a column faceted in the wrong axis rather than as an error. - An arc is sampled inclusively across its 32 segments (33 points) so it closes on the end angle instead of stopping a step short, and a half sweep must not return to its start. - A polygon with two vertices, and an empty CSG result, add no path. An empty `points=""` renders as a stray dot in some viewers. - The `Exporter` impl itself: the cacheless `export`, the cached one (asserted byte-identical), and the write-failure arm, whose message is what the status label shows. Only `build_svg` was covered before, so a broken `export` would have shipped. - A 90 degree yaw must change the projected outline of a polygon. The box arm had rotation covered; the polygonal and round arms use a different projection helper and had none. Floors: arch_svg 79 -> 97, total 89 -> 92. Verified with: ./tools/test-cad-coverage.sh (510 tests green, total 93.44%)	2026-08-17 04:25:04 +00:00
andodeki	a82916b006	test(cad): cover the scene-graph builder API, 81.73% -> 98.53% ... cad_scene.rs is the type every other CAD file is written against, and 388 of its lines had never been executed. The gap was not in exotic corners -- it was the fluent builder the whole editor and the script VM construct scenes through. `NodeBuilder`'s setters, `SceneBuilder`'s starters and domain builders, `push_raw`, `CadTransform`'s helpers, the 2-D solids' size/set_size arms, `ParamHash` over half the variants, `walk_scene`'s dispatch, and the `Exporter` default methods were all at zero. 32 tests, grouped by what they protect: - The setters are checked for what they must NOT touch as well as what they set. `.cube().radius(9.0)` has to be a no-op, not a silent solid swap; `.rotate_y(30).rotate_y(15)` has to be 45 degrees, because every one of these helpers is additive and a helper that assigned instead would drop the earlier call. - The domain sugar is pinned to its documented axes: length/width to size.x, domain_height to size.y, thickness/depth to size.z. Getting one onto the wrong axis gives a wall 0.2 m long and 6 m thick, which reads as a modelling mistake rather than a code one. - The six domain builders are checked for layer, name and their documented default colour. Asserting the colour rather than "not the default material" is deliberate and was found the hard way: Column's grey IS the default colour, so it correctly shares the default material instead of registering a duplicate. - `set_size` is checked on every parametric solid. It is what the properties panel calls, and a missing arm is a control that does nothing -- the same class of defect the by-value-getter CI gate already guards. - `size()` on a CSG or extruded solid is checked against a real mesh bounding box, including the empty-result case. That arm used to return a hardcoded 1x1x1, which made those parts unpickable outside a 1 m box at their origin. - `ParamHash` is checked to move for every field of the 2-D and section variants. It keys the mesh cache AND the viewport's GPU buffers, so a field it does not hash is an edit that leaves stale geometry on screen. - `walk_scene` is checked to route all twelve `CadSolid` variants to their own callback, in order, with `leave_node` always firing. The exporters are all visitors: a variant landing in the wrong arm is a part that silently vanishes from the STL, the SVG or the PDF. A visitor overriding nothing is walked too, so the trait's default bodies are executed rather than assumed. - `export_to_vec`, `export_with_cache` and `spawn_export` -- the default methods an exporter gets for free, all on the async export path -- are driven through a counting stub, with the worker thread joined so the callback assertion is deterministic. Floors raised to lock it in: cad_scene 78 -> 96, total 85 -> 89. Remaining 44 lines are small accessors and defensive arms. Verified with: ./tools/test-cad-coverage.sh (499 tests green, total 92.44%)	2026-08-17 04:23:24 +00:00
andodeki	ab17c72c55	feat(makepad-table): drag-reorder columns, and the first tests this crate has ... Phase 4 of the README's table, plus the test infrastructure Phases 1-3 never had. The crate had zero tests before this; it now has 21. Drag-reorder: - A press on a column header no longer commits to an action. It arms a drag and resolves on release: travel more than 8px and it reorders, release without travelling and it opens the column menu as before. Without that ambiguity resolved, every menu open would jitter into a one-pixel drag. The threshold matches `TouchTracker::MOVE_THRESHOLD` so a mouse and a finger agree on what a drag is. - While dragging, the carried column is tinted full-height and a 2px bar marks the boundary it would land on. The bar is suppressed when the drop is a no-op, so no bar means nothing will happen rather than a bar sitting misleadingly at the source edge. - `TableAction::ColumnMoved { from, to }` fires only when the index actually changed, so a host persisting column order is not asked to write on every wobble. An open cell editor is cancelled, because it addresses a cell by index and the indices just moved underneath it. `draw_drag: DrawVector` — declared, never used anywhere — is replaced by two `DrawColor` layers. `DrawVector` is a full tessellator with path, vertex, index and paint state; a translucent rectangle and a vertical bar do not need any of it. Testability, which needed a structural change rather than a test file: `Table` derives `Script` and `Widget`, so it has no `Default` and cannot be constructed without a live `Cx`. Nothing about it was unit-testable. The logic worth testing does not need a widget, so it moved off it — `ColumnGeometry` owns boundary and drop-position arithmetic, and a free `reorder_columns` owns the move. `Table` forwards to both, and `compute_layout` now goes through `ColumnGeometry` too, so there is one implementation rather than two that can drift. The 21 tests cover column geometry at even and uneven widths and at a non-zero origin, drop-position resolution including the exact-midpoint case and clamping outside the table, the index shift in both directions, no-op drops, out-of-range refusal, cells travelling with their header, ragged rows, a permutation property over repeated drags, and the Phase 3 menu's geometry and hit-testing. Verified by reintroducing three defects separately: removing the shift for the removed source column fails 7 tests, dropping the no-op guard fails 1, and moving headers without their cells fails 3. Phase 3 was marked "scaffolds only" in the README and was in fact substantially complete — menu state, open, hit-test, apply, and drawing all present, with 15 row and column actions wired. Corrected to done, with its geometry now under test. Also adds `.forgejo/workflows/makepad-table.yml`, the first CI this tree has had. Every step passes `--manifest-path` explicitly: the crate is excluded from the root workspace, so `-p` from the repo root cannot reach it and `--workspace` skips it — omitting the flag does not fail loudly, it silently tests nothing. The workflow gates tests, clippy at `-D warnings` and fmt, and asserts three invariants that would otherwise regress quietly: that the exclusion still holds from both sides, that no manifest tracks a git branch instead of pinning a revision, and that monetary fields stay integer. Each gate was checked by breaking what it protects. The exclusion check caught a defect in itself while being tested: a bare grep for the path also matched the explanatory comment above the exclude list, so deleting the entry and keeping the comment passed. It now anchors on the quoted entry. Two pre-existing clippy warnings fixed so the new `-D warnings` gate starts from zero.	2026-08-17 04:22:22 +00:00
arena-agent	d62cc13d34	style(nigig-build): cargo fmt the two test targets left unformatted ... tests/ui.rs and tests/cost_estimator.rs drifted out of rustfmt shape in the recent feature/merge series, failing the whole-crate fmt gate (cargo fmt -p nigig-build -- --check). Mechanical reformat only.	2026-08-17 04:17:36 +00:00
arena-agent	ff0cba1b80	fix(project): replace retired SpreadsheetGrid::select_cell with set_selection_pair ... spreadsheet-ui 6a3c467 removed the direct selection helper, leaving apply_spreadsheet_op calling a method that no longer exists — the nigig-build lib (and therefore every cargo gate) stopped compiling. set_selection_pair((r, c), (r, c)) is the API the same migration series already adopted (8fe7db2); both call sites redraw the view right after, covering the paint the removed helper used to trigger.	2026-08-17 04:17:36 +00:00
arena-agent	5e5adf962d	fix(doc): boot the CRDT editor with content and migrate persistence to the app-data store ... The Android APK (pageflipnav) booted the doc workspace to a blank page. Two compounding causes, both invisible to sandbox gates: - CrdtDocEditor (the active editor since the navigation switch) had no boot init: it starts from DocumentController::default() and only the legacy DocEditor seeded the showcase document behind its initialized gate. The first event on a factory-fresh editor now runs init_document: load the on-disk save when it decodes as #MP_CRDT_V1 wire (initial_document_source gates that so classic-format saves stay with the legacy workspace's first-edit migration), otherwise seed_demo_doc builds a CRDT mirror of demo_doc_blocks() -- styled headings, accent runs, divider, image node, the 4x3 table with bold header, and the closing hint. set_engine flips the same flag so a host-installed document is never overwritten. - persistence.rs resolved its save file under env!("CARGO_MANIFEST_DIR"), baking the build machine's absolute source path into the binary; on device that path does not exist, so Open read nothing and Save wrote nowhere (the errors were swallowed), and on desktop the app polluted its own checkout. Writes now go only to app_data_dir()/nigig_build_store/generated/current.doc.json (the crate-wide convention the CAD store already uses); reads keep a one-way fallback to the legacy source-tree file so an unreplicated developer save is honored once. Boot and migration emit [DOC_TRACE] lines so a device logcat session names the branch that fired. Tests (8 new): boot-source gate (CRDT wire boots verbatim; classic JSON and None route to the demo seed), runtime boot on first event (source-agnostic non-empty projection + flag), host-installed-engine no-overwrite guard, full structural assertion of the seeded showcase, and four temp-dir persistence tests (round trip, store-beats-manifest precedence, manifest fallback, empty-file rejection). DEVICE_VERIFICATION.md gains the matching section-9 hardware rows (9.0 fresh-install demo boot, 9.3 classic-save coexistence).	2026-08-17 04:17:36 +00:00
andodeki	ea98d4d95c	fix(makepad-table): exclude from the workspace, pin makepad, fix money defects ... `crates/apps/makepad_table` is a nested workspace that has never been compiled — its README says as much: "written without a local cargo/rust toolchain, so the first compile on your machine is the verification step." This is that step. Four crates, all of which build, and three defects in the money code that only a compiler and a test runner could have found. Workspace containment, which is what was asked for: - The root manifest now names `crates/apps/makepad_table` in `exclude`. Cargo already declined to absorb it, because the crate carries its own `[workspace]` table — but that made the isolation a property of someone else's manifest. Deleting that table would have pulled four crates and a second makepad checkout into every workspace-wide build. Verified: the root workspace resolves 58 members and none of them are these. - `examples/table_demo` belonged to no workspace at all and had no `[workspace]` table of its own, so `cargo metadata` failed outright in that directory. It is now a member of the nested workspace. Kept rather than deleted: it is the template the README's "drop into makepad" section refers to. - All three manifests pinned to the fork revision the rest of the repo uses (`gitdab.com/andodeki/makepad` @ ecf5a57) instead of tracking `github.com/makepad/makepad` branch `dev`. A floating branch means the same commit of this repo builds against a different makepad from one day to the next, and against a different makepad from every other crate here. All four crates verified to compile against the pin. The defects, in the order they surfaced — each was hidden by the one before it: 1. `format_with_thousands` computed `(i - first_group_len)` before the `i >= first_group_len` guard that protects it. `&&` short-circuits left to right, so the check never ran in time. Any number whose leading group is short of three digits — 2, 3, 5, 6, 8, 9, 11, 12 digits wide — underflowed a usize: a panic in debug, silent wrapping and misplaced commas in release. Every currency string in the application went through it. The two existing tests used 1234 and 1234567, the two widths that happen to work. 2. With the panic gone, `Currency::format` was visibly wrong on negatives. The symbol was emitted before a signed whole part, giving "$-12.34" instead of "-$12.34"; and `whole` truncated toward zero while `frac` used `rem_euclid`, so the two disagreed below zero. -1234 formatted as "$-12.66" and -1 as "$0.99" — the wrong sign, the wrong place, and the wrong amount. 3. `invoice_totals_arithmetic` asserted `1_840_00` where the sample data totals 1_840_000 minor units. The prose in the same comment said 18,400.00, which is right; the literals were a factor of ten low. The arithmetic was never wrong, the expectations were. The two loose range assertions on tax and grand total are now exact equalities. Tests 12 -> 16 across the two crates, and all 16 pass; previously 6 of 12 failed. Each fix was verified by reintroducing the defect on its own: the guard-order bug fails 5 tests, the sign bug fails 4 with the overflow fix left in place, and dropping the per-line discount from the tax calculation fails the arithmetic test by 71.32 — an error the old range assertions were wide enough to have accepted.	2026-08-17 04:10:26 +00:00
andodeki	5cb1bfe9f3	test(spreadsheet): cover the branches the report named ... Nine tests against `workbook_api.rs`, the weakest file in the engine at 77.80%. Now 88.80%; engine total 88.97% -> 90.09%, tests 259 -> 260. One of them pins a trap rather than a bug. `set_cell` calls `begin_recording()` before `apply()`; the other eight public mutators (`remove_cell`, `put_cell`, `set_col_width`, `set_row_height`, `toggle_bold`, `set_number_format`, `set_alignment`, `set_bg_color`) do not. Calling one of those directly and then `undo()` reverts the *previous* recorded edit, not the one just made. Nothing ships broken: every `spreadsheet-ui` call site takes its own `data.snapshot()` first, checked one by one in `grid.rs`. But the asymmetry is invisible at the call site and the next caller will not know to snapshot. `only_set_cell_records_its_own_undo_step` states the current contract so a change to it is a deliberate decision rather than an accident.	2026-08-17 04:05:36 +00:00
andodeki	83839ea0a3	test(spreadsheet): coverage for the UI controllers, and fix a 0% report ... The coverage script measured the engine only, and it cherry-picked four source files to report on, which flattered the number: 91.15% against a hand-picked subset versus 88.97% for the whole of `src/`. Rewritten to cover both crates honestly, with per-crate floors and a listing of uncovered lines. Two bugs in the script itself: - The ignore regex contained the work-directory name, so it excluded the very sources being measured and reported a confident 0%. The work dir also cannot live inside the repo, or Cargo treats the copied crates as workspace members and refuses to build them. - `llvm-cov show` filename headers carry no trailing colon, so the awk matcher never fired and the uncovered-line listing was always empty. `spreadsheet-ui/src/{grid,ui,workspace}.rs` and `src/bin/` are excluded: the first three are `script_mod!` generated DSL and the last is desktop startup, neither of which a unit test can reach. UI controllers now measure 94.55%: `event_router.rs` 70.59% -> 97.96%, `selection.rs` 80.65% -> 100%. UI tests 9 -> 17.	2026-08-17 04:05:29 +00:00
andodeki	6f05c47f20	fix(pay): restore visible:false on pin_input, and gate it (0.3 regression) ... An upstream commit removed `visible: false` from `pin_input` in the shared pay sheet while leaving it on `pin_eye_btn`. Every existing gate still passed, because they all probe the *compile* surface: they prove the field is absent from a packaging build. None of them read the DSL, where the field legitimately exists in a default build and the hiding is what keeps the control off screen until a demo build unhides it on init. That is the DSL-reload hole review item 0.3 asks to close: a live reload re-reads the DSL, so a control that is visible by default there is visible on screen regardless of what init did. `check-no-pin-capture.sh` now walks the DSL for both PIN controls before it runs the compile probe. Verified it fails on the unfixed sheet and passes on the fixed one.	2026-08-17 04:05:21 +00:00