88 commits

Author	SHA1	Message	Date
andodeki	087965f17d	docs(pdf): does the PDF viewport have a minimal-drawcall strategy? No -- and the renderer is unwired ... The same question CAD_DRAWCALL_STRATEGY_ANALYSIS.md asked of the CAD viewport, asked of pdf-makepad, and prompted by the same datagrid brief: "virtual viewport on both axes" and "an optimal minimal drawcall strategy". Those are two techniques, and PDF has a partial version of the first and none of the second. The finding that precedes every other one: `PdfRenderer` is exported from lib.rs and referenced by nothing in the widget's draw path. grep returns the `pub use` and nothing else. `PdfPageWidget::draw_walk` draws a background, then a placeholder or link/field affordances -- it never constructs a renderer and never replays a RenderCommand. Page content costs zero draw calls because page content is not drawn. That also explains the `#[allow(dead_code)]` on ClipRect "retained for the scissor-rect work in Phase 7": clipping is modelled but unreachable. So the numbers below are projections of what happens the moment the renderer is wired in, which is exactly when a strategy stops being theoretical. They are stated as projections in the document. Makepad's batching model was read from source rather than inferred, because the CAD note records getting precisely this wrong in its own first draft (its section 0.1). In draw_vector.rs at the pinned rev, `cx.new_draw_call` appears exactly twice, both inside `end()`. `begin()` clears accumulation buffers; `stroke()` and `fill()` tessellate and issue no draw call. An unbounded number of paints therefore cost one draw call provided nothing calls `end()` between them. renderer.rs calls `end()` from `finish_path()`, and `finish_path()` runs on Save, Restore, PushClip, PopClip and the two Clip ops. Save/Restore are `q`/`Q`: graphics-state operations, not clip operations, and very frequent in real files. Measured by replaying the corpus through that exact state machine -- tools/analysis/pdf_drawcall_census.rs, so the numbers can be reproduced instead of trusted. 165 pages, 15,185 commands, 3,911 draw calls, 23.7 per page. Of the 2,578 vector draw calls, 2,460 are caused by q/Q and **two** by clipping. The renderer flushes on the operation that does not need a flush, and the operation that does need one barely occurs. Colour, stroke width and the CTM are all baked into vertices on the CPU before tessellation, so a state change needs no draw-call boundary; only a clip does, being a GPU scissor concern. Flushing only on clip change takes the vector side from 2,578 to 166 -- about one per page, 15.5x. The blended figure is a more modest 2.6x and the document leads with that rather than the flattering one, because text then dominates: DrawText exposes begin_many_instances, renderer.rs uses neither it nor begin_deferred_slug_flush, so every run is its own batch, and the renderer alternates between three DrawText objects which breaks a batch even when the API is used. On virtual viewports PDF is genuinely ahead of CAD, and the document says so: cache.rs is a real LRU with a byte budget rather than an entry count, generation-tagged, and phase7_exit_criterion.rs asserts the behaviours by name. That is a tested virtual viewport on the page axis. There is none within a page -- draw_affordances loops every annotation filtering only by page index and visibility, never against the viewport rect it already holds, and nothing skips an offscreen command. The asymmetry worth recording for anyone porting the datagrid approach: a grid's virtual viewport is cheap because cell geometry is derivable by division. A PDF's is expensive because geometry is accumulated through a stateful CTM, so a command's screen rect is unknowable without interpreting everything before it. The bbox index is the price of entry and belongs in RecordingDevice, which already tracks the CTM. What is not measured is stated plainly: no GPU profiling, no frame times, because the ui.rs suite that would host a benchmark is still #[ignore]d on the missing Makepad headless backend. Draw calls are a proxy for cost, not cost. 24 per page is not alarming on a desktop GPU; the argument is that the count scales with document complexity rather than viewport size. No source was changed. The suggested order puts "stop flushing on q/Q" first because it is a deletion, and puts wiring the renderer third so the strategy lands with the feature instead of after it.	2026-08-21 05:04:41 +00:00
andodeki	06a3de6126	feat(spreadsheet): dropdown cells and button cells (#9 remainder, complete) ... Finish the widget-in-cell catalog with the two deferred controls, each a plain value plus a render flag (same convention as checkbox/slider/Markdown). - Dropdown: CellStyle.choices (a serialized, pipe-escaped list) makes a cell cycle through its choices on click. The toolbar "Drop" button parses the selected cell's value as a "Low|Med|High" list into choices (undoable SetChoices) and selects the first entry; a dropdown renders its value with a trailing ▾ affordance. dropdown.rs holds next_choice / parse_choice_list. - Button: CellStyle.button makes a cell a button whose value is a TARGET[+N] action spec (A1 ref + optional signed step, default +1). Clicking increments the target cell's numeric value through set_cell (undoable, recalculates dependents) — the spreadsheet-native counterpart of the reference's "+10" boost. button.rs holds parse_button_spec / button_step / format_step_value; the cell renders as a raised box with the spec centred. Toolbar "Btn" toggles the flag. Both flags ride in the render cache so cached cells stay styled, and both commands route through WorkbookCommand (apply + apply_command + dirty marking). Choices serialize after the markdown/slider/button flags; older files default to off/empty. Engine: 491 lib tests (+4) + integration. UI controllers: 142 tests (+8, button 5 + dropdown 4). Coverage: engine 96.59%, ui-controllers 99.45% (floors 96); button.rs and dropdown.rs at 100%.	2026-08-21 04:52:00 +00:00
andodeki	dc1defd7e8	perf(cad): one draw call per shape, not per part -- Phase 3 of the render plan ... A 2,000-part model of six repeated shapes now draws in six calls. It drew in 2,000 before, and in 733 after Phase 1 culling at a working camera distance -- but at 400 m, with the whole site on screen and nothing to cull, it still drew in 2,000. Culling decides which parts are submitted; instancing decides how many calls carry them. They are different axes and this is the one that does not care where the camera is. No shader change was needed and that was the surprise of the phase. `DrawCadMesh`'s `transform`, `color`, `depth_clip` and `display_mode` are `#[live]` fields after `#[deref] draw_vars` in a `#[repr(C)]` struct, which is precisely the per-instance row `DrawVars::as_slice` packs and sends. The loop was already sending instance rows -- one per call. Three methods (`begin_instances`, `push_instance`, `end_instances`), ported from `DrawPbr::begin_many_instances_for_mesh` in the pinned fork, keep the call open across a group instead. New `batching.rs` (pure, 100% covered, floored): group_in_first_appearance_order(items) -> Vec<(K, Vec<T>)> batch_count(keys) -> usize First-appearance order rather than iterating a `HashMap`, deliberately. `HashMap` order is randomised per process, so batch order -- and the order parts reach the GPU -- would differ between runs and after a rehash. For opaque depth-tested geometry that is invisible, which is exactly what makes it a bad thing to depend on: the day someone adds a translucent material it becomes a flicker that reproduces on one machine in five. `FrameBudget` now carries `MeshSubmission { instances, batches }`. Named fields rather than a second positional `usize` because the entire point of the phase is that the two numbers now differ, and a caller that swapped them would report the win backwards. Measured (`bench_frame_submission_budget`, counts): camera 20 m, 2000 parts: 733 visible -> 6 draw calls (was 2000) camera 80 m, 2000 parts: 1459 visible -> 6 draw calls (was 2000) camera 400 m, 2000 parts: 2000 visible -> 6 draw calls (was 2000) ...and with every part a different size, 733/1459/2000 -- the ceiling, which is in the table for the same reason Phase 2's is. WHAT IS NOT VERIFIED, plainly. The instanced submission has never run. There is no GPU, no window and no `Cx` here, and tests/ui.rs still fails at child-build exit 101. What is verified: it compiles against the real Makepad API; the grouping is right (seven tests, including "no item is lost or duplicated", which is the failure mode hardest to see in a screenshot); the budget arithmetic is right; and the batch cannot be left open on any path -- `every_instanced_batch_is_closed_before_the_loop_turns` is a source check in the same style as the redraw_all guard, because a batch left open drops its rows and the parts simply vanish with no error anywhere. Someone with a window needs to open a 3D model and confirm the picture is unchanged. Two things limit the damage if it is not: `begin_instances` returning false falls back to the old one-call-per-part loop (it returns false while the draw shader is still compiling, which happens on the first frames of every window), and batch order is deterministic, so a defect reproduces instead of flickering. Verified: tools/test-cad-coverage.sh green -- batching.rs 100%, cull.rs 100%, total 97.30%, all floors met; cargo check --locked -p nigig-build --lib clean at the 127-warning baseline; cargo test --lib 1100 passed (1091 + 9 new); --test cad_integration 154 passed; CAD_BENCH=1 harness green; cargo fmt --check and git diff --check clean.	2026-08-21 04:41:48 +00:00
andodeki	4aaebafe4e	perf(cad): cull parts against the viewport -- Phase 1 of the render plan ... Before this, `grep -niE "cull|frustum|offscreen|in_view"` over the 2,438 line renderer returned nothing. Every part in the document was tessellated in 2D and issued its own draw call in 3D, on screen or not. At a 5 m drafting zoom over a 200 m site, a 2,000-part model spent 2,000 tessellations and 2,001 draw calls per frame to show twelve parts. New `cull.rs` -- pure, host-testable, 100% covered with a floor in tools/test-cad-coverage.sh: draw_part_2d(centre, half_w, half_h, view, decorated) -> bool Frustum::from_view_projection(view, projection) -> Frustum Frustum::draw_part_3d(aabb, decorated) -> bool world_aabb_from_local_bounds(min, max, model) -> WorldAabb It lives in its own module for the same reason nav_pad.rs and render_budget.rs do: viewport_render.rs is 2,438 lines at 0.00% coverage and needs a Cx, so a predicate written inline there could not be tested at all. Both draw loops AND CadViewport::frame_budget call these same functions, which is what stops the reported budget from drifting away from what is drawn -- the discipline render_budget.rs already applies to the grid loops. Measured (bench_frame_submission_budget, 1920x1080, parts on a 200 m site; full tables in BENCH_BASELINE.md): 2D, 5 m zoom, 2,000 parts: 12 visible. 2,170 -> 182 tessellations, 2,001 -> 13 draw calls. 3D, 20 m camera, 2,000: 733 draw calls, 63% culled. 3D, 80 m camera, 2,000: 1,459 draw calls, 27% culled. Whole site on screen: nothing culled, and nothing should be. That last row is the honest half and it is in the doc too: when the view holds the whole model, all 2,000 parts are genuinely visible and culling cannot help. Phases 2 and 3 (shape-shared geometry, then instancing) are what address that case. Three things came out different from the plan I wrote last commit, and the plan was wrong about each: - **AABB, not bounding sphere.** A sphere around an AABB is looser at the same cost -- a 6 m wall gets a 3 m radius ball. The p-vertex AABB-versus-plane test is strictly tighter. So there is no bounding_sphere helper, and in particular no bounding_sphere FIELD on CadNode: the AABB is already cached by PlacedHash in SceneCache, which invalidates itself on a move or a resize, where a stored field would have to be maintained at 77 construction sites. - **world_aabb_from_local_bounds is shared with pick_part**, replacing the eight-corner transform that was inline there. Two copies would be two chances to disagree about a part's bounds, and picking a part the renderer culled is precisely the bug that disagreement produces. - **The "drawn extent" hazard split into two concrete rules.** The 2D test takes part_to_plane_2d and part_size_on_plane -- the very values the draw uses -- so it tests the drawn rect, not the model extent. And no selected or hovered part is ever culled: the highlight and the tooltip are drawn at the cursor, arbitrarily far from the part. The frustum comes from Gribb-Hartmann row arithmetic on projection * view, deliberately not from mat4_inverse -- that function had a mistyped index in all sixteen cofactors until 2ea5a74, and the terms that were wrong are exactly the ones a perspective matrix makes non-zero. Makepad's Mat4f::perspective is the OpenGL convention (xr/src/scene/xr_root.rs builds the camera with it), so -w <= x,y,z <= w is right; a 0..w depth projection would only make the near plane more permissive, which is the safe direction. Both predicates are conservative by construction: NaN coordinates, NaN AABBs, negative extents and an identity camera all fall through to "draw it". A part kept but invisible costs one submission; a part culled but visible is a bug the user sees. One test of mine was wrong before the code was: the first draft asserted a part 40 m off axis was invisible from a 50 m camera at 60 deg on 16:9. tan(30 deg) * 16/9 is 1.03, so the horizontal half-angle reaches past 45 deg and the part is on screen. The frustum was right; the test is now narrow-fov and says so in its docstring. Verified: tools/test-cad-coverage.sh green -- cull.rs 100.00%, total 97.26% (was 97.20%), all floors met; cargo check --locked -p nigig-build --lib clean; cargo test --locked -p nigig-build --lib 1085 passed (1062 + 23 new); --test cad_integration 154 passed; cargo fmt --check clean; git diff --check clean. Not done here: the two hard-coded 1.2 margins in viewport_render.rs (397-400, 1374-1377) still do not read render_budget::VIEW_MARGIN. cull.rs does, so cull and grid agree by the constant rather than by the code. Folded into Phase 4, which rewrites those loops anyway.	2026-08-20 21:45:45 +00:00
andodeki	4d681dfdbe	feat(spreadsheet): slider cells and inline-Markdown cells (#9 remainder) ... Complete the widget-in-cell gap the same value/style-convention way as the checkbox: two more cell kinds, both plain data with a render flag. Engine: - CellStyle gains `markdown` and `slider` bool flags, serialized as two trailing columns on the CELL line (older files default them off), plus WorkbookCommand::{SetMarkdown, SetSlider} routed through apply and apply_command via mutate_cell — undoable like every style mutation. UI: - markdown.rs (measured): a flat inline parser splitting a cell value into Regular/Bold/Italic/Code runs (`**bold**`, `*italic*`, `` `code` ``); unclosed markers and empty spans stay literal/dropped. The grid draws each run with the bold or regular resource (code tinted like formulas). - slider.rs (measured): 0-100 fraction/value mapping (rounded to whole steps) and track/fill/handle geometry. A slider cell draws the control instead of text, and a press/drag sets the value through set_cell — one undo step per drag (reverse-order ChangeSet application restores the pre-drag value). - Toolbar "Md" and "Slider" buttons toggle the flags on the selection; the render cache carries the two flags so cached cells stay styled. Dropdown and button cells are intentionally not included: a dropdown needs a per-cell choices list and a button needs an action semantic that a spreadsheet does not have — both would require a CellKind in the data model rather than a render flag. Engine: 481 lib tests (+2) + integration. UI controllers: 133 tests (+13, markdown 10 + slider 3). Coverage: engine 96.62%, ui-controllers 99.42% (floors 96); markdown.rs and slider.rs at 100%.	2026-08-20 21:36:21 +00:00
andodeki	14aa0d5017	perf(cad): measure what a frame submits — Phase 0 of the render plan ... Phase 0 of REVIEWS/CAD_RENDER_OPTIMISATION_PLAN.md. No optimisation here; this is the measurement everything after it depends on. 0a first: Makepad does not count draw calls. `Cx::performance_stats` is `PerformanceStats { last_frame_time, max_frame_times }` — frame times only. So the seam had to be built. `render_budget.rs` reports two costs, deliberately not summed: tessellations CPU calls into tessellate_path_stroke draw calls GPU submissions Measured at 1920x1080, now in BENCH_BASELINE.md: zoomed in (0.2 m grid, 170 lines) 2000 parts -> 2170 tess, 2001 draw calls zoomed out (5 m grid, 270 lines) 2000 parts -> 2270 tess, 2001 draw calls The grid column barely moves across a 40x zoom range because the step adapts — that half is already virtualised. The parts column is every part, every frame, on screen or not. **This is not a model of the renderer.** The obvious way to count submissions is to write a second copy of the loop structure and count what it would do, which is exactly how the scene-cache benchmarks ended up timing a function that cannot cache. So `grid_range` owns the decision and `draw_2d_vector_scene` now drives its loops from it: the count and the drawing come from one function and cannot disagree. The ~25 lines of nice-number step arithmetic that were inline in the renderer now live in the pure module, with tests. 11 tests, 100% of the new module, and two of them are there to pin things people get wrong: - the 2D scene is ONE draw call regardless of part count, because DrawVector::end() submits the whole accumulation; - 3D is one draw call per uploaded part, which is where they multiply. One test — culling_shows_up_as_fewer_part_tessellations — asserts the *shape* of Phase 1's improvement before the work starts: parts fall proportionally, the grid column does not move. Verified: 1062 lib tests pass, cad_integration 154, cargo fmt clean, engine coverage 97.20% with every floor met including render_budget at 100%. The benchmark runs host-only under CAD_BENCH=1.	2026-08-20 21:01:52 +00:00
andodeki	8c1a4ad446	feat(spreadsheet-ui): Big Data virtual tab — 1B cells, procedural, timed sort ... Close the UI half of the virtualization gap. A new VirtualGrid widget renders one billion virtual cells (1,000,000 rows × 1,000 columns) from the engine's VirtualSheet: only the visible range is drawn, values derive from a hashed (row, col), and a column-header click sorts the full million rows by permuting an index — with the elapsed time reported in a status strip, as the reference does. - virtual_grid.rs (DSL widget, excluded from coverage like grid.rs): zebra-striped cells, row/column headers with the sort glyph, gridlines, drag-to-pan and wheel scroll, and a 3-state header sort (asc → desc → off) delegated to VirtualSheet::sort_rows/reset_sort. All placement and hit-testing reuses the measured GridMetrics; text widths reuse TextMeasureCache. - workspace.rs: a "BigData" toolbar button overlays the virtual grid over the spreadsheet grid via two child Views toggled with View::set_visible, so the existing spreadsheet path is untouched. Engine: 479 lib tests + integration (grid data-provider commit included). UI controllers: 119 tests. Coverage: engine 96.67% (data_source.rs 100%), ui-controllers 99.35% (floors 96).	2026-08-20 20:48:35 +00:00
andodeki	7946aa889f	feat(spreadsheet-ui): OS-clipboard TSV copy and toolbar zoom (#13, #7) ... Close the two remaining small UI gaps from the datagrid analysis: - TSV copy to the OS clipboard (#13): clipboard.rs gains a tested tsv_escape/rows_to_tsv pair (tab/newline/quote escaping, Excel-style), and the grid answers Hit::TextCopy with the selection as TSV while Ctrl+C also writes it straight to the OS clipboard via Cx::copy_to_clipboard. The in-app clipboard still feeds Ctrl+V paste. - Zoom (#7): a new measured zoom module holds the step factor (×1.15), 50-400% clamps and the minimum/maximum cell-size clamp. The grid's apply_zoom/reset_zoom rescale the default cell size deterministically from a captured base (no floating-point drift), and the cell text's font_scale follows the zoom — with text widths scaled at draw time so right/centre alignment stays true. The toolbar gains - / 100% / + buttons. Engine untouched. UI controllers: 119 tests (+7). Coverage: ui-controllers 99.35% (floor 96), clipboard.rs and zoom.rs at 100%.	2026-08-20 20:14:51 +00:00
andodeki	2adec957e9	feat(spreadsheet-ui): TrendChart line + candlesticks wired to the selected row ... Close gap #11 (chart integration) from the datagrid gap analysis, split as usual into testable headless logic plus a thin DSL widget: - chart.rs (measured): candle bucketing (OHLC from a price series), series bounds, line-point mapping with the reference's 8% padding, the vertical strips that trace a polyline with axis-aligned quads, candle body/wick geometry with up/down classification, and the 1/2/5×10^k axis tick step. - market.rs (measured): a deterministic live market — the reference's splitmix-style `mix64`, a HISTORY-capped random walk per symbol, lazy per-row symbol growth (so any selected row charts), tick() with roll-off, and the derived stats (last/change/pct_change/day_range/candles). - trend_chart.rs: a `TrendChart` widget (excluded from coverage like grid.rs) that colours and draws chart.rs output — gridlines, a polyline for a series, or candle bodies + wicks — via `set_series`/`set_candles`. - workspace.rs glue: a 220px chart panel under the grid (line + candlesticks side by side) fed from a 0.25s `Timer` ticker; selecting a grid row switches the charted symbol and updates the title label. Engine untouched. UI controllers: 112 tests (+16 chart/market). Coverage: ui-controllers 99.32% (floor 96), chart.rs 100%, market.rs 99.36%.	2026-08-20 19:02:02 +00:00
andodeki	1a36ca5538	feat(spreadsheet-ui): interactive checkbox cells (widget-in-cell) ... Close gap #9 (cells hosting widgets) the spreadsheet-native way. The datagrid reference hosts a live CheckBox widget per visible cell and recycles instances from a per-template pool; our grid instead draws the control from batched quads, so there is no per-cell widget object to instantiate or recycle — the render cache and reused draw buffers already fill the role the pool does. - A plain value cell holding TRUE/FALSE now renders as a checkbox: a centred square box with a checkmark when TRUE, and its TRUE/FALSE label to the right. A single click toggles the value (through the normal set_cell path, so it is undoable and recalculates dependents), selects the cell, and does not open the editor — a checkbox is a button, not a text surface. Formula cells are never checkboxes, so a click can't clobber a formula. - The predicate (is_checkbox), the value flip (toggled) and the box geometry (checkbox_layout) live in a new measured checkbox module with unit tests; grid.rs stays thin glue (draw the box/tick/label, reposition the label, and the click handler's toggle). UI controllers: 96 tests (+6). Coverage: ui-controllers 99.20% (floor 96), checkbox.rs at 100%. No engine changes.	2026-08-20 18:28:46 +00:00
andodeki	1a4b479013	feat(spreadsheet): SPARKLINE(range) formula rendered as in-cell sparkline bars ... Close gap #10 (sparkline cells) from the datagrid gap analysis, split as usual into testable headless logic plus thin grid glue: - Engine: a `SPARKLINE(range)` function resolves its single range argument to a flat numeric series and returns a new `Value::Sparkline(Vec<f64>)`. The variant is not a scalar — arithmetic on it is a `#VALUE:` error, and the aggregation/lookup helpers skip it — so no code path invents a number from a chart. `SpreadsheetData::apply_formula_result` stores the series on `CellData.sparkline` (derived state, like spills: never serialized, re-derived on recalculation) and clears it when the formula stops returning a sparkline. Dependency tracking is inherited from the range reference, so editing a cell in the source range re-derives the bars. - UI: a new measured `sparkline` module computes the bar rectangles (bars rise from the series minimum, tinted up/down by last-vs-first trend, gap-shrunk for narrow cells) — the same geometry as the reference `Sparkline` widget, but testable headlessly. The grid draws the bars with a dedicated `draw_spark` resource (depth 0.4) when a cell carries a sparkline and skips the text path; the series is cached in `CellRenderState` alongside the display text. The in-cell editor keeps its text on top by suppressing the bars while editing. Engine: 467 lib tests + integration (483 total, +6 sparkline tests). UI controllers: 90 tests (+5). Coverage: engine 96.60%, ui-controllers 99.15% (floors 96); sparkline.rs at 100%.	2026-08-20 18:17:27 +00:00
andodeki	e13aa03dca	feat(spreadsheet-ui): header sort, column reorder and row/col/all selection ... Wire up the three missing header interactions from the datagrid gap analysis, split as usual into testable headless logic plus thin grid glue: - Selection kinds: SelectionController gains GridSelectKind (Cells/Rows/Cols/All). Header clicks select a whole column / row, the corner (or Ctrl+A) selects the sheet, and bounds() expands the row/column/all kinds to the sheet's full extent so the fill, copy, delete, format and autofill paths handle every kind without branching. - Header-click sort: clicking a column header toggles the sort direction (same column flips asc/desc, a new column starts ascending) and re-sorts the active sheet; the sorted header shows an ▲/▼ glyph. The toggle lives in the new measured sort_state module; the grid holds the (col, asc) state and the workspace applies Workbook::sort_active_sheet. - Column reorder: dragging a header past the tap threshold moves the column, with a drop-indicator line at the insertion index (computed by the new, tested GridMetrics::col_insert_at). Engine addition SpreadsheetData::move_column(from, to) remaps cells and column-width overrides, rebuilds the dependency graph, recalculates, and clears undo history; Workbook::move_active_column propagates to cross-sheet readers via the new WorkbookCommand::MoveColumn. Selection overlay drawing is clipped to the cell area so a full row/column/sheet selection border no longer paints over the headers and the surrounding workspace. Engine: 461 lib tests + integration (477 total). UI controllers: 85 tests. Coverage: engine 96.66% and ui-controllers 99.11% (floors 96).	2026-08-20 17:54:54 +00:00
andodeki	780e323674	refactor(spreadsheet-ui): headless formula-bar state machine, wired to the workspace ... The formula bar's state — formula text, target cell, dirty flag, and the "ignore the next change" latch — lived as five scattered fields plus inline logic inside workspace.rs, a `script_mod!` DSL file that no unit test can construct. That made the formula bar the one piece of the UI whose behaviour was untestable headlessly. Extract a `formula_bar` controller module (the same pattern as the earlier geometry.rs extraction) and wire the workspace to it: - `FormulaBar` owns text/target/dirty/latch with `sync_to_cell`, `on_user_input`, `insert_reference`, `take_commit`, and `sync_after_commit`. The workspace now only bridges it to the Makepad text input; the change/return/focus-loss/undo-redo handlers route through the controller unchanged in behaviour. - Point-and-click formula building (new, Excel-style): while the formula bar is focused, selecting a cell appends its reference instead of replacing the formula being edited. `reference_for(sheet, row, col)` and `quote_sheet_name` build `A1`, `Sheet2!A1`, or `'My Sheet'!A1` with Excel's quoting rules (bare identifier unquoted; spaces, punctuation or a leading digit quoted; embedded quotes doubled) — the machinery that makes the cross-sheet reference syntax buildable from the UI. - The coverage script now measures formula_bar.rs alongside the other controller modules. Tests: 11 new headless unit tests (quoting, reference construction, sync/latch/commit round trip, and insertion into empty/value/formula bars). UI lib tests 62 -> 73, all pass; ui-controllers coverage 98.74% (floor 96) with formula_bar.rs at ~98%.	2026-08-18 11:40:49 +00:00
andodeki	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	77255965fa	test(spreadsheet-ui): measure headless controller coverage	2026-08-18 05:30:48 +00:00
andodeki	df3c650c3e	fix(coverage): invoke native preflight portably	2026-08-18 05:25:29 +00:00
andodeki	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	6bf138d027	ci(email): cover the trip-report modules ... The domain test filter and floor (225) now include finance_report and email_receipts, and test-email-coverage.sh instruments both new files. Domain tests 216 -> 234; coverage 90.6% over 15 files. The review doc records the new feature.	2026-08-17 12:08:19 +00:00
andodeki	0596fc66ed	test(cad): measure the widget layer — 13.15%, and six files at zero ... The number nobody had. The engine harness is structurally blind to viewport*.rs, workspace*.rs and friends, so "97.14% covered" has always been a statement about the smaller half. With the Makepad Linux packages installed, the real crate builds and its tests run under -C instrument-coverage, which makes the other half measurable in about six minutes. 10,714 lines. 13.15% covered. 9,305 never executed by any test. viewport_input.rs 736 lines 0.00% viewport_render.rs 2,083 lines 0.00% workspace_actions.rs 690 lines 0.00% cad_editor_sheet.rs 224 lines 0.00% viewport_2d.rs 138 lines 0.00% code_editor.rs 67 lines 0.00% viewport.rs 3,548 lines 14.37% workspace.rs 2,462 lines 14.18% script_bindings.rs 724 lines 71.27% viewport_input.rs is every click, drag, modifier and keystroke the editor handles, and not one line of it has ever run in a test. viewport_render.rs is every draw call. This reframes the six sessions of engine work above it. 1044 green tests and a 97% engine coexist with an input layer nothing has touched. Those facts were never in tension — they were just never on the same page, because the tool that produced the good number could not see the bad one. A coverage figure that excludes the risky half is not a summary, it is an average with the interesting term deleted. The new script reports the widget files ONLY, on purpose. Folding them into one number would let a 97% engine hide a 0% input layer, which is the arithmetic this exists to prevent. No floors yet, deliberately: a floor at 13% reads as a blessing rather than a debt. The first real input test should set one behind it. Verified: two runs, cold and warm, same numbers; script cleans its profraw data on exit and refuses with a useful message when the native packages or llvm-tools are missing.	2026-08-17 11:59:06 +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
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	5f999b7e9f	fix(tools): make test scripts executable for CI runners	2026-08-17 09:57:13 +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	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	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	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	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	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	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	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
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	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	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	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
andodeki	8701f5df51	feat(pdf): image embedding and header/footer stamping — Phase 4 complete ... The last gap in Phase 4: dart-pdf's header_footer_test, image_stamp_test and image_pdf_test had no counterpart here. What was missing is worth stating precisely, because it is the shape of bug ADR 0017 exists to catch. ContentWriter::draw_image has emitted `q w 0 0 h x y cm /Name Do Q` since Phase 2, and was tested. But nothing in the stack could *create* the image XObject that /Name resolves to. So every Do operator ever written named a resource that did not exist, no document could contain a raster image, and nothing anywhere returned an error. The writing half was present, the reading half faithfully reported the content stream, and the image was simply never there. stamp.rs adds: image XObject embedding, header/footer banners with left/centre/right alignment, image stamp content, and stream composition. A JPEG is embedded as-is with /DCTDecode — PDF's image model is the same DCT data the file already holds, so re-encoding would lose quality for nothing — and its geometry is read from its own SOF marker rather than trusted from the caller, because a /Width that disagrees with the codestream renders as diagonal garbage in every viewer. Raw samples embed as Flate. Embedding an image then adding the page that draws it exposed a live defect in PdfDocBuilder. add_object derived its number from `3 + 2 * pages.len()`, so every add_page after an add_object silently shifted a number already handed out. Embedding an image and then adding its page — the natural order, since the page's content stream has to name the image — produced a page whose /XObject entry pointed at the page object itself: 3 0 obj <</Type /Page ... /XObject <</Im0 3 0 R>>>> The file parsed. The reference resolved. The resource was the page. This is the same positional-numbering defect already fixed once for fonts, one layer out — the comment above first_extra_object_number describes the font version, where /ToUnicode pointed at the descriptor and /FontFile2 at the Type0 wrapper. Both come from deriving object numbers from collections that are still growing. Fixed at the root: the page count is frozen when the first extra number is issued, and pages added afterwards are allocated past the fixed block instead of colliding with it. Non-contiguous page numbers are legal — /Kids is an explicit array — and 952 tests confirm nothing depended on the order. The integration tests parse the generated file back with PdfDocument and assert the image appears in `page.xobjects` with subtype Image, that its /Width and /Height match the SOF marker, and that the header and footer baselines are at opposite ends of the page. Reading the resource back is the assertion that matters: a substring check for "/Im0 Do" passed throughout the entire period when no image could be embedded at all. Verified by mutation, five injected defects, each confirmed red: numbering fix reverted 4 fail JPEG width/height transposed 5 fail header positioned from bottom 3 fail sample-count check removed 1 fail attach_image_to_page a no-op 5 fail One test needed correcting rather than the code: three assertions grepped the output for operators, which are Flate-compressed by default, so they were asserting against compressed bytes. They now disable compression explicitly — the structure is identical either way, and the alternative was a test of miniz_oxide. Engine suite 920 -> 952. Coverage 86.16% -> 86.40%; stamp.rs at 94.64% with a floor at 90. Phase 4 is complete and the plan records it, including the numbering defect, since a status table that lists only features would not have told the next reader why the object numbers look the way they do.	2026-08-16 22:27:32 +00:00
andodeki	1c91d6b398	ci(cad): gate the engine coverage, with per-file floors ... The harness measured; nothing enforced. A coverage number nobody gates goes down. tools/test-cad-coverage.sh now exports llvm-cov JSON and fails when the total drops below 85% or any of the fourteen engine files drops below its own floor. The per-file floors are the point: deleting every test in persistence.rs moves the total by under two points, so a single number would wave that through. Each floor sits a couple of points under today's measurement, so refactoring does not trip it and a real loss does. The low floors are the honest ones. arch_pdf (72) and arch_gltf (72) have gaps in byte-layout paths that only a real PDF or GLB consumer reaches; arch_svg (79) and cad_scene (78) have gaps in widget-facing helpers and defensive arms on invariants SceneBuilder already enforces; exporters (88) cannot reach the save-dialog branch without a windowing system. Raising those needs work, not a bigger number here. Also in this commit, from running the script the way CI will rather than with a warm local checkout: - the Makepad fetch is sparse + blobless + depth 1 over the actual path-dependency closure (math, csg and its six siblings, micro_serde, its derive, micro_proc_macro, live_id, id_macros). 29 MB and two seconds instead of a 319 MB checkout of a repository that is mostly shaders, fonts and demos. Two of those crates were found by the run failing at manifest-read time, which is why the script now verifies all thirteen manifests exist before building instead of trusting the sparse pattern. The new cad-engine-coverage job needs no native packages and no GPU -- makepad-math and makepad-csg are dependency-free Rust, which is the whole reason the engine can be measured at all. It installs its own toolchain into a temp dir and deletes everything through a shell trap: nothing cached between runs, nothing left in the workspace. Verified end to end with a cold run: fresh toolchain, fresh sparse fetch, 466 tests green, total 88.75%, all floors met, environment cleaned.	2026-08-16 22:26:42 +00:00
andodeki	b87d8b0762	test(email): coverage over the full domain; IMAP feature gate in CI ... tools/test-email-coverage.sh now instruments all twelve email files (the new pacing, credential-store, cache, session and imap modules) and enforces per-file floors; measured 90.7% line coverage over the domain. email.yml: the domain test filter gains imap_client::/credential_store::, the test floor ratchets 150 -> 190, the sample-data gate is now a hard zero (sample_thread is test-only), and a new step checks the feature-gated IMAP transport still compiles. The review doc marks Phase C and Phase D complete with the honest caveats (sockets/keystore/pool-reuse are not host-verified).	2026-08-16 22:22:55 +00:00
andodeki	674b2be66d	feat(pdf): JPEG 2000 decoding — Phase 3 complete, all three codecs ... The last codec ADR 0015 deferred. The plan recorded the blocker as a dependency decision, not an algorithm: openjpeg would add a C dependency that breaks the Android cross-compile. This is pure Rust and adds no dependency at all. It shares the MQ arithmetic decoder with JBIG2 — T.800 and T.88 specify the same coder — so the previous tranche paid for most of this one. Context::with_state moved onto the shared type because JPEG 2000 starts three of its nineteen contexts away from state 0 and JBIG2 starts all of them at 0. Implemented: codestream and JP2 container parsing, packet headers with tag trees and the bit-stuffing rule, EBCOT tier-1 (all three passes, four zero-coding context tables, run-length mode), both 5/3 reversible and 9/7 irreversible wavelets, RCT and ICT, arbitrary decomposition levels, and multiple components. Refused by name: multiple tiles, custom precinct partitions, code-block style options, COC/QCC/RGN/POC overrides, subsampled components. Each error says which feature the file needs. This matters more here than anywhere else in the stack, because a JPEG 2000 decoder that quietly skips something does not fail — it returns a slightly soft or banded image that looks entirely fine. That property also dictates how this is tested. Fixtures are produced by OpenJPEG via Pillow and compared **exactly**, sample for sample: the fixtures are lossless 5/3 so no tolerance is needed, and a tolerance is where a subtly wrong decoder hides. Four images — grayscale raw codestream, the same in a JP2 container, a larger one whose tag trees actually branch, and RGB. A generator script is checked in beside them so CI can prove the fixtures still match what produced them. Verified by mutation. The first round was misleading and is worth recording, because it is the same lesson as ADR 0017: DC level shift dropped 3 fail 5/3 lifting rounding changed 2 fail RCT sign flipped PASSED <- survived RCT components swapped PASSED <- survived cleanup run-length disabled PASSED <- survived sign-context XOR dropped PASSED <- survived Four mutations survived because Pillow writes MCT=0 by default, so the RGB fixture coded its three components independently and never reached the colour transform at all. The RCT branch was completely untested while appearing covered — an untested branch that looks tested is worse than one that looks missing. Added rgb8_mct.j2k with mct=1; all four now fail. The header bit-stuffing mutation is caught by the unit test rather than the round-trip. Two real defects found while writing the tests: - A corrupt marker length in a tile-part header walked the read cursor past the codestream and panicked on a slice. Found by the corruption sweep, not by review. The sweep now truncates at every length and flips every byte of a real file, and asserts only that nothing panics. - The 9/7 flat-signal test initially asserted an amplitude I had derived from my own arithmetic. That is a test agreeing with the code by construction. It now asserts flatness — a ripple means the lifting or the edge extension is wrong — and the amplitude is pinned by the OpenJPEG round-trips instead, which use pixels this code did not produce. Also removed two dead fields and an unused parameter that clippy found: Subband::x0/y0 are always zero in the single-tile case this supports, and dead state implying multi-tile support exists is worse than no state. JPX decodes on the image path, like JBIG2, because the codestream carries its own geometry; it stays in REFUSED_CODECS with a reason string saying where it is decoded rather than that it is missing. Engine suite 866 -> 920. Coverage 85.66% -> 86.16%; jpx.rs at 93.72% with a floor at 88. Phase 3 is complete: CCITT, JBIG2 and JPX all land, and the plan is updated to say so and to record how the two gating questions — JBIG2's CVE record and JPX's C dependency — were actually answered.	2026-08-16 22:17:40 +00:00
andodeki	24a26f052e	test(cad): host-only coverage harness for the CAD engine ... The CAD module had 411 tests and no way to find out what they miss. `cargo test -p nigig-build` needs the full Makepad desktop stack -- wayland, X11, GL, alsa, polkit -- so nobody had ever run it under instrumentation, and "well tested" was an assertion, not a measurement. Fourteen of the module's twenty-six files are pure: geometry, the scene graph, undo/redo, the four exporters and file I/O. Their only Makepad imports are the math types, the CSG library and two log macros, all of which are dependency-free Rust. This script copies those fourteen into a temporary crate that carries the SAME module path (`nigig_build::construction_frame::pages::workspace::cad::*`), so the sources compile byte-for-byte with no edits, and runs them plus the real tests/cad_integration.rs under `-C instrument-coverage`. Baseline on this commit: 84.41% of lines over the fourteen engine files and the integration suite. math.rs is 20.40% and persistence.rs is 0.00%. Excluded from the report, per the coverage plan: the Makepad checkout (vendored/generated upstream code), the cargo registry and git caches, the rustc sysroot, and the harness's own lib.rs/shim/picker -- the platform-startup stand-ins the script writes itself, which are scaffolding and not CAD code. The exclusion is enforced twice, by -ignore-filename-regex and by an explicit source list, because the regex alone breaks when CAD_COV_MAKEPAD points outside the temp dir. What it does NOT measure, and does not pretend to: mod.rs, viewport*.rs, workspace*.rs, script_bindings.rs, cad_editor_sheet.rs, code_editor.rs, tools.rs and profile_benchmarks.rs. Those need live_design!, Cx and an event loop; the full-crate-check job in nigig-build.yml gates them. Everything -- toolchain, cargo home, target dir, profraw data, the fetched Makepad tree, the report -- lives under one mktemp directory removed by a shell trap on success, failure, interrupt or termination. The two enums the integration suite borrows from the widget-bound mod.rs are extracted from the real file at run time rather than copied, so the harness cannot silently drift from the crate.	2026-08-16 22:16:56 +00:00
andodeki	81e846ae35	feat(pdf): JBIG2 generic-region decoding, and the bitonal image path ... The second codec Phase 3 deferred. ADR 0015 made a threat review the precondition for implementing JBIG2 rather than an effort estimate, so the review's conclusion is encoded in what this does and does not do. What is implemented: the MQ arithmetic decoder (T.88 Annex E), generic region decoding with templates 0-3 and AT pixels, TPGDON typical prediction, and MMR-coded regions. Segment header and region info parsing, and page composition. What is refused, by name: symbol dictionary, text region, halftone region, refinement region, and a non-empty /JBIG2Globals. Those are the segment types that carry the composition machinery, and shipping them means shipping an interpreter over untrusted input — it is what FORCEDENTRY built its computer out of. A file needing them gets a typed error naming the segment type, exactly as the whole codec used to. The MQ coder itself is pure arithmetic with no file-controlled addressing, which is why it is safe to run and the composition parts are not. Every bound is checked against the declared region size before a buffer is indexed: region dimensions against MAX_DIMENSION and a pixel budget before allocation, segment lengths against the remaining stream, and the region's declared position against the page before a single pixel is written. That last one is the format's actual exploit surface and it has its own test saying so. MMR regions delegate to ccitt.rs rather than carrying a second G4 decoder, so the two cannot drift apart. A test decodes the same coded bits through both paths and requires identical pixels — that is what catches an inverted convention, and JBIG2 is natively 1=black where PDF is 0=black, so the inversion is real and easy to get backwards. JBIG2 is decoded on the image path, not in the filter facade, because it needs /Width and /Height from the image dictionary. It therefore stays in REFUSED_CODECS with a reason string that says where it *is* decoded, so a host showing that string does not tell a user the codec is missing when it is not. CCITT moved the other way for the same reason inverted: it derives its dimensions from /DecodeParms, so it decodes in the facade. Wiring both into ImageInfo::decode_to_rgba surfaced a defect in the parallel-array rule that the CCITT tranche had not reached. For /Filter [/FlateDecode /CCITTFaxDecode] the /DecodeParms array has one entry per filter, and the obvious implementation takes arr[0] — handing the Flate parameters to the fax decoder. ccitt_parms_of finds CCITT's own index instead. This is the same bug ADR 0015 records for the old chain code, in a new place. A declared-but-unresolved /JBIG2Globals returns None rather than decoding without it. Decoding anyway yields a blank or partial image that every caller reads as a success — the declared-versus-delivered failure of ADR 0017. Verified by mutation, six injected defects, each confirmed red: compose bounds check removed 1 fails pack() stops inverting 4 fails (both suites) globals silently ignored 1 fails refused segments silently skipped 1 fails declared-globals check dropped 1 fails ccitt_parms_of always takes slot 0 1 fails 29 unit tests and 12 integration tests, asserting pictures rather than buffer lengths. ADR 0016's stub JPEG decoder returned a correctly sized black rectangle and passed everything that checked a length; these say which colour they expect. Engine suite 825 -> 866. Coverage 85.15% -> 85.66%; jbig2.rs at 94.84% with a floor at 90, and image.rs 31.76% -> 44.77% so its floor rises 28 -> 40. JPX remains refused and is the next tranche.	2026-08-16 22:02:26 +00:00
andodeki	b26e6a1f14	feat(pdf): CCITT G3/G4 decoding — the codec Phase 3 deferred ... ADR 0015 refused CCITTFaxDecode by name and recorded it as the recommended next codec: well specified, no arithmetic coding, no C dependency. This implements it. T.4 and T.6, all three schemes selected by /K: G3 1D modified Huffman, G4 two-dimensional, and G3 mixed with a tag bit after each EOL. Both run-length code books, makeup and extended makeup codes, and the pass/horizontal/vertical mode codes. /Columns, /Rows, /BlackIs1 and /EncodedByteAlign are honoured; /Columns and /Rows are bounds-checked before anything is sized from them, because both are attacker-controlled in a hostile file. It decodes to real pixels, so unlike DCTDecode it belongs in the filter facade rather than the image path: the generic filter contract promises decoded bytes and this can honestly keep that promise. Removed from REFUSED_CODECS, added to SUPPORTED_FILTERS — the registry now describes what the crate actually does. Both existing data-driven registry tests pick this up without editing. Three defects were found by writing the tests rather than by reading the code: - A zero-length run recorded no transition. That is exactly how a row beginning with black is coded — a white run of zero, then the black run — so every such row came out with its colours shifted by one run: "####...." decoded as "....####". - Decoding stopped at bits_left() == 0, but encoders pad the final row to a byte boundary. The padding was fed to the decoder as though it were a code, failed to match, and lost the whole image. Now a trailing all-zero tail is recognised as padding, which is unambiguous because every code book needs a 1 bit. - A row of zero-length runs did not advance the pixel position and looped forever. Found by mutation, not by review. Bounded by the column count: a hang is a worse failure than an error. Verified by mutation, five injected defects, each confirmed to turn the suite red: a0 starts at 0 not -1 1 fails pack_row fills black 13 fails find_b1 parity dropped 1 fails short-/Rows check removed 1 fails read_run returns 0 2 fails Two of those did not fail on the first attempt and changed the tests: - a0 = 0 survived, because no fixture placed a colour change at column 0 — the one position where the off-by-one is visible. Added group4_codes_a_change_at_column_zero. - read_run returning 0 survived because the new run bound also errors, so an assertion of merely "some CCITT error" could not tell the two mechanisms apart. The assertions now name the specific failure. 30 unit tests in the codec, asserting decoded pictures rather than byte counts, plus 6 integration tests through the filter facade covering the chain case, truncation and the spec defaults. The facade test asserts output != input: ADR 0015 records DCTDecode "succeeding" by returning its own compressed input, and a test that only asserted Ok passed against that bug. Engine suite 796 -> 825. Coverage 84.82% -> 85.15%; ccitt.rs at 92.57% with a floor at 88. JBIG2 and JPX remain refused and are the next two tranches.	2026-08-16 21:50:46 +00:00
andodeki	3dab4a1fd5	test(email): coverage floors for the email domain ... tools/test-email-coverage.sh instruments the nigig-core email domain and enforces a whole-domain floor (90%) plus per-file floors on the files that harboured the bugs. It runs in an isolated temp dir and reports over only the seven email source files, excluding Makepad's generated code. Wired into email.yml, which also now runs mail_proxy tests and ratchets the domain test floor to 150. Measured 93.4% line coverage across the domain.	2026-08-16 21:49:12 +00:00
andodeki	82eb6b9c73	feat(pdf): internal links that actually go somewhere ... ADR 0017 left destinations.rs at 0% coverage as an open item. The obvious reading is "an untested module". The real one is worse: nothing called it. It was pub use'd from lib.rs and referenced from nowhere else in the workspace. 0% was not a gap in the tests, it was the symptom of dead code, and nothing else was doing the job. Meanwhile PdfAnnotation read a link's target as dict.get_name("Dest") - a *name* /Dest and nothing else. Not /Dest [4 0 R /Fit], and not /A << /S /GoTo /D ... >>, which is how internal links are written in practically every real document. The corpus has had one since Phase 6, in annotations/links.pdf, and no test asserted where it went: Link { uri: None, dest: None } -> action=None Clicking it did nothing. No error, no warning - the viewer got no action and correctly performed none. A link to nowhere and a link the reader cannot parse look identical from outside. The viewer was already wired for this: PdfAction::GoToPage exists, is matched in test_host.rs, and was never constructed by anything. A complete delivery path with nothing at the source. Now: all three legal spellings parse, named destinations resolve through the /Names /Dests tree *and* the pre-1.2 /Root /Dests dictionary, and resolution happens in page_annotations where the catalogue is in reach. XYZ keeps Option per component because null is meaningful there and only there - it means "leave unchanged". Reading it as 0.0 scrolls to the origin at 0% magnification. Zoom 0 means the same as null and is normalised. Lookup uses a deliberate shallow resolve. Deep-resolving a destination array replaces [4 0 R /Fit] with the page dictionary and destroys the only thing identifying the target - the defect that once emptied every AcroForm (ADR 0006) and every annotation reference (ADR 0004). GoToAction now requires /S to be GoTo. The old code ignored /S and took /D from whatever it was handed, so a /GoToR (another file), /Launch (a program) or /JavaScript carrying a /D was reported as a local page jump. Refuse by verb, same policy as ADR 0012. An unresolvable destination is left unresolved, never defaulted to page 0: silently landing on page one is the worst outcome because it looks like the link worked. Seven mutations, all killed. M1 - removing the /S check - reported as surviving on the first attempt. It had not survived: the patch string omitted an interleaved comment so the mutation never applied and I measured the unmutated build. A harness that does not verify its own mutation says "weak test" when the truth is "never ran", and the conclusion would have been to delete a real security check. Every mutation now asserts it applied. destinations.rs 0% -> 98.65%; total 83.42% -> 83.86%. Floors added for destinations.rs and annotations.rs, verified to fail when breached. AnnotationType::Link changes shape (dest: Option<String> -> destination: Option<Destination>) and AnnotationAction gains GoToDestination; the old field could not express an explicit destination, so keeping it meant keeping the bug. AnnotationAction loses Eq because a destination carries f64 coordinates. pdf: 724 passed (was 695). pdf-ui: 769 passed (was 725). ADR 0018.	2026-08-16 19:34:01 +00:00