87 commits

Author	SHA1	Message	Date
andodeki	4cbb155cb7	perf(cad): stroke per group, not per item -- Phase 4 of the render plan ... A 2,000-part plan view with everything on screen cost 2,270 tessellation calls a frame. It now costs four: two for the grid, two for the parts. `stroke()` tessellates the whole accumulated path and clears it -- `tessellate_path_stroke` ends in `path.clear()` -- so queueing many subpaths and stroking once is one tessellation instead of N. The idiom was already in this file: `queue_dashed_line` has done it for the axis grid since Phase 3.9, guarded by a test. Phase 4 applies it to the two loops that never adopted it. **Base grid: two passes, two strokes.** Minors queued and stroked at 0.55, majors at 1.6 -- the stroke width is the one thing that genuinely needs its own call. `GridRange::has_minor_lines`/`has_major_lines` decide whether a pass runs at all and `frame_budget` counts strokes with the same two predicates, because an empty `stroke()` still enters the tessellator and a budget that assumed two when the renderer made one would be wrong in the direction that hides work. Minors stroke first so majors land on top where they cross; same colour either way, so the only visible difference is that the thicker line wins a crossing, which is the right answer. **Parts grouped by colour.** New `batching::ColorKey` -- the bit pattern, because `f32` is not `Hash` and two colours whose bits differ are two colours -- feeding the same `group_in_first_appearance_order` that Phase 3 groups shapes with. The colour policy moved out of the two draw loops into `constants::part_outline_color`, so the renderer and `frame_budget` cannot disagree about how many groups a frame has; the 2D loop had `vec4(1.0, 0.82, 0.40, 1.0)` written out where `PART_SELECT_COLOR` already existed. **Selected and hovered parts stroke last**, in their own groups, so a highlight is never hidden under a neighbour's outline. They were interleaved in document order before and could be. `FrameBudget` gained `grid_lines` and `part_outlines` beside the call counts. Geometry volume and call count are different numbers now and both are worth reading -- `VectorSubmission { outlines, stroke_calls }` mirrors Phase 3's `MeshSubmission` for the same reason. Measured (bench_frame_submission_budget, 1920x1080, 200 m site): zoom 5 m, 2000 parts: 12 visible outlines -> 4 tessellations (was 2170) zoom 200 m, 2000 parts: 2000 visible outlines -> 4 tessellations (was 2270) The second row is the point, and it is the row Phase 1 could not move: everything is on screen, culling removes nothing, and the frame still costs four calls. WHAT THIS DOES NOT DO: vertex volume is unchanged. The same 2,000 rectangles are tessellated -- in two calls rather than 2,000. What is saved is per-call overhead: tessellator setup, two `std::mem::take`s and an `append_geometry` each time. If a 2,000-part plan view is still slow after this, the remaining cost is triangles, which is Phase 5 and should only happen if a measurement asks for it. One visible-behaviour caveat, stated rather than buried: parts of the same colour are now drawn together, so where two outlines of *different* colours overlap, which is on top can change. They are 1.8 px outlines and the highlight ordering got strictly better, but it is a change to what is drawn, not only to how. Two tests were wrong before the code was, which is becoming this plan's pattern. `constants.rs` fell to 81.82% and the coverage floor caught it -- `part_outline_color` had no tests, and it now has five. And the guard test's first draft looked for a closing brace at a fixed indentation, matched the wrong one, and failed on correct code; it matches braces properly now. Verified: tools/test-cad-coverage.sh green -- total 97.33%, batching.rs 100%, cull.rs 100%, render_budget.rs 99.68%, constants.rs 98.55%, all floors met; cargo check --locked -p nigig-build --lib clean; cargo test --lib 1114 passed (1100 + 14 new); --test cad_integration 154 passed; CAD_BENCH=1 harness green; cargo fmt --check and git diff --check clean.	2026-08-21 05:08:14 +00:00
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	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	1e5d014198	perf(cad): one GPU buffer per shape, not per part -- Phase 2 of the render plan ... Two hundred identical columns held two hundred GPU vertex buffers. They build byte-identical triangles -- `CadSolid::build_mesh` is a pure function of the solid payload and returns local-space geometry, and the upload path (`part_mesh_buffers_from_mesh` -> `MeshSpace::Model`) carries no transform and no colour -- so the duplication bought nothing but memory and upload time. `part_geoms` is now `HashMap<ShapeHash, Geometry>`. **ShapeHash**, new in cad_scene.rs: the solid payload and nothing else. No node id, no transform, no material. `ParamHash` is now *defined in terms of it* -- `id` then `ShapeHash` -- rather than repeating the match arm by arm. That is deliberate: last commit corrected a plan written on the assumption that equal `ParamHash` meant "same shape", which the leading `node.id.hash()` quietly made false. With one shared body the two hashes cannot drift apart again, and `param_hash_moves_whenever_the_shape_hash_moves` pins the join. Three consequences worth naming: - **Staleness became structural.** The key IS the content hash, so the draw loop's `.filter(|(hash, _)| *hash == ParamHash::from_node(part))` is gone: an edited part looks up a key that does not exist yet and `ensure_part_geometry` uploads it on the same frame. There is no "entry uploaded from different parameters" state left to guard against, which is the class of bug `subdivide_selected` shipped. - **Eviction is by live shape, not live id.** This is the hazard the change introduces and it is not obvious: deleting one of two hundred identical columns must NOT drop the buffer the other 199 draw from. An eviction written as "remove the deleted node's entry" would blank most of the model. `geometry_is_retained_by_live_shape_not_by_live_id` pins it. - **Uploads deduplicate within the frame.** Without the `queued` set, the first frame of a 200-column scene would call `get_or_build` two hundred times before the map had anything in it. Measured (`bench_geometry_buffers_shared_by_shape`, counts not timings): 200 identical walls 200 parts -> 1 buffer (200x) 420-part repetitive model 420 parts -> 6 buffers (70x) 420 all-distinct parts 420 parts -> 420 buffers (1x) The last row is in the table on purpose. Sharing is a property of the model, not of the code; a scene where every part differs gets nothing from this phase. And draw calls are unchanged -- still one per visible part -- exactly as the plan predicted. Collapsing those is Phase 3, which needed these shared buffers to be possible at all. Estimated at a week, took under a day. Two things the estimate did not know: `MeshCache::get_or_build` was already a pure function of `node.solid`, and the upload path already produced transform-free, colour-free buffers. It assumed both would need untangling; they were built right the first time. Verified: tools/test-cad-coverage.sh green -- total 97.27%, cad_scene.rs 98.56%, cull.rs 100%, all floors met; cargo check --locked -p nigig-build --lib clean (and one warning fewer: the `ParamHash` re-export in mod.rs is no longer needed by the widget layer); cargo test --lib 1091 passed (1085 + 6 new); --test cad_integration 154 passed; CAD_BENCH=1 harness green; cargo fmt --check and git diff --check clean.	2026-08-21 04:18:33 +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	42b1e7152a	docs(cad): merge a second render review into the plan, and correct the hash both reviews got wrong ... A second optimisation plan arrived (frustum culling, geometry merging, GPU instancing, octree/LOD). Rather than run two plans, every claim in it was checked against the tree at 14aa0d5 and merged into REVIEWS/CAD_RENDER_OPTIMISATION_PLAN.md with a verdict and the evidence behind it. Three of its ideas change what the phases will do; two would have made things worse; one of them found nothing, but reading it found something in mine. The correction that matters, and it lands on my own plan: ParamHash::from_node hashes node.id FIRST (cad_scene.rs:1871), before a single geometric parameter. My Phase 2 said "parts with equal ParamHash are geometrically identical by definition -- that is what the hash means". True and vacuous: equal ParamHash also means the same node. Two identical columns hash differently, so re-keying part_geoms by ParamHash -- the central move of that phase -- would have shared exactly nothing. The incoming plan inherited the same assumption from the same type name, which is the kind of coincidence that argues for a test rather than a paragraph, so there is now one: param_hash_is_not_a_shape_key_it_includes_the_node_id. The id is not a bug. MeshCache and part_geoms are both keyed by NodeId and only ask "is this entry still valid for this node", where the id is a constant. Sharing geometry needs a second hash, ShapeHash: the same body without the id. Phase 2 now carries that instead. Adopted from the incoming plan: - Frustum culling in 3D. SceneState3D { view, projection } is already captured into self.last_view/last_proj every frame (viewport.rs:3781) immediately before draw_scene, so the planes are a row add/subtract away -- and deliberately not via mat4_inverse, which had a mistyped index in all sixteen cofactors until 2ea5a74. My Phase 1 had only an AABB-versus-rect test, which is right for 2D and wrong for perspective. - Instancing, upgraded from "confirm the machinery exists" to "here it is". DrawPbr::begin_many_instances_for_mesh / push_many_instance_with_transform / end_many_instances (draw/src/shader/draw_pbr.rs:2696-2745 in the pinned fork) do exactly this for a shared mesh geometry, and per-instance data is just the #[live] fields after #[deref] draw_vars -- the layout DrawCadMesh already has (color, transform, depth_clip, display_mode). No shader rewrite; ~30 lines ported. The incoming plan rated it 7 days and high risk with "if supported"; it is neither. DrawCadMesh is also declared alpha_blend: false, so regrouping draw order is safe here. - Its dependency graph: instancing batches per geometry, so shared geometry genuinely gates it. Rejected, with reasons in the doc: - bounding_sphere as a stored field on CadNode. 77 construction sites, and this codebase has already paid for hand-maintained derived state once -- the part_geoms staleness bug (viewport.rs:4629) drew the pre-edit shape because one edit site forgot to invalidate. The sphere is two lines from SceneCache::world_aabb_for, which is keyed by PlacedHash so it invalidates itself on a move or a resize, and is benchmarked at 4.72x. - Merged vertex buffers per PartKind with transforms baked in. MeshCache stores local-space meshes on purpose ("the transform and the material are NOT hashed"); baking transforms inverts that and turns dragging one column into a full re-concatenate and re-upload of every column, every frame of the drag. It also cannot express per-part selection colour. That phase's own draw loop still sets a transform per part, which is still N draw calls -- the two halves contradict each other. - Octree above 500 parts. The cached world-AABB pass is 20 us at 500 parts, 0.12% of a 16.7 ms frame. A tree that replaces 20 us cannot pay for itself. Revisit on a measurement, not a part count. - "Log skipped-part count per frame" as verification. Phase 0 already built the counting seam; a log line nobody reads is a step that cannot fail. Also corrected REVIEWS/CAD_DRAWCALL_STRATEGY_ANALYSIS.md, which is where the ParamHash claim originated: section 0 is now two corrections rather than one. Verified: tools/test-cad-coverage.sh green, total 97.20%, all floors met, cad_scene.rs 98.53%; rustfmt clean; git diff --check clean.	2026-08-20 21:17:07 +00:00
andodeki	f27ace8b7f	docs(cad): render optimisation plan — and a correction to the analysis it rests on ... Writing the plan meant checking the assumption the analysis rested on, and the assumption was wrong. I had called `draw_vector.stroke()` a "tessellation/flush point" and then reasoned about the 2D path as though each cost a draw call — "~670 flush points per frame where four would do". Reading draw/src/shader/draw_vector.rs instead of inferring from a comment: begin() clears CPU accumulation buffers stroke() calls tessellate_path_stroke + append_geometry — no draw call end() the ONLY place cx.new_draw_call appears, twice, both inside it The whole 2D vector scene is **one draw call**. Makepad's DrawVector is already a batching design and the CAD code uses it correctly in that respect. The guard test's wording — "thousands of tessellations per frame" — was accurate and literal, and I read "draw call" into it. What survives: no culling anywhere, which was and remains the main finding; and 3D issuing one draw call per part, which is where draw-call multiplication is actually real. What changes: batching the 2D loops is a CPU per-call-overhead win, not a draw-call win, so it drops from second place to third in the plan and the document says plainly that it is the small one. The plan itself, ordered on the corrected facts: 0. Make it measurable. profile_benchmarks.rs has sixteen benchmarks and none measures frame submission. Check whether Cx already counts draw calls; if not, add a counting seam. This doubles as the first test surface viewport_render.rs has ever had — it is 2,083 lines at 0.00% coverage. 1. Cull against the viewport, reusing the cached world AABB that is already benchmarked at 4.72x and already wired to the mouse-move path but not the per-frame one. 2. Key part_geoms by ParamHash instead of part id so identical parts share geometry — the real draw-call win, and the one that matters for drawings full of repeated columns. 3. Reduce 2D tessellation calls with the queue-then-stroke idiom the axis grid already uses. 4. LOD, only if the Phase 0 numbers justify it. Explicitly not doing: a BVH (a linear pass over cached AABBs is microseconds at this scale) or a render-path rewrite (0% coverage). One implementation hazard recorded in Phase 1: cull on the drawn extent, not the model extent. Selection outlines and hover highlights exceed a part's AABB, and culling on the AABB alone makes them vanish at the viewport edge.	2026-08-20 20:44:18 +00:00
andodeki	e6fbcc12da	docs(cad): analysis — the renderer has no culling and no draw batching ... Asked whether the CAD viewport has the kind of minimal-drawcall strategy a datagrid needs, and whether virtual-viewport techniques transfer. Answer: the codebase already knows the technique, applies it in exactly one function, and does not apply it in the two loops that run every frame. `draw_vector.stroke()` is the tessellation/flush point. `queue_dashed_line` queues segments and issues ONE stroke for the whole axis grid, and a test in viewport.rs enforces it — "the grid lines should share exactly one stroke", with a comment warning that stroking per dash is "thousands of tessellations per frame". The discipline is understood and guarded. Two loops away from it: - the base grid strokes once per line (~167 on a 1080p viewport, where two would do: one for minors, one for majors); - the parts loop strokes once per part; - the 3D path issues one draw_mesh.draw() per part, each with its own geometry buffer and uniforms. No instancing, no state sorting. And there is no culling at all: grep for cull/frustum/offscreen/in_view across the 2,461-line renderer returns nothing. Every part is submitted every frame whether on screen or not. The sharp part is that the broad-phase already exists. SceneCache::world_aabb_for is cached by PlacedHash and BENCH_BASELINE.md records it at 4.72x faster than recomputing. Its only caller is pick_part — the mouse-move path, which is additionally throttled by HOVER_PICK_MIN_MOVE_PX. The cheap visibility test is wired to the occasional path and not to the per-frame one. The grid, to be fair, IS virtualised properly: visible world bounds plus 20%, with a 1/2/5 nice-number step that adapts to zoom. That is the datagrid technique done right. It just stops at the grid. Also recorded: what does not transfer. Widget recycling has no CAD analogue, and index-range virtualisation does not either — CAD is continuous space, so it needs a spatial test rather than a row range. At 500 parts a linear pass over cached AABBs is microseconds; a BVH only earns its complexity somewhere past ~50k parts and nothing suggests that is the target. Caveat stated in the document: no profiling was run, and there is no frame-submission benchmark in profile_benchmarks.rs — its sixteen benchmarks all measure CPU work. The structural claims are read off the code and are solid; the consequence in dropped frames is not measured, and measuring it needs a live Cx.	2026-08-20 20:37:00 +00:00
andodeki	a82c8f7ff7	feat(pdf): Unicode-aware search and layout-aware reading order ... Phase 8 bullets one and two. Probing the existing code first, as the workflow requires, found five defects rather than the one the plan names: SPLIT MATCH 'Hello': 0 hits plain_text: "Hello" PRECOMPOSED 'café': 0 hits COLUMNS plain_text: "LeftTopRightTop\nLeftBotRightBot" OUT OF ORDER plain_text: "second\nfirst" `PageText::find` searched one run at a time and documented that as a known limitation. It is a limitation from inside the code and a broken feature from outside it: a writer starts a new run wherever it adjusts kerning, so an ordinary word arrives as two runs, and the find bar says a word plainly visible on the page is not there. `search.rs` indexes the page as one flattened string with a map back to (run, character), so a cross-run match is found and highlighted with one rectangle per run — never a merged box, which across a line break covers half the paragraph. The separator between two runs is a geometric question with three answers: abutting runs join with nothing (one word, split by kerning), separated runs with a space, and a different line or column with a newline. The newline matters as much as the empty join: joining lines with a space lets "one Right" match across a column gutter, text that appears nowhere. Whether two runs share a column is *asked* of the layout analysis rather than re-derived, or the extracted text and the searched text disagree about where a column ends — the original defect wearing a different hat. NFD, never NFC: composition needs the next character, so an NFC fold applied per character composes nothing and the two spellings of an accent stay different. That was a real bug in the first draft. And case *folding*, not lowercasing — Rust lowercases ß to ß, so "Strasse" never found "Straße". Columns are detected before lines, because two columns share their baselines; that is what makes them columns. Bands are separated by a gutter rather than by bare non-overlap, since two abutting runs on a line do not overlap either. Also fixed, found by running the gates rather than by looking: a stream reader trimmed a trailing CR before `endstream` as if it were the writer's separator. Binary data ends in CR about one time in 256, and when it did the reader returned a stream one byte short — no longer AES-block-aligned, so decryption produced garbage and Flate failed. Roughly one encrypted document in 250 was silently corrupt on read. The test failed once under coverage, passed five times in isolation, and failed 2 in 40 when actually counted. A /Length consistent with the file is now the authority; both stream readers are fixed and a test reads one file through each. 1477 tests pass (was 1426), coverage 88.37%, all floors met, external readers pass. 10 mutations across the two modules, all killed. ADR 0034.	2026-08-19 16:12:12 +00:00
andodeki	1740da3f34	feat(pdf): render a form XObject to pixels — the golden caught what the ... assertions missed `Rasteriser::register_xobject` takes a form's recorded commands from a caller that can resolve the page dictionary, so Phase 7's last golden-corpus criterion is met with pixels instead of with a request recorded by name. The first golden of that page showed the form drawn at the **page origin**, ignoring the `1 0 0 1 20 20 cm` that placed it. The recorded commands' `SetTransform`s are absolute in form space, and replaying them overwrote the page's CTM rather than composing with it. Nested lists now compose against the CTM in force at the `Do`. The colour assertions written next to that golden all passed while the bug was live — a red square two pixels from where it belongs is still a red square somewhere. That is the argument for pixel goldens in one sentence, and it is why the golden is compared after the assertions and not instead of them. The offset now has its own assertion too. The new fixture's form deliberately overflows its own /BBox, so the clip is visible in the golden as an absence rather than being taken on trust. Phase 7's golden-corpus exit criterion is now met in full. The `ui.rs` smoke tests remain blocked on the Makepad headless backend, as they have been since Phase 1, and are still not claimed as done. 1426 tests pass.	2026-08-18 20:06:50 +00:00
andodeki	d3089bc62a	feat(pdf): nested content, the wire codec and tiled rendering — Phase 7 closed ... Three bullets, and the Phase 7 status table rewritten row by row. **Nested content (ADR 0032).** A form XObject and a Type 3 glyph are the same problem: a content stream inside a content stream. Both were parsed completely and then not run. `paint_x_object` reported the name for "the host" to resolve and no host existed, so `Do` painted nothing. Type 3 was worse because it looked more correct — `d0`/`d1` reached the device, so the pen advanced by the declared width and the page rendered an invisible line of text with correct spacing after it. `nested.rs` runs both, in pdf-graphics because the dependency runs graphics → document and this is the only crate that can see the interpreter and the object model at once. Forms get their `/Matrix`, their `/BBox` clip and a save/restore wrapper, because without the wrapper a form's colour leaks into every object after it and looks like a bug in the document. Type 3 composes translate-then-matrix; the other order scales the translation and puts the glyph at (1.7, 16.8) instead of (72, 700). Recursion is bounded in both: unbounded, a self-referencing form is a stack overflow reachable from an untrusted document, which is a denial of service and not a rendering bug. **Wire codec and tiling (ADR 0033).** `worker.rs` moved interpretation off the UI thread only because both ends shared a Vec. Tags are explicit numbers, never declaration order, so reordering the enum cannot silently make old recordings decode as different commands. Truncation is an error rather than a short list — a decoder that stopped early would render a page missing its last few operations, plausible and wrong. The obvious truncation test failed, correctly: `Save` is one byte, so a cut on a command boundary really is a complete list. It now tries every cut position and requires each to be a named error or a genuine prefix. Tile skipping is conservative. A command whose geometry is unknown is kept, because dropping a state change corrupts everything after it in that tile, silently. Only untransformed geometry that provably falls outside is dropped. Every tile is asserted pixel-identical to that region of the whole-page render: tiling that is fast and different is not an optimisation. Eight mutations across the two modules, all killed. Phase 7 status is now two tables — the eight spec bullets and the exit criteria — with what is missing named in the row rather than rounded up. Three rows are not green: Makepad blend compositing needs render-to-texture, the image-XObject pixel golden asserts the request rather than pixels, and the `ui.rs` smoke tests remain blocked on the headless backend they have been blocked on since Phase 1. 1425 tests pass, coverage 88.10% (was 87.60%), all floors met, external readers pass. ADRs 0032 and 0033.	2026-08-18 20:00:18 +00:00
andodeki	f37197781e	feat(pdf): glyph outlines from TrueType and CFF, and glyph-aware text runs ... `sfnt.rs` read the metric tables and nothing else. It could say how wide a glyph was and not what shape it had, so every renderer drew embedded text with a substitute font at the correct advance — the failure mode that looks most like success: the line breaks land right and the letterforms belong to somebody else. `outline.rs` returns one outline type for both formats. TrueType quadratics are degree-elevated to cubics, which is exact, so no format detail leaks to a consumer. Composite glyphs are placed by their offsets and scales, with a depth bound because a font can reference itself. CFF Type 2 charstrings run through an interpreter with biased local and global subroutines, hints, hintmask byte counting, the leading width operand, and the FontMatrix as declared rather than assumed to be 1/1000. Separately, `ShowTextWithMetrics` carried one advance for a whole run — enough to move the pen to the next run and nothing else. So `text.rs` guessed: `seg.advance / char_count`. For "Wi" that puts the boundary between the letters at 5 when it is at 9, and every caret, drag-selection and search highlight in the application was wrong by that much for every proportional font. `GlyphPlacement` now carries per-glyph pen offsets, computed with the same expression as the run total so the two cannot drift. The even-spacing fallback stays for fonts with no width table, which is what `advance_is_measured` has always been for. The fixture story is ADR 0029's, again. `cff_sample.otf` is a fontTools conversion of DejaVu: no subroutines, no hints, no width operands. It proved the interpreter draws the right shapes, and then four mutations of that interpreter survived because nothing in the corpus reached the code they broke — each of which produces a plausible wrong glyph from a font that parses. `cff_subrs.cff` is hand-assembled for exactly those four, and fontTools agrees with every expectation asserted against it. A fifth mutation survived a composite test that counted contours; it is killed now by one that measures where the components land. Coordinates are asserted against fontTools ground truth, not against our own output. Seven mutations, all killed. 1397 tests pass. Deferred and recorded, not claimed: CID-keyed CFF, `seac` accents, rendering outlines through the Makepad device. ADR 0031.	2026-08-18 19:44:18 +00:00
andodeki	0bef30a6d5	feat(pdf): compositing and overprint — the blend maths had no backdrop ... `transparency.rs` implemented all sixteen blend modes and unit-tested them against the specification's formulas. Nothing ever called them with a backdrop. The Makepad renderer's `SetBlendMode` pushed a `TransparencyError::Unsupported` and then painted the source colour, so a /Multiply highlight and a /Normal one produced byte-identical output and every test passed — because every test asked "was the right command issued", not "does the page look right". Overprint had no code at all. /OP, /op and /OPM were not parsed, so an overprinting object knocked out the inks under it. That is not a missing feature, it is the inverse of the instruction: on a press it is the difference between a colour and a hole. - `composite.rs`: a straight-alpha RGBA `Canvas` implementing §11.3.6's union formula, weighted by backdrop alpha so a Multiply over transparency is the source rather than black. Constant alpha and per-pixel soft masks. Transparency groups composite as a unit; knockout groups are refused by name rather than silently treated as non-knockout. - Overprint as `composite_cmyk`, separate from the RGB path rather than a flag on it: overprint is a statement about inks and RGB has none. /op defaults to /OP per table 58 — defaulting it to false makes the common `<< /OP true >>` knock out every fill. §10.7.5's "no effect on an RGB device" is asserted, so our doing nothing there is the spec rather than an omission. - `raster.rs`: a CPU rasteriser that replays a command list onto a canvas. Not on the display path, no anti-aliasing, no fonts; it exists so compositing has a verifiable output. In pdf-graphics and not pdf-makepad because a test that needs a GPU is a test that does not run. - Golden **pixels** for shading, mesh, blend and overprint pages — Phase 7's exit criterion, which the Phase 2 command-text goldens cannot meet. ASCII grids with a colour legend, quantised to quarter steps; each test asserts its exact colours before comparing, so a wrong-but-stable render cannot be blessed by an UPDATE_GOLDEN run. Six mutations, all killed, including the two that describe the old behaviour: discarding the blend result, and ignoring the overprint flag. 1364 tests pass. ADR 0030.	2026-08-18 19:25:15 +00:00
andodeki	728fbc3ad0	fix(pdf): mesh shadings — three bugs in code that had no fixture ... ADR 0028 shipped types 4-7 and said honestly that they were unproven: the uncovered lines of `shading.rs` were exactly `parse_mesh`, "the position `image.rs` was in before ADR 0016 found the JPEG decoder was a stub". Writing the fixtures found three real bugs. - Type 5 has no per-vertex flag; `/VerticesPerRow` delimits it. Reading 8 phantom bits shifted every vertex after the first, decoding plausible coordinates that were entirely wrong. - Types 6 and 7 are patches: 12 or 16 control points carrying no colour, then four corner colours. The old loop read a colour per point, consumed three times too many components, ran off the stream, and the None-on-truncation path swallowed it as "the mesh ended". - A flag-0 triangle is three vertices whose second and third flags are ignored (§8.7.4.5.5). Acting on them cleared the strip every time and produced no triangles at all. Caught in new code, before it shipped. And one omission: `color_at_point` returned None for a mesh, so a mesh that parsed perfectly still painted nothing — indistinguishable from one that failed. `MeshTriangle::color_at` now interpolates the corner colours by barycentric coordinates, None outside, because black is a colour a mesh can legitimately produce. Shared-edge patches (flags 1-3) inherit the previous patch's edge rather than being read as fresh patches, which desynchronised the rest of the stream. Five corpus fixtures, generated from named coordinates and colours so every expected value in the tests is one the generator wrote deliberately. Eight tests, five mutations, all killed. Coons flattening is still an approximation and still reports `is_approximate`. ADR 0029.	2026-08-18 19:10:49 +00:00
andodeki	a2b05c56c9	feat(makepad-table): opt-in capabilities feature, and raise the matrix_client defect ... The two caveats from the dependency investigation. ## The capabilities feature Camera and location attachments are now available behind `features = ["capabilities"]`, which pulls `nigig-uikit` and supplies `UikitAttachmentProvider`. Measured: 89 crates by default, 275 with the feature on. That cost is real and it is inherent, not packaging waste. `camera_widget` imports `send_geocode_request` and `request_map_tile` from `nigig-core`, both of which call `spawn_async` — the shared Tokio runtime — and the first makes an HTTPS call to Nominatim. A camera that geocodes needs an async runtime and an HTTP client; there is no lighter honest version. It is affordable because it is opt-in, and because any app enabling it already depends on `nigig-core`, so that app's own tree grows by nothing. Everything touching `nigig-uikit` is in one module, so the boundary is a file rather than `#[cfg]` scattered through the widget. The provider holds no widgets of its own: the host owns the `CameraWidget` already in its tree and this asks it to open, because a provider that instantiated a second camera would fight the first for the device. A second request while one is outstanding is refused rather than overwriting. The table turns that refusal into `AttachmentUnavailable`, so the user is told the camera is busy instead of watching their first request vanish. File picking is deliberately declined here — `robius-file-picker` already ships unconditionally and costs nothing, and two paths for one job is one too many. Two CI gates, both verified to fail when they should: the opt-in build must keep compiling, and the default build must pull none of `tokio`, `reqwest`, `hyper`, `clap`, `csv`, `image`, `nigig-uikit` or `nigig-core`. The second checks the resolved `cargo tree` rather than the manifest, because feature unification can switch an optional dependency on from a sibling crate. Tests 99 default, 105 with the feature. Both clippy-clean. ## The matrix_client defect Raised in REVIEWS/MATRIX_CLIENT_FEATURE_GATE.md rather than fixed. It is not my crate, nothing depends on the broken combination, and a blind fix could change behaviour someone relies on. `matrix_client` declares `native = ["dep:tokio", "dep:reqwest", "dep:rusqlite"]` but its source gates on `#[cfg(not(target_arch = "wasm32"))]`. Two switches for the same modules, so on a native target with the feature off the modules compile and their dependencies do not — 19 errors, 26 ungated uses across 7 files. There is no CI job for the crate, which is why it rotted unnoticed. The note corrects an overstatement I made while arguing for the trait hook. I said fixing this would unblock wasm. It would not: `matrix_client` already builds clean for wasm32 with `--no-default-features`, and `nigig-core` has 8 wasm errors of its own (`crate::platform::spawn` missing) that have nothing to do with it. The only broken combination is native-target-with-feature-off, which nothing builds. I also said earlier that `matrix_client` was heavy — it is a 7-dependency local crate, not matrix-sdk. That was wrong and it inflated the case for the trait hook; the note records the measured numbers instead.	2026-08-18 18:03:26 +00:00
andodeki	c1d1e67f3a	feat(pdf): shadings — the sh operator was parsed and thrown away ... ADR 0028, the first of Phase 7's eight bullets. content.rs contained `PdfOp::Shading(_name) => {}`. The operator was lexed, given its own variant, matched during interpretation, and discarded. A page whose background is a gradient rendered as nothing. Nothing caught it for the usual reason: a blank region is a legal thing for a page to contain, so "drew nothing" and "drew what was asked" are indistinguishable without an assertion naming the expected colour. The golden corpus had no shading page, so there was nothing to be wrong. Two of the three pieces already existed — function.rs evaluates the colour function and colorspace.rs converts it to RGB. What was missing was the geometry between them. Sampling rather than a gradient primitive: a PDF shading is defined by an arbitrary function, possibly a sampled table or a PostScript program, and neither reduces to a stop list without loss. A device with a native gradient can still recognise the two-stop case from the samples. "No colour here" is None, not black. Black is a colour a shading can legitimately produce, so returning it for "outside an unextended shading" would paint a rectangle the author never asked for and the caller could not tell the two apart. Types 1-5 exact. Coons and tensor patches are flattened to their corners, which loses the curvature, and is_approximate says so rather than leaving a caller to assume fidelity. An unknown type is refused by number: a mesh drawn as a flat fill is a plausible-looking wrong answer. paint_shading is a new trait method, so the compiler found every implementor. The Makepad renderer records the request in pending_shadings, mirroring pending_xobjects — it cannot resolve a /Shading resource because it does not own the page dictionary, and recording the request is what stops the operator vanishing a second time. That holds even for types we refuse, so a host can warn the user. Four mutations, all killed. The first — discarding sh again — fails three tests. Stated plainly and left unticked: the mesh path is written but NOT exercised by any real stream. shading.rs is at 68% and the uncovered part is exactly parse_mesh and triangulate. Mesh support should be treated as unproven, not working: the code runs and produces triangles, and nothing yet demonstrates they are the right triangles. That is the position image.rs was in before ADR 0016 found the JPEG decoder was a stub. The Phase 7 status line is a table from the start this time — one row per spec bullet, seven of them saying "not started". Per ADR 0021, written before the work rather than after it. pdf: 1321 passed (was 1291). pdf-ui: 1366. Coverage 87.60%, floors met.	2026-08-18 17:30:39 +00:00
andodeki	374af5ccad	feat(pdf): the five Phase 6 bullets the status line omitted ... ADR 0027. Asked whether Phase 6 was 100% complete, I checked the plan's bullets against the code instead of answering from the status line. Five were not implemented and the status line named none of them: detached/ATTACHED signatures /SubFilter hardcoded to adbe.pkcs7.detached PAdES basics ETSI.CAdES.detached was a string in a match external_signing_test.dart absent; SigningIdentity needs an in-memory key OCSP/CRL lookup CRL only; OCSP counted, never parsed Fulcio identity absent (optional in the plan) This is the second time. ADR 0021 recorded the same failure in Phase 4 and wrote the rule meant to prevent it — enumerate criteria from the plan text first, then mark each done or explicitly deferred. I wrote that rule and then produced another prose summary of what I had built. A summary written from the work cannot show what the work omitted. PAdES is a real profile, not a label. CAdES signs a set of signed attributes, one carrying the document digest, and the signature is over those attributes re-tagged as a SET (RFC 5652 5.4) rather than over the [0] IMPLICIT SEQUENCE they are carried in. Verification checks the messageDigest attribute against the document as well as verifying the attribute signature; without that, a signature over somebody else's digest would be accepted. /SubFilter now comes from the profile, so a document cannot claim CAdES while carrying plain PKCS#7. ExternalSigner is a trait: bytes in, signature out. A smartcard or KMS never hands out its key, so SigningIdentity could not represent one. SigningIdentity implements the trait rather than sitting beside it, so there is one signing path — a second path for hardware keys would be a second place the byte range could be computed differently. OCSP is decoded with the der crate already present rather than adding the ocsp crate for two fields. Revoked from any response beats Good from any other. Attached signatures are REFUSED, not deferred. Both attached profiles (adbe.pkcs7.sha1, adbe.x509.rsa_sha1) are SHA-1 based, and SHA-1 is broken for signatures. They are parsed so such documents can be read; they cannot be written, enforced by the absence of a SignatureProfile variant. Same decision as RC4 in ADR 0024. Recorded as refused rather than not-done, because "not done" invites someone to finish it. Four mutations, all killed first attempt: messageDigest not compared, CAdES verified against the wrong bytes, /SubFilter hardcoded again, OCSP revoked read as good. The status line is now the plan's own bullets in a table, one row per spec item, not prose. Two wrong status lines in the same direction is a pattern, and the fix is structural: a missing row is visible, a missing sentence is not. Four rows are left unticked — Fulcio, independent review, Acrobat interoperability, and signing a document that already has an AcroForm. qpdf accepts documents under both profiles. pdf: 1289 passed (was 1276). Coverage 87.96%.	2026-08-18 12:06:59 +00:00
andodeki	99aebc202a	fix(pdf): security review of the signing code — a forgery verified as valid ... ADR 0026. Both ADR 0024 and ADR 0025 said this code needed a security review before shipping. This is that review, done adversarially: for each way a signature could be defeated, a test that attempts it. It found a critical vulnerability in the code as shipped last turn. FINDING 1, critical, exploitable with no special access. Verification recovered the certificate and the signature by *scanning* the blob for DER-shaped bytes rather than decoding it. The signature was checked against certificates[0]; trust was checked against ANY certificate present. Two questions, two different certificates. So: the attacker signs a forgery with their own key the attacker appends the victim's trusted certificate to the blob signature_valid = true (their signature over their own content is real) chain_trusted = true (the victim's certificate is present) is_valid() = true Demonstrated before the fix, with the message "I hereby transfer everything to the attacker" verifying as valid. Fixed by decoding the ContentInfo/SignedData structure and finding the certificate the SignerInfo actually names, by issuer AND serial, then evaluating both the signature and the trust path against that one certificate. Trailing data now fails the decode instead of being ignored. The scanning functions are deleted, not left unused: dead code that once returned the wrong answer is an invitation to call it again. FINDING 2, moderate. signer_certificate() returned chain[0] unconditionally, so a chain whose first entry was not the signing key's certificate made the SignerInfo name the wrong one. Not a forgery route — the signature fails — but a UI showing "signed by <somebody trustworthy>" beside a failed check is its own kind of dangerous. Now it finds the entry whose public key matches the key doing the signing. FINDING 3, informational. digest_matches was hardcoded true under a comment claiming it was computed. Not exploitable, because is_valid() also requires signature_valid and the signature covers the bytes — but a field asserting an unperformed check is ADR 0017's pattern exactly. The four items ADR 0025 left unticked are closed: PKIX chain building, with each link's issuer signature verified. A name match alone is not a chain; anyone can put any name in a certificate. Pinning still short-circuits first. Stapled revocation from /DSS, offline only. Unknown is the default and a first-class answer: treating "no information" as "not revoked" is a claim a verifier cannot support. Signature appearances, with the claimed time labelled "Time claimed" because a self-declared /M carries no authority. One-call sign_document. Three things were wrong first: the /ByteRange placeholder was too narrow for real offsets so patching them moved every later byte; /Contents must be a hex string because a literal full of NULs needs escaping and changes length; and a signature dictionary nothing points at is invisible — the first version wrote one and the reader reported zero signatures over a correctly signed document. Four mutations, all killed — two only after strengthening the tests. My first smuggling test put the attacker's certificate first, where certificates[0] finds it anyway, so it passed with or without the issuer/serial match. Putting the TRUSTED certificate first is what distinguishes them, and writing that test is what exposed Finding 2. qpdf --check accepts the signed documents. pdf: 1276 passed. Coverage 87.98%. Left unticked, deliberately: an independent review by someone who did not write the code. This is a self-review; it found two real vulnerabilities, which is evidence the method works and not evidence that nothing remains. Also untested against Acrobat, which is stricter than the spec, and sign_document replaces rather than merges an existing AcroForm.	2026-08-18 10:39:20 +00:00
andodeki	9989043a37	ci: gate the coverage that was already measured and unenforced (Phase 0) ... Phase 0 of REVIEWS/REPO_COVERAGE_100_PLAN.md, and the reason it is Phase 0: no new tests, no new measurement, just ratchets on numbers that were already good and already decaying-capable. **spreadsheet** — `tools/test-spreadsheet-coverage.sh` has had a 96 floor for the engine and another for the UI controllers, and no CI job has ever run it. New `.forgejo/workflows/spreadsheet.yml`, two jobs: engine-coverage 98.83% of lines (floor 96) ui-controller-coverage 98.85% of lines (floor 96) Split in two because the halves cost very differently. The engine is pure Rust and finishes in about three minutes; the UI half has to build Makepad's Linux backend to link a test binary. One job would hide an engine regression behind a ten-minute build. **CAD widget layer** — `cad-widget-coverage` in nigig-build.yml, deliberately REPORT-ONLY. It sits at 13.25% of 10,637 lines with six files at exactly zero, and a floor there would read as a blessing rather than a debt. What the job buys is that the number is printed on every push instead of being rediscovered in six months. The first real input test should set a floor behind it. Also corrects the plan. It claimed the doc workspace module was ungated; it is not — nigig-build.yml has run doc-workspace-coverage since before the plan was written. I had surveyed by grepping workflow files for the word "coverage" and attributed nigig-build's coverage jobs to CAD alone. I nearly committed a duplicate workflow on the strength of it. The census table was right; the prose under it was not, and the correction is in the file. One thing checked and deliberately NOT changed: the spreadsheet script appears to skip its UI half when the native packages are absent. It does not. `makepad-native-libs.sh --check` returns 1, the script runs under `set -e`, and it aborts. What misled me was reading `$?` after piping the script into `tail` — which reports tail's status, not the script's. The same class of mistake this repository's CI comments warn about; no fix was needed and none was made. Verified by running each job's exact command line: COVERAGE_TARGET=engine ./tools/test-spreadsheet-coverage.sh rc=0 COVERAGE_TARGET=ui ./tools/test-spreadsheet-coverage.sh floors met ./tools/test-cad-widget-coverage.sh 13.25%, rc=0	2026-08-18 10:20:22 +00:00
andodeki	3928063392	ci(email): raise the domain floor; record the finance-email path ... email.yml: FLOOR 225 -> 230. The review doc records the email-to-finance sharing and notes the chat/Matrix path remains unbuilt (matrix_client has login+sync only).	2026-08-18 10:07:54 +00:00
andodeki	9a5ce9c0e6	feat(pdf): signing and verification — Valid becomes reachable, with a policy ... ADR 0025, completing Phase 6's functional core. This partially reverses ADR 0010, which refused signing and cryptographic verification outright, and it reverses only the half whose justification expired. ADR 0010 gave two reasons. The first — a parsing library has no business signing — stopped being true when Phase 4 began creating documents and Phase 5 editing them. The second is still true and is preserved intact: deciding which certificate authorities to trust is a policy decision that belongs to the host, not to a parsing library So VerificationStatus::Valid is still not reachable by default. Verification returns three independent booleans and is_valid() needs all three; the third, chain_trusted, can only become true through a caller-supplied TrustAnchors. There is no TrustAnchors::system(), no bundled root store, no Default that trusts anything. A caller with no policy is told "cryptographically intact, signed by somebody you have not said you trust" — a different fact from "forged", and a host that cannot tell them apart shows the wrong thing to a user. RSA PKCS#1 v1.5, ECDSA P-256 and Ed25519, all with SHA-256. PSS is stronger and not universally accepted by PDF verifiers, so v1.5 is what is written. Ed25519 carries an interoperability caveat in the doc comment on the variant itself, because that is where someone choosing it will read it: ISO 32000-2 does not list it and most desktop viewers will reject it. No network. Revocation is not implemented rather than smuggled in: the engine crates are CI-gated against reaching outward, and that gate is a rule about layering, not an obstacle to work around. Every test generates a real key and a real certificate at run time. Nothing asserts against a checked-in blob — a fixed expectation only proves the code still does what it did, which is the wrong question for a signature. The tampering tests assert the signature verifies FIRST, then flip a bit; without that half they could pass by never verifying anything. Four mutations, all killed. The one that matters is the first: making an empty anchor set confer trust is exactly the regression that would turn this back into the thing ADR 0010 refused, and it fails immediately. Two bugs the tests found: UTCTime cannot encode a year past 2049 (RFC 5280 4.1.2.5.1). The first fixture used a 2096 expiry and every certificate failed to encode. The certificate scanner assumed a two-byte DER length. RSA certificates are large enough to use that form, so RSA and P-256 passed while Ed25519 found no certificate at all — its certificate is small enough for the short form. A scanner tested only against the largest input fails silently on the smallest. 72 dependency packages pulled in, zero non-compliant licences, no C. Stated plainly and left unticked in the ADR: chain_trusted is anchor identity matching, not PKIX path building. Correct for certificate pinning, a false negative for a real CA hierarchy. Also outstanding: revocation, signature appearance generation, and one-call incremental signing. pdf: 1247 passed. Coverage 88.08%, floors met.	2026-08-18 09:45:01 +00:00
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	ad3fe19b90	docs(pdf): the four missing ADRs — codecs, Phase 4 completion, editing, redaction ... Ten PDF feature commits landed without an ADR, covering three whole phases. Every feature gets one; these are the four that were owed. Written against the code as it stands and re-verified by running it, not transcribed from the commit messages. 0020 CCITT, JBIG2 and JPEG 2000 — the codecs ADR 0015 refused by name 0021 Phase 4 completion — stamping, reconciliation, CFF, cmap, and the audit that corrected a false "complete" in ADR 0019 0022 Editing — content_edit, page_ops, flatten, catalog_edit 0023 Redaction and compaction, and the three reader defects they found Verified rather than assumed, on the tree at f75c1cc: ccitt 30, jbig2 29, jpx 44 unit tests; jpx_roundtrip's 7 compare against OpenJPEG-generated pixels, exactly, because a JPEG 2000 bug produces a plausible image rather than an error. Redaction re-checked with a test written from outside the module. The middle line is the one worth keeping: after redact, secret present = true <- earlier revision after compact, secret present = false public text retained = true Redaction alone leaves the secret in the bytes. That is why the two shipped together, and why the report carries earlier_revisions_retain_content. Determinism re-checked by generating the same document from four separate processes: byte-identical. The HashMap key-order defect really is fixed. This ADR contains a correction to its own first draft. 0022 initially listed text-run rewriting and annotation sub-types as deferred, on the strength of a grep for "rewrite" finding nothing. Both are implemented — the rewriter is ContentEditor::set_text, which preserves the operator kind so a ' keeps its line advance and a TJ keeps its kerning; and "callout" and "comment" are dart-pdf's names for a FreeText with a /CL and a Text annotation, not distinct /Subtype values. Checking the plan's claim against the code was right; concluding from a failed grep was not. One criterion across the four is left unticked, and it is real: the ui.rs interaction tests, blocked on the Makepad headless backend since Phase 1. 0021 also records a process note. ADR 0019's merge criteria were derived from what had been built, so all of them ticked while three unbuilt items stayed invisible. Criteria should come from the plan text first, then be marked done or explicitly deferred — a deferral stated is fine, a deferral unstated is a false claim. pdf: 1187 passed. No code changed.	2026-08-18 05:53:36 +00:00
andodeki	4a955c5c89	docs: repo-wide coverage plan — census first, then phases ... The CAD plan covered one module. This is the level above it: all 58 workspace members, 310,287 lines. The census is the point of the document: A gated 5 crates 45,190 lines B measured, not gated 2 crates 15,373 C tested, unmeasured 27 crates 225,149 D zero tests 5 crates 4,160 E templated shells 19 crates 20,415 Three things the census turned up that were not visible from inside any one crate. Tier B is free money. spreadsheet and the doc workspace both have coverage scripts with per-file floors already written, and no CI job runs either. Same for the CAD widget layer. Fifteen thousand measured lines with nothing stopping them decaying. pageflipnav is 24,117 lines with 20 tests, no coverage tooling, and no mention in any review document in this repository. It is the worst ratio here by a distance and nobody has looked at it. The nineteen "app" crates are one program. Diff any two main.rs files after normalising the name and you get a background colour and a root screen identifier. Covering them as nineteen crates is nineteen times the work for one crate of risk; the plan asks for a decision rather than quietly doing it. On the goal itself: the only honest cost estimate available is the measured one. The CAD engine's 13,752 lines took about six sessions to reach 97%, on the easiest half of one module. Tier C is 225,149 lines, so straight-line extrapolation is ~98 sessions and the extrapolation is optimistic. That is not an argument against 100% — it is an argument that sequence matters more than destination, because the first fifth of the effort can cover most of the risk if pointed at the right code. So the phases are ordered by risk, not size: gate what is measured, measure everything else, then payments and SMS before anything larger. Phase 4 (GUI) is explicitly gated on the CAD makepad-test spike, so nobody commits three months to an approach that may not work here. Also recorded: #[coverage(off)] is unstable on the pinned 1.97.1 toolchain, verified with E0658, so unreachable code cannot be annotated away. It has to be covered, moved to an excluded file, or subtracted in the open.	2026-08-17 12:44:56 +00:00
andodeki	211ce31e9f	docs(cad): a phased plan to 100% coverage, written after measuring ... Asked for a plan to 100%. Four things had to be established first, because each one changes the plan's shape, and three of them contradict what I would have assumed. **There are already 148 widget tests and they have never run.** tests/ui.rs is 1,889 lines of #[makepad_test] tests driving the real app through TestApp/Selector. Line 1 imports a crate that is not a dependency, so the file has never compiled, and no CI job names it. Same defect the spreadsheet-ui suite documents about itself; same class as the nigig-email binary that had never been built. **They compile with a one-line manifest change, and they run.** Adding makepad-test as a dev-dependency produces a binary; under xvfb-run all 148 execute in 59 seconds without hanging. **All 148 fail at a known point.** The harness's child build of the app exits non-zero before startup: code 127 with no cargo on the child PATH, code 101 after fixing that — while `cargo check -p nigig-build --bins` passes. So the blocker is in how the harness invokes the child, not in the app, and spreadsheet-ui already documents the workaround. **#[coverage(off)] is unstable on 1.97.1.** There is no way to annotate a line as legitimately unreachable, so anything genuinely uncoverable has to be covered, moved to an excluded file, or subtracted openly. The plan puts unblocking those 148 tests first, because it is the cheapest large prize and because its outcome resizes everything after it. Engine cleanup runs in parallel since it is independent. Extraction work is explicitly held until Phase 0 reports, so nobody extracts logic the app-level tests already cover. It also argues against 100% as a target for the widget half. The 148 tests are mostly wait_visible(); they will move the number a long way while proving that widgets exist. Of the four real defects this work has found, three came from reading uncovered regions and asking why they were unreachable, not from driving a percentage. The plan targets 100% of what is worth executing and names the ~48 subtracted lines.	2026-08-17 12:40:35 +00:00
andodeki	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	f1c3c18374	docs(cad): the widget layer was never unbuildable — I never tried ... Every claim I have made about this environment's limits was wrong, and it cost six sessions of work routed around an obstacle that was not there. `cargo test -p nigig-build` needs wayland, X11, GL, alsa and polkit. I turned that into "does not build here" early on, wrote it into commit messages, wrote it into TEST_BASELINE.md, wrote it into the review, and never retested it. In a plain container: bash tools/makepad-native-libs.sh --install # ~8s cargo check --locked -p nigig-build --lib # 2m52s, clean cargo test --locked -p nigig-build --lib # 5m22s, 1044 passed cargo test --locked -p nigig-build --test cad_integration # 154 passed cargo fmt -p nigig-build -- --check # clean The install script existed the whole time, written for exactly this, with a comment explaining that libasound and libpulse are needed to *link* a test binary rather than merely to check it. This file's own "Reproducing" section pointed at it. So did the review's "Required next commands". Two things follow. First, every commit I have pushed to this crate is now verified rather than CI-gated-and-hoped-for, including `refactor(cad): one screen-to-world path, not two`, whose message says plainly that its compile could not be checked locally. It compiles; the suite passes; the gates pass. Second, the honest reading of the last six sessions: I asked once whether to attempt the heavy build, got a reasonable "ship what you can actually test", and then treated that as settled fact rather than a decision worth revisiting when the cost of being wrong kept growing. An assumption made once and never retested is indistinguishable from a fact. That is precisely the criticism this work levelled at the CAD review entry, and I earned it too. The 750-test figure in this file was also stale; it is 1044 now. The host-only harness stays. It needs no apt, no root and no desktop packages, runs in ~20s warm against five minutes for the full crate, and is what the CI coverage gate uses. The full build is what to reach for when a change touches the widget layer, which the harness cannot see.	2026-08-17 11:55:25 +00:00
andodeki	189377a3a3	ci(email): build the wasm path; document the closed §8 gaps ... email.yml: install the wasm32-unknown-unknown target and check the email domain's credential-bearing wasm half (call_email_api, WasmFetchTransport, set_email_api_url) so a browser-only breakage cannot reach main unseen. The domain test floor ratchets 205 -> 210. The review doc's §8 is rewritten: the TLS handshake, the wasm build, B1 and the test/clippy baselines are now executed/measured; the only entries left are the ones that genuinely cannot run in CI (a live relay's cert, a browser's fetch), stated with their exact reasons.	2026-08-17 09:39:04 +00:00
andodeki	98460026d3	docs(cad): close the review entry that has said "unchanged" for ten tranches ... The CAD blocker in this document was recorded once, in tranche 1, and then carried forward nine more times as "**CAD** — unchanged" without anyone re-testing it. Both halves of it are false. "No benchmark output is checked in" was already false when written: BENCH_BASELINE.md sits at the repository root with a full table. "No Cargo toolchain is available in this execution environment" was true of CAD and false of the repository — this document's own "Required next commands" section praises `tools/test-rust-clean.sh`, an isolated runner that installs the pinned toolchain and deletes itself on exit, and uses it to validate the two pure Pay crates. The technique was three directories away from the problem for ten tranches. Applying it to CAD is `tools/test-cad-coverage.sh`. Both listed CAD commands are resolved: `send_sync_audit` runs on every push at 100%, and `profile_benchmarks` runs host-only under CAD_BENCH=1 and reproduces the baseline. Replaces the assertion with a table of measurements, and — the part that matters more — keeps three things explicitly *not* claimed: the widget layer still has no coverage number, the host-only harness runs the same sources but not the same target as `cargo test -p nigig-build`, and the two cost_estimator test targets still do not compile. A review that repeats a stale blocker is worse than one that says nothing, because it is what people read to decide what to work on.	2026-08-17 09:24:06 +00:00
andodeki	fc0b1f287f	ci(email): run the conversation-kit tests; mark Phase E complete ... email.yml: the domain test floor ratchets 190 -> 205, and the nigig-email job runs cargo test -p nigig-uikit --lib -- conversation so an email-driven regression in the shared kit cannot silently surface in SMS. The review doc marks E1-E6 done and records the honest correction E5 surfaced: lettre's timeout bounds only the TCP connect, not the greeting/command reads — the send path now bounds the whole operation.	2026-08-17 05:09:15 +00:00
andodeki	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	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	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	4426cd2c43	docs(map): reconcile makepad fork with upstream dev 2026-08-16 ... - Fork portallist_flow_adaptive_view at ecf5a572 is 1 ahead / 0 behind upstream dev abd70f47 (Aug 15); extra commit restores test/gltf/csg re-exports - Hard-reset nigig-dev-reexports from stale 2c5cd97 to ecf5a572 and force-pushed so both tracking branches are current - Pinned rev stays ecf5a572 in map + pdf-makepad Cargo.toml (no bump needed) - Document baseline strategy: nigig-map/makepad_map as control surface tracking upstream widgets/src/map, not enabling map feature in nigig-rider for rendering; upstream routing (map_nav/geodata/route app) stays separate from widget (valhalla vs map_nav decision) - Note periodic diff workflow for packed-vertex, LOD, dissolve, growing-archive watcher improvements	2026-08-16 21:30:47 +00:00
andodeki	7d6fc4cbbe	feat(pdf): document creation — outlines, forms, attachments, font subsetting ... Phase 4 of NIGIG_PDF_FEATURE_PARITY_PLAN.md. ADR 0019. Almost none of it existed: Outlines, PageLabels, EmbeddedFiles and ViewerPreferences appeared nowhere in the workspace, in any crate. What did exist was a builder whose central method was pub fn add_page_with_content(&mut self, _width: f64, _height: f64, ...) which accepted a page size and discarded it. Asking for 200x400 and 300x500 gave two US Letter pages, because no /MediaBox was written at all. The test asserted the output contained the string "/Type /Page", which it did. Two more defects sat in the object writer, both producing files our own parser rejects: dictionary keys were written unescaped (a key with a space reparses as "expected number"), and f64::NAN was emitted as the literal token NaN, so one non-finite value anywhere made the document unreadable. Added: outline trees with the open/closed state in the sign of /Count, /PageLabels as a number tree with real roman and A..Z/AA..ZZ numbering, named destinations, attachments with file specs, /Info, XMP, viewer preferences, page mode and layout; AcroForm creation for text, checkbox, radio, choice and signature fields with generated appearances; and TrueType subsetting - DejaVu Sans goes from 759,720 bytes to 4,348 for twelve characters. cmap is deliberately not rebuilt: the subset is embedded as a CID font with Identity-H, so the content stream addresses glyphs by id and /ToUnicode serves extraction. A cmap disagreeing with the content stream is worse than none. CFF is refused by name rather than emitting a font with no glyphs. Nine real bugs, every one found by running the output through an independent tool rather than by reading the code: 1 page size discarded reading a generated file back 2 dict keys unescaped probing the writer 3 NaN written as a keyword probing the writer 4 subset zeroed the lsb fontTools outline compare 5 hmtx indexed by new gid fontTools outline compare 6 name table format read as count BaseFont came out "Embedded" 7 add_font shifted numbers already handed out 8 trees allocated over font numbers - object 29 written twice 9 widgets missing /F Print, /P and appearance /Resources 7 and 8 are the instructive pair: every reference resolved and every object existed, each simply named the wrong thing. pypdf reported correct field values from a file PDFium rendered blank. 9 is the one only a renderer could find - /F defaults to non-printable, and a form XObject naming a font its /Resources does not declare is discarded whole. Verified by three independent implementations: fontTools (0 outline mismatches of 12 against the source font), pypdf (metadata, page sizes, outline with resolved page numbers, all five fields, attachment byte-for-byte, labels ['i','1']) and PDFium, which renders both pages correctly. cargo run -p nigig-pdf-graphics --example generate_sample regenerates the sample. Fourteen mutations. Three survived and each exposed a weak test: the key test used an attachment name (written as a string, never a key), nothing read the outline open state, and /P could not be witnessed because page_index is supplied by the reader, which already knows the page. All three now killed. pdf: 789 passed (was 730). pdf-ui: 775. Coverage 85.17%.	2026-08-16 21:02:19 +00:00
nigig-ci	c0b27d0586	feat(email): MailBackend trait and BackendKind — both backends (C1a/C1b) ... You chose to support IMAP-on-device AND a server-side proxy, user selectable. This is the seam that makes that contained rather than two parallel apps. Why it is cheaper than it sounds: wasm cannot open a raw TCP socket, so a proxy always had to exist for the browser target. The second backend was never optional -- it was implied scope nobody had named. C1a, mail_backend.rs: BackendKind { ImapSmtp, ProxyApi } with three predicates that exist so the UI cannot get them wrong: is_available_on_wasm() IMAP is raw TCP; a browser cannot open one, so the chooser must not offer a dead option stores_reusable_password() IMAP keeps a REUSABLE mailbox password on the device. For most people that is the password-reset channel for every other account they own. A revocable proxy token is strictly safer, and the chooser must say so rather than presenting a free choice summary() the honest one-liner, asserted by test to actually mention "password" / "revoke" BackendSettings is the PERSISTABLE half and carries no secret, exactly as EmailAccount does for the password (S2). BackendDraft::validate returns (settings, Secret) and reports every problem in one pass. The trait is deliberately synchronous and tiny -- kind(), is_configured(), describe(). Anything computable above the line (grouping, previews, threading) is NOT a backend concern, which is why email_store did not change at all. I/O stays in the free functions that already own the async context, so this file is host-testable with no runtime. ImapSmtpBackend exists with validation but no protocol client yet; that is C1e and nothing here claims a connection works. C1b: EmailAccount gained `backend: BackendSettings`, #[serde(default)] so existing persisted accounts still load. A test asserts the serialised account -- including the backend section -- contains neither the token nor a field named password/token. Provider defaults now fill IMAP too, so a Gmail user still fills one field. Outlook is special-cased: its IMAP host is outlook.office365.com, not imap.outlook.com, so the naive smtp->imap rewrite would produce a name that does not resolve. New gate, negative-tested both ways: stores_reusable_password() and is_available_on_wasm() must exist, and the persisted settings structs must not declare password/token/secret fields. Domain tests 99 -> 126. Test floor 95 -> 120.	2026-08-16 20:30:33 +00:00
nigig-ci	901cddc716	fix(email): abandon_send shipped as dead code; wire it and gate it (B6) ... Auditing Phase B against the tree rather than against my own notes found that abandon_send() existed in nigig-core and NOTHING called it. The user had no way to stop waiting on a hung send. I had marked B6 "partial" for the right reason -- lettre cannot cancel mid-transaction -- and missed that the part I did implement was unreachable. A control the user cannot reach is not a control. It is dead code wearing a safety label, which is worse than an acknowledged gap because it reads as done. Now wired: while a send is in flight the Send button becomes "Stop waiting". The label is deliberately not "Cancel" -- this does not stop delivery, because once DATA is accepted the message is sent whether we wait for the reply or not. It frees the UI and suppresses a result the user has stopped caring about. The 20s timeout from A6 bounds the window. New gate: abandon_send() must exist in nigig-core AND be called from the UI. The wiring is the thing checked, not the function. That gate was ALSO broken when first written -- it grepped for `abandon_send()` across src/, and the comment block explaining why the control exists mentions it by name, so unwiring the call left the gate green. Same flaw as the B5 gate in the previous commit, found the same way: delete the fix, watch the gate. Now excludes comment lines. Twice in two commits I have written a gate that its own explanatory text satisfied. Worth stating rather than quietly fixing: a gate is only evidence if you have watched it fail. Phase B verified closed: B1-B6 all done, 11 gates pass, 99 domain tests, check --all-targets clean on both crates, fmt clean.	2026-08-16 20:04:41 +00:00
nigig-ci	d889cbecd4	ci(email): gate multi-recipient send, and a gate that did not work ... Two new gates, and one of them was broken when I first wrote it. B1 gate: the send path must call email_send::parse_recipients, must NOT contain a single-Mailbox parse of the whole To field, and must add every accepted recipient. Three checks rather than one, because each failure mode is separately reachable. B5 gate: spawn_send_email must keep the SEND_IN_FLIGHT swap. THE B5 GATE DID NOT WORK AS FIRST WRITTEN. It grepped the whole file for `SEND_IN_FLIGHT.swap(true`, and the unit TESTS for the guard contain that same string -- so deleting the guard from production code left the gate green. I found it by negative-testing, which is the only reason I know. Now scoped to the text before `#[cfg(test)]`. That is worth recording rather than quietly fixing: a gate whose own test fixtures satisfy it is indistinguishable from a gate that works, and the only way to tell them apart is to break the thing on purpose. Negative tests, all confirmed firing: remove the list parse -> fires reintroduce `let to_mbox: Mailbox = ..` -> fires delete the in-flight guard -> fires (after the fix) and all 10 gates pass on the clean tree. Test floor 60 -> 95 (actual 99). Bulk page: builds through EmailSendRequest, so a partly-invalid list reports what was dropped instead of refusing everything, and requires a second tap before sending. The prompt quotes the recipient count and any duplicates or rejections, so the user knows what they are confirming. Editing the message after arming re-prompts rather than sending the old confirmation.	2026-08-16 19:51:21 +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
nigig-ci	3786e7c1cf	docs(email): threat model, and mark Phase A complete (A6) ... New crates/apps/nigig-email/THREAT_MODEL.md. Separate from the root THREAT_MODEL.md, which is Nigig-Pay's and shares no assets with this feature. Nine threats with the control named for each, so a reader can check the claim rather than take it on trust. The two that matter: T-E2 (password leaked by our own code) was LIVE, not hypothetical -- SmtpConfig derived Serialize over a plaintext String and the whole struct was POSTed on wasm. T-E7 (DoS via a hostile message) is the SMS A3 bug class. One inbound message containing emoji took down the SMS list on every frame until it was deleted; email bodies are more hostile, not less. Sections that exist specifically to avoid overclaiming: * "Residual" notes on every mitigation. Secret does not zero on drop. We trust the platform root store; no certificate pinning. Header injection is handled by lettre, NOT by us -- which means the C1d proxy backend, which does not go through lettre, must sanitise or T-E4 becomes unmitigated. * "What has not been tested": no live SMTP server has been contacted, the wasm path has never been built, and no IMAP code exists, so T-E1/T-E2 cover the SMTP direction only. * Four open risks ranked, each tied to a plan item, including two (proxy auth, keystore storage) that MUST land with C1d/C1f rather than after -- the proxy is only safer than on-device IMAP if its token is revocable and scoped. Marks A1-A6 done in the plan. Phase A is complete.	2026-08-16 19:22:40 +00:00
andodeki	cf73ef4c1d	test(pdf): assert what a file declares is delivered, and floor the coverage ... Every serious bug in this stack has had one shape: a valid, well-typed, empty-or-default value where the file plainly declared content. xobjects empty for every document; acroform() dropping every field behind an indirect reference; DCTDecode returning its own compressed bytes; a JPEG decoder that was a stub returning black. None errored, none panicked, and the tests asserted Ok, which they got. Coverage would not have caught any of them. Measured when each shipped: page.rs 92.4%, form.rs 93.6%, content.rs 89.2%, xref.rs 95.2%. The buggy lines ran; nobody checked what they produced. So: a property test that walks the raw object graph of every corpus fixture, counts what the file declares, and requires the API to deliver it - fonts, xobjects, graphics states, colour spaces, form fields, filters, MediaBox. It reimplements the resolution rule independently of page.rs on purpose; a test that asks the code under test what to expect agrees with the bug. It failed the day it was written, on a shape the corpus had never contained. Every fixture wrote /Resources inline, and all six extractors read it with dict.get_dict("Resources") - which returns None for an indirect reference and never consulted /Parent. A page with "/Resources 5 0 R", the commonest shape in real PDFs, reported no fonts, no xobjects, no graphics states and no colour spaces. Same for a page inheriting resources from its /Pages node. Empty, not wrong, so nothing failed. Fixed by resolving /Resources once in PdfPage::from_obj through a helper implementing the full inheritance rule (32000-1 Table 30), and passing the resolved dictionary down. Indirect /MediaBox entries resolve too. Six resources/ fixtures cover the shapes that were missing. Mutation-checked: reverting inheritance kills 5 tests, the sub-dict reference 3, indirect MediaBox 2, and removing the depth bound hangs. One mutation survived - a visited-set guarding a /Parent cycle, which the depth bound already handles - so it was deleted rather than left as untested defence with a reassuring comment. tools/test-pdf-coverage.sh enforces a floor instead of printing a number, with per-file floors as well as a total: image.rs could fall from 33% to 5% and move the total by under a point. All three failure modes verified to fail. It caught a bug in itself first - its ignore regex matched its own work directory and reported a confident TOTAL 0.00%. .gitattributes marks *.pdf binary. An xref entry must be exactly 20 bytes (7.5.4), so with a one-digit generation field it ends in a space, and git diff --check was reporting unfixable "trailing whitespace" on every fixture in the corpus. TEST_TARGET=pdf: 695 passed, 0 failed (was 680). Coverage 83.42%. ADR 0017 records the four mutations so they can be repeated by hand.	2026-08-16 19:04:57 +00:00
nigig-ci	cce6889d35	docs(email): C1 decided — both backends, user-selectable; Phase 0 done ... C1 was the one blocking product decision in this plan. Answer: support IMAP-on-device AND a server-side proxy, let the user pick, with a form appropriate to each. Recorded why that is cheaper than it sounds: wasm cannot open a raw TCP socket, so a proxy always had to exist for the browser target. The second backend is not new scope, it is scope that was already implied. What makes it tractable is a trait boundary rather than two parallel UIs: one `MailBackend` with two impls and a `BackendKind` discriminant on the account. Everything already built -- grouping, preview_line, the inbox list, the thread reader, unread handling -- sits ABOVE that line and consumes `Vec<EmailMessage>` without caring where it came from. That was deliberate in C2 and it is what keeps two backends contained. The two forms genuinely differ (IMAP+SMTP wants two servers, two ports, username and password; the proxy wants an HTTPS base URL and a token), so this is a backend chooser followed by the matching form, not one form with rows hidden behind a toggle. Broken into C1a-C1f, with the proxy first: it is smaller, it is the only option on wasm, and it exercises the trait boundary end to end. One thing recorded rather than glossed: offering both DOUBLES the security surface, and IMAP is the path that keeps a reusable password on the device. A revocable proxy token is strictly safer than a password that also unlocks the user's password resets. The setup UI should say which is which instead of presenting them as equivalent. Also marks Phase 0 complete -- 0.1 through 0.7, with 0.7 fixed upstream by 005bed1 (i_tree 1.0.0 -> 0.19.0, exactly the fix predicted here).	2026-08-16 18:35:39 +00:00
andodeki	63ff45149a	feat(pdf): a real JPEG decoder — the old one was a stub returning black ... Completes Phase 3 of NIGIG_PDF_FEATURE_PARITY_PLAN.md. Design and merge criteria in REVIEWS/adr/0016-pdf-image-decode-surface.md. ADR 0015 refused DCTDecode at the generic filter boundary and left the image path alone, noting JPEG "is decoded on the image path". That claim did not hold: fn decode_jpeg_data(_data, _pixels, _width, _height, _components) -> Option<()> { Some(()) } Every argument discarded. It wrote nothing and returned success. The caller allocated a zero-filled buffer, passed it in, and returned it as decoded pixels. Probing a real 8x8 JPEG through ImageInfo: decode_to_rgba -> 256 bytes, first 12: [0,0,0,255, 0,0,0,255, 0,0,0,255] Pure black at full alpha. Not an error, not None - a correctly sized, entirely fabricated image. EVERY JPEG IN EVERY PDF rendered as a black rectangle and nothing reported it. The underscore-prefixed parameters are the tell: the signature was written to silence the unused warnings that would otherwise have announced the stub. image.rs was at 14.2% line coverage, the lowest in the crate. Replaced with a real baseline decoder in pdf-graphics/src/jpeg.rs: huffman, dequantisation, IDCT, chroma upsampling, YCbCr/YCCK conversion including the Adobe APP14 transform flag. No new dependency - adding `image` or `jpeg-decoder` would pull a tree into a crate that has one, on a target the team is already fighting to cross-compile. Progressive JPEG is refused BY NAME rather than approximated; a partial implementation would reproduce exactly the defect being fixed. decode_to_rgba's Option is why the stub survived - "could not decode" and "decoded to nothing" were the same value. The decoder returns a typed JpegError so a caller learns why an image is missing. Also in this tranche, from the same plan bullets: - ImageInfo::downsample, integer-factor box filter. Refuses factor 0, and refuses data that is not raw samples rather than averaging compressed bytes as though they were pixels. - Round-trip tests for encode_flate and encode_ascii_hex over adversarial inputs: empty, single byte, all-zero, all-0xFF, random binary. THE IDCT TOOK THREE ATTEMPTS AND THE FAILURES WERE INFORMATIVE The first version, adapted from a hand-tuned integer kernel, decoded greyscale exactly (128 -> 128) while colour came out a UNIFORM 64 levels off. A constant offset across every channel is a scaling-factor mistake, not a coefficient one - guessing at coefficients would never have found it. Two rounds of guess-and-check made it worse. The fix was to stop guessing: derive ground truth from the float reference in T.81 A.3.3, then transcribe the separable form directly with a documented fixed-point scale. The cosine table is a const fn so it cannot drift from the formula beside it, and tests assert against the reference rather than our output. 4 corpus fixtures with real JPEGs (Pillow at generate time only; the .pdf files are committed so CI never needs it), 16 acceptance tests asserting PIXEL VALUES rather than buffer lengths - a length assertion would have passed against the stub. Mutation-checked: reinstating the zero buffer fails four tests. Coverage on image.rs 14.2% -> 32.9%, new jpeg.rs 82.8%, crate 83.65% -> 84.22%. TEST_TARGET=pdf 651 -> 680, TEST_TARGET=pdf-ui 696 -> 725. rustfmt and clippy -D warnings clean.	2026-08-16 18:33:56 +00:00
andodeki	fb95b25a67	fix(pdf): LZW was broken outright; refuse image codecs instead of faking them ... Phase 3 of NIGIG_PDF_FEATURE_PARITY_PLAN.md, lossless half. Design and merge criteria in REVIEWS/adr/0015-pdf-filters-and-codecs.md. LZW DID NOT WORK Fed the worked example from PDF 32000-1 section 7.4.4.2: LZW default : Err("LZW previous code out of range") decode_lzw seeded a 256-entry dictionary but set next_code = 258, because 256 and 257 are the clear and EOI codes. New entries were appended with table.push, landing at index 256 - so the counter and the real index were permanently two apart and every dictionary reference resolved to the wrong entry. Any PDF using LZW was affected, which is a whole class of older files. Also in the same area: - /EarlyChange was ignored. It selects when the code width grows; a file setting 0 decoded to GARBAGE rather than failing, which is worse. - Predictors were applied to Flate only, though /Predictor is equally legal on LZWDecode. TWO MORE BUGS FOUND WHILE IMPLEMENTING decode_stream read /Filter as a single NAME and fell through to "unsupported filter" for an array. The document layer calls decode_stream, so every chained stream in every document failed to decode - including the common [/ASCII85Decode /FlateDecode]. It now delegates to decode_stream_with_params, leaving one decoding path. decode_flate_with_predictor inflated its own input, so calling it from a chain decompressed already-decompressed bytes. Split into apply_predictor, which works on decoded data. IMAGE CODECS: REFUSED, NOT FAKED DCTDecode and JPXDecode previously returned their COMPRESSED bytes as though decoded: "DCTDecode" | "JPXDecode" | "Crypt" => data, A caller received a Vec<u8> that looked like image data, was not, and produced garbage pixels rather than an error. CCITTFaxDecode, JBIG2Decode, JPXDecode and DCTDecode now return a typed error naming the filter. image.rs still sniffs and decodes JPEG on the image path, so that route is unaffected; what stops is the generic filter claiming a success it did not achieve. /Crypt stays a pass-through, correctly - decryption already ran. Not implementing CCITT/JBIG2/JPX is a decision, not an omission: JBIG2's CVE record is why browsers sandbox it, and JPX via openjpeg would add a C dependency that breaks the Android cross-compile the team is already fighting. CCITT is the tractable one and is the recommended next step. 4 corpus fixtures, 14 acceptance tests. Mutation-checked - and one check initially misled me: removing the reserved-slot seeding did not fail the tests, because the clear-code branch re-seeds independently and every real LZW stream opens with a clear code. Removing both fails all three LZW tests. Recorded in the ADR. One pre-existing defect deliberately left: the PNG predictors do not consume the per-row filter-type byte. Fixing it risks every Flate-with-predictor document in the corpus and is not what this ADR set out to do, so it is documented rather than quietly half-fixed. TEST_TARGET=pdf 637 -> 651, TEST_TARGET=pdf-ui 682 -> 696. rustfmt and clippy -D warnings clean.	2026-08-16 18:04:07 +00:00
nigig-ci	0c14f8d848	docs(email): correct dead commit SHAs; record the SMS parity mechanism ... Three fixes to the plan, one of them a real defect in the document. 1. Every commit SHA it cited was dead. I wrote them before the final rebase, which rewrote them, so the progress log pointed at eleven references that `git cat-file -e` cannot resolve. A plan that cites nonexistent commits is worse than one that cites none. Remapped: 28d0608 -> 5a5b817 1ea9ad6 -> b91f97b 50760e0 -> 18bbb7b 169563f -> b701ce5 2. Documented HOW the SMS similarity is achieved, since "like the SMS list" is the requirement and prose does not prove it. Added a table naming the six shared components both features now consume from nigig-uikit/src/shared/conversation/ -- the preview row, its action, its props, the stack-navigation view, the message bubbles and the bottom-nav actions. Parity is structural, not cosmetic. Also recorded the consequence: those widgets are now load-bearing for two features, so an email change can regress SMS. The shared kit has no tests of its own. New item E6. And the one intentional divergence: email rows key on a normalised (lowercased) sender address, because Alerts@Bank.co.ke and alerts@bank.co.ke are one sender. 3. Updated item 0.7. The makepad `maps` break is fixed by ce0eaae, but that commit added `i_tree = "1.0.0"` to crates/apps/map and crates.io publishes only up to 0.19.0, so the workspace still does not resolve. Verified pre-existing by stashing all email changes and reproducing on a pristine tree. This remains the highest-priority blocker: while it holds, no crate in the repo can be verified on a runner.	2026-08-16 17:44:59 +00:00
andodeki	6a18886185	feat(pdf): Type 3 fonts and streaming interpretation — Phase 2 complete ... The last two items of NIGIG_PDF_FEATURE_PARITY_PLAN.md Phase 2. Design and merge criteria in REVIEWS/adr/0014-pdf-type3-fonts-and-streaming.md. TYPE 3 FONTS DREW NOTHING A Type 3 font's glyphs are not outlines - they are content streams, listed in /CharProcs and mapped to text space by /FontMatrix. Probing a document with one: fonts on page: ["T3"] T3: subtype=Type3 base=Unknown -> are the glyph procedures reachable? no CharProcs field exists -> is /FontMatrix exposed? no field exists The font was detected and then nothing could be done with it. /CharProcs and /FontMatrix appeared nowhere in the crate, so the procedures were unreachable and the text was silently invisible - a page that renders, reports no error, and is missing content. New pdf-document/src/type3.rs parses /FontMatrix, /CharProcs, /Differences, /Widths, /FontBBox and the font's own /Resources, and resolves a character code to its glyph procedure's decoded bytes. /FontMatrix is applied as written rather than assumed to be the common 0.001 scale - Type 3 fonts routinely use other matrices, which is the point of the entry. A missing /CharProcs entry is a typed error naming the glyph, not a blank. A THIRD BUG, FOUND WHILE WIRING d0/d1 The interpreter parsed both operators and discarded them: PdfOp::Type3Width(_wx, _wy) => {} PdfOp::Type3BBox(_x1, _y1, _x2, _y2) => {} They are how a Type 3 glyph declares its advance, so even a renderer that could draw the glyphs would stack them all at one point. Wiring them to the device exposed that `d1` takes SIX operands - wx wy llx lly urx ury - and the parser read four, so the "bounding box" was really the advance and the advance was lost entirely. Now `Type3BBox { wx, wy, bbox }`, reading all six. STREAMING INTERPRETATION parse_content_stream materialised every operator into a Vec before interpreting any of them: peak memory proportional to the whole content stream, on a stream walked once and discarded. Adds ContentStreamIter and interpret_streaming, with parse_content_stream reimplemented on top of the iterator so there is ONE tokeniser rather than two that can drift. Equivalence is proven, not asserted: a test compares both paths across every corpus fixture, and a streaming_interpreter fuzz target compares them over arbitrary bytes, which is where a divergence would actually hide. 4 corpus fixtures, 13 acceptance tests, 9 unit tests. Mutation-checked: reverting d0 to a no-op fails glyph_advances_reach_the_device. Phase 2 is now complete; the plan is updated with an item-by-item audit. Several entries were already done (inline images, Do, text state, shading); the plan's "biggest gap" was xref streams, closed in ADR 0013. TEST_TARGET=pdf 615 -> 637, TEST_TARGET=pdf-ui 660 -> 682. rustfmt and clippy -D warnings clean.	2026-08-16 17:28:44 +00:00
nigig-ci	b701ce5eeb	docs(email): assessment and live execution plan ... Lands in REVIEWS/ rather than the repo root, where 35 markdown files already compete for attention. Records the audit (architecture, performance, bugs, design, security, code quality) with each finding tied to evidence that was executed, not inferred, and a progress log that marks what has shipped. Two things worth reading even if you skip the rest: - I got the port-465 TLS finding WRONG on the first pass and wrote it up as critical credential exposure. Checking lettre 0.11.23's source showed relay() is implemented with the same three calls and TlsParameters::new already sets accept_invalid_certs: false and a TLS 1.2 floor. Downgraded to Medium and the error is recorded rather than quietly removed, because a document like this is worthless if you cannot tell which claims survived scrutiny. - C1 is the single blocking decision: IMAP on device vs a server-side proxy. The inbox list, thread reader and grouping are done and work; what they display is sample data until that is answered. The plan lays out the tradeoff and does not pretend it is a technical call. Also notes that origin/main does not currently resolve — the makepad bump in 86c9595 dropped the `maps` feature pageflipnav declares — which is now item 0.7 and blocks CI verification for every crate, not just this one.	2026-08-16 17:27:35 +00:00
andodeki	2faadb777f	feat(pdf): read xref streams and object streams (PDF 1.5+) ... Phase 2 of NIGIG_PDF_FEATURE_PARITY_PLAN.md, the item it calls "the biggest parse-side gap". Design and merge criteria in REVIEWS/adr/0013-pdf-xref-streams.md. The parser could not open a PDF 1.5 file. Not render it wrong - not open it: PARSE FAILED: PDF error at byte 382: expected xref keyword XRefTable::parse_section required the literal bytes `xref` at the startxref offset. A PDF 1.5+ file has an indirect object there instead - the xref stream - so the parse aborted and the entire document was unreadable. Every feature built on top of the parser (encryption, signatures, forms, structure tree, transparency) was unreachable on any file produced in the last twenty years. ObjStm, XRefStm and /Type /XRef appeared nowhere in the crate. Implemented on the read side: - Xref streams: the packed binary table, /W field widths, /Index sparse subsections, and types 0/1/2. A zero-width /W column means "use the default" (type 1) - missing that rule yields a table of all-free entries and an apparently empty document rather than an error. - Object streams: type-2 entries resolve through /ObjStm, reading the header pairs and /First. The xref's index is used but verified against the object number it claims to be, because a wrong-but-in-range index would silently return a different object. - Hybrid files: a traditional table plus /XRefStm. Both are read, with the traditional table winning on conflict, which is the point of the layout. Bounds and refusals rather than silent degradation: /W widths are clamped and every field read is checked against the decoded buffer; a truncated table is flagged, not padded with free entries; an object claiming to live inside itself is refused; a /Type that is not /XRef is named in the error. Scope note: the writer is untouched. ADR 0003 keeps appending a traditional xref section, which remains correct - the appended trailer carries /Prev to the stream, so the chain stays readable by us and by conforming readers. Also verified against the rest of Phase 2: inline images, XObject Do, shading, and the full text state (Tc/Tw/TL/Tz/Ts/Td/TD/Tm/Tf) are already implemented and tested. Type 3 fonts and the streaming interpreter remain genuine gaps, but each degrades one feature rather than the whole file. 6 corpus fixtures, 9 acceptance tests asserting real page content rather than a successful parse, and a parse_xref_stream fuzz target because the table is attacker-controlled binary. Mutation-checked: restoring the old error fails 5 of the 9. TEST_TARGET=pdf 606 -> 615, TEST_TARGET=pdf-ui 651 -> 660. rustfmt and clippy -D warnings clean.	2026-08-16 17:10:42 +00:00
andodeki	bd01604e65	docs(pdf): Phase 1 status, and stop pdf-ui failing for an environmental reason ... Phase 1 of NIGIG_PDF_FEATURE_PARITY_PLAN.md. Findings verified by running them, written up in REVIEWS/PDF_PARITY_PHASE1_STATUS.md. Three of the five exit criteria are met: the workspace is green on the new rev (pdf 606 passing), both pins are already at 5efe6e24c, and the upstream baseline is documented - libs/pdf_parse is 4,575 lines with no save/write path at all, against nigig-pdf's 25,845 lines with writing, encryption, signatures, structure tree and transparency. nigig-pdf supersedes both libs/pdf_parse and widgets/src/pdf_view.rs; nothing in either is a capability we lack. The remaining two criteria need fork work this repo cannot do. WHY THE UI SUITE CANNOT PASS YET The six #[ignore] markers were removed from pdf-makepad/tests/ui.rs and the docs now claim the suite runs without a Studio hub. The markers went but the tests did not start passing - TEST_TARGET=pdf-ui was simply red. Three layers, each found by fixing the one in front of it: 1. studio/hub/src/build_manager.rs:398 spawns the build with `sh -lc`. The -l makes it a LOGIN shell, which discards the inherited PATH and rebuilds it from /etc/profile, where ~/.cargo/bin does not appear. cargo is not found and the child exits 127 in 0.4s. This breaks any rustup-based CI, not just this sandbox. `sh -c`, or resolving cargo through the CARGO env var, would fix it. 2. Past that the build runs (88s) and the failure becomes 101. libs/makepad_test sets MAKEPAD=headless for the child, but platform/src/os/linux/windowing_backend.rs only knows X11 and Wayland - there is no headless backend and the env var is not consulted. The app selects X11, finds no display, and segfaults (139). This is the real Phase 1 fork task: "terminal/standalone mode" needs a backend, not just an env var the harness sets. 3. Under xvfb-run the app starts properly and OpenGL initialises, so the binary is fine - but the hub spawns its child outside that display. The markers are restored, with a reason pointing at the status document. A red suite everyone knows to disregard stops reporting the next real regression, which is strictly worse than an explicit skip. Also fixes a latent build break this exposed: the fork's app_main! macro expands to #[cfg(native_activity)], a cfg this crate never declares, which is a hard error under -D warnings. Declared as expected-but-unset via [lints.rust] check-cfg rather than silencing unexpected_cfgs wholesale, which would also hide our own typos. One genuine improvement on this rev: pdf-makepad now builds in release inside the workspace. That was previously blocked by a Makepad os::linux feature-unification bug. TEST_TARGET=pdf 606 passing, TEST_TARGET=pdf-ui 651 passing + 6 ignored.	2026-08-16 16:48:02 +00:00
andodeki	7fdf436510	test(pay): Phase 5 lifecycle matrix as domain tests (R2.3) ... The exit criterion names five scenarios: permission denial, cancellation, backgrounding, app restart, out-of-order callbacks. Four of the five are state questions, not hardware questions. A device adds confidence that Android really emits a given callback sequence; it cannot tell you how the domain reacts, because the state machine decides that. So the matrix runs against the real coordinator on every commit instead of when a phone is free, and a regression names the invariant it broke. 13 tests in crates/nigig-pay-domain/tests/lifecycle_matrix.rs, including the cases that only exist as races: a success arriving after a cancellation; backgrounding before a grant (must refuse) versus after one (must be preserved — the user did authorise); restart before dispatch versus after; a foreign grant; a replayed grant. Plus a clean-path test so the matrix cannot pass by refusing everything. ## A coverage hole the matrix found a_restart_after_dispatch_cannot_redispatch passed with the duplicate-dispatch budget removed. The state machine refuses Submitted -> Dispatching first, so the budget was never reached. That is good defence in depth and bad coverage — nothing proved the budget still worked. the_dispatch_budget_survives_a_state_machine_walk_back forces the intent back to Dispatching, exactly as a faulty recovery path would, leaving the budget as the only guard. It fails when the budget is removed. The forcing hook is behind a `test-hooks` feature, not #[cfg(test)]: an integration test is a separate crate and does not see cfg(test), so the method was simply missing. The isolated runner enables it explicitly, otherwise that test is silently filtered out and proves nothing. ## Verified by injection authorization gate removed -> 7 of 13 fail dispatch budget removed -> 1 fails (the new one) ## What still needs hardware That Android actually produces these sequences: permission dialogs, process-death timing, callback ordering under memory pressure. This file asserts the response is correct for each sequence; a device confirms the sequences are the real ones. Different claims, both needed. Tracked as R2.3b. ## Validation domain 148 unit + 13 matrix, fmt, clippy -D warnings, bench pass storage 46 / platform 64 / mpesa 29 / pay-ui 78 pass clippy -p nigig-pay-ui --no-deps -D warnings 0 errors	2026-08-02 09:47:52 +00:00