Integration Tests:
- Add tile_decode_integration.rs with 17 test cases
- Test simple and complex geometry decoding
- Test security limit enforcement
- Test error handling for malformed input
- Test various OSM feature types
- Test different zoom levels
- Test Unicode tag handling
Benchmarks:
- Add tile_decode_bench.rs with 6 benchmark suites
- Benchmark simple JSON decoding
- Benchmark scaling with feature count (10-1000 features)
- Benchmark MVT parsing performance
- Benchmark geometry tessellation
- Benchmark POI extraction (100 POIs)
- Benchmark label extraction (50 labels)
Fuzz Testing:
- Add fuzz testing infrastructure with cargo-fuzz
- Create 3 fuzz targets: MVT parser, Overpass parser, full pipeline
- Test parsers with random input to find crashes and edge cases
Test Runner:
- Add tools/run_map_tests.sh for easy test execution
- Support for unit, integration, bench, fuzz, and coverage tests
- CI-friendly test execution
Documentation:
- Add PHASE6_TESTING_VALIDATION.md with comprehensive guide
- Document test categories, running tests, and best practices
- Include CI workflow examples and future improvements
This completes Phase 6 of the code quality improvements.
Phase 4 of REVIEWS/DART_PDF_VS_MAKEPAD_PDF_GAP_ANALYSIS.md. The previous
commit added the interaction logic but left it an orphan: nothing routed
Makepad events into it, which is the exact defect the review names when it
says PdfPageView declares is_interactive() -> false and an empty
handle_event.
New pdf-makepad/src/page_view.rs, PdfPageWidget:
- is_interactive() returns true and handle_event routes real events.
- Step 4.1 ordering: form field events, then annotation hit testing, then
text selection. The order is enforced inside InteractionState::click so
it cannot drift between the widget and the tests.
- Step 4.2: link underlines and form widget borders are drawn, and the
cursor changes over links and text fields.
- Step 4.3: field values draw inside their widget rects, a focused field is
highlighted and shows its uncommitted buffer.
- Step 4.4: navigation is emitted as PdfPageAction to the host. The widget
never opens a URL (rule 5).
- Key focus loss commits the pending edit, so an edit is never silently
lost, while Escape still abandons it.
- Only keys the editor handles are consumed; everything else falls through
to the host instead of being swallowed.
The widget keeps no decision logic of its own: it syncs the viewport from
its on-screen rect and delegates. That keeps the part that needs a live Cx
as small as possible, because it is the part that cannot be unit tested.
Also adds Selection to interaction.rs so drag-to-select and copy read
through the Phase 5 PageText API rather than re-deriving positions.
Tests: new tests/phase4_exit_criterion.rs drives the exit criterion against
the real two-page AcroForm fixture, not synthetic dictionaries. Click a link
gives OpenUri; click a field, type, commit, and the value changes, the
appearance regenerates with the new text and the draw list shows it. It also
asserts an abandoned edit changes nothing, the checkbox uses the /On state
declared in the file, and form fields route before annotations.
Validation: TEST_TARGET=pdf-ui ./tools/test-rust-clean.sh
202 tests pass; rustfmt and clippy -D warnings clean.
Phases 4 and 5 of REVIEWS/DART_PDF_VS_MAKEPAD_PDF_GAP_ANALYSIS.md.
pdf-makepad had never compiled. It was missing eight PdfDevice methods, had
a non-exhaustive RenderCommand match, called a function through the wrong
path and held a borrow conflict. Everything previously claimed about the
Makepad rendering path was therefore unverified. It now builds and is tested
under a new pdf-ui target; the makepad build needs system GUI libraries, so
that target checks for them with pkg-config and names the missing packages
rather than failing in the linker.
Phase 4 (new pdf-makepad/src/interaction.rs):
- Viewport maps between PDF space (y up) and screen space (y down) in one
place, so hit testing and rendering cannot disagree. Inverted /Rect
corners are normalised.
- Click routing follows the review order: form fields first, then annotation
hit testing. A field overlapping a link takes the click.
- Clicking a link emits PdfAction::OpenUri to the host. The viewer never
opens a URL itself (rule 5).
- Text editing is buffered: typing changes a working copy and only Enter,
Tab or blur commits it, so Escape abandons an edit with the document
untouched. Caret arithmetic is in characters, not bytes, so a non-ASCII
value cannot panic.
- Checkboxes toggle on click using the state the widget declares.
- Hover reporting for cursor feedback; read-only fields are not targets.
- render_fields() returns placement data so a focused field shows its
uncommitted buffer while drawing stays trivial.
Phase 5:
- New pdf-graphics/src/cid.rs: composite font support. Codespace ranges give
variable-width code decoding (the previous CMap was u8 to char, so every
two-byte CID font decoded as garbage), plus cidchar/cidrange, ToUnicode
bfchar/bfrange including array destinations and multi-character values
such as ligatures, /W and /DW CID widths, and Identity-H/V. An unknown
predefined CMap returns None instead of silently substituting Identity;
unmapped codes decode to U+FFFD instead of vanishing. Width ranges are
bounded so a malformed /W cannot exhaust memory.
- New pdf-graphics/src/text.rs: the PageText extraction API. Segments carry
origin, advance and font size; find() returns matches with rectangles;
selection spans runs and works in either drag direction. Runs sharing a
baseline stay on one line even with mixed font sizes.
- renderer.rs: the two unexplained `fs * 0.75` constants are replaced by a
named DEFAULT_EM_TO_CAP_RATIO used only as a fallback, with the real
ascent preferred when the descriptor supplies one. Ts (text rise) was
stored but never applied, so superscripts drew on the baseline.
Also fixes clippy across pdf-makepad, which had never been linted.
Validation:
TEST_TARGET=pdf ./tools/test-rust-clean.sh (167 tests)
TEST_TARGET=pdf-ui ./tools/test-rust-clean.sh (191 tests)
Both rustfmt and clippy -D warnings clean.
b7d23f2 committed the golden expectations and the AcroForm fixture but not
the interpreter changes they exercise: a stash/pop during a pre-commit
check left the modified sources unstaged, so the tree on main referenced
paint_x_object, PdfOp::InlineImage, RenderCommand::SetDash and
format_commands without defining any of them.
This adds the sources described in that commit message, with no change to
its intent:
- PdfDevice::paint_x_object, set_dash and set_miter_limit now emit commands.
- BI/ID/EI parsed into PdfOp::InlineImage with the real dictionary and bytes.
- ImageInfo::from_inline expands abbreviated inline-image keys.
- cm parsed before m, and rg/RG before r/R, so neither is shadowed.
- Missing /Filter means raw data rather than FlateDecode.
- format_commands / format_command for deterministic golden output.
- .gitignore exceptions so the fixtures are tracked.
Validation: TEST_TARGET=pdf ./tools/test-rust-clean.sh
132 tests pass; rustfmt and clippy -D warnings clean.
Phase 2 of REVIEWS/DART_PDF_VS_MAKEPAD_PDF_GAP_ANALYSIS.md. The exit
criterion is that a real content stream renders through the device path and
is verifiable; previously nothing could fail, so nothing was proven.
Step 2.3, the operations the review calls "the lies", all reached a dead end
in the interpreter. Each now reaches the device:
- Inline images: BI/ID/EI were emitted as two empty marker ops and the
payload was discarded, so an inline image could never be drawn. They are
now parsed into a single InlineImage op carrying the dictionary and bytes.
Abbreviated keys (/W /H /BPC /CS /F) and colour-space and filter
abbreviations are expanded. The EI scan requires delimiters on both sides
so binary data containing the bytes "EI" does not truncate the image, and
an unterminated image yields no image rather than invented pixels.
- Do (XObject) was an empty match arm. The interpreter cannot resolve a
resource name, so it now reports it through PdfDevice::paint_x_object and
the device performs the lookup.
- set_dash and set_miter_limit only mutated interpreter-local state and
emitted no command, so dashes never reached any renderer.
Three further defects surfaced while reviewing the generated goldens, all
of which silently corrupted output rather than failing:
- `cm` was never parsed at all. The single-character `m` arm matched first
and consumed it as a moveto, so every CTM change in every document was
lost and content drew at the wrong position. Two-character operators are
now tested before their one-character prefixes.
- `rg` and `RG` were shadowed by the `r` and `R` arms, so an RGB fill was
read as a single-component grey: `0.1 0.2 0.3 rg` produced 0.1 0.1 0.1.
- ImageInfo defaulted a missing /Filter to FlateDecode. An absent /Filter
means the data is stored raw, so every uncompressed image was undecodable.
Testing: adds format_commands(), a deterministic one-line-per-command text
form of a RenderCommand list, and eight golden files covering vector paths,
text, kerning and spacing, dash and stroke parameters, fill rules, inline
images, XObjects and colour operators. Floats are fixed-precision and
negative zero is normalised so no spurious diffs appear. UPDATE_GOLDEN=1
regenerates them for review.
Also fixes .gitignore: blanket *.txt and *.pdf rules were silently excluding
the golden expectations and the AcroForm fixture added in the previous
commit, which would have left both test suites unable to run on a fresh
clone.
Validation: TEST_TARGET=pdf ./tools/test-rust-clean.sh
132 tests pass; rustfmt and clippy -D warnings clean.
Phase 3 of REVIEWS/DART_PDF_VS_MAKEPAD_PDF_GAP_ANALYSIS.md. The previous
form.rs was keyed by name strings, had no inheritance and no way to edit
anything, so the review item "delete PdfFormFilling and replace it with a
DocumentFormEditor that returns real errors" had no implementation.
Step 3.1 object identity:
- Fields are keyed by ObjRef, not by name. Widget-to-field identity is
preserved across parse, and a field defined as a direct object is skipped
rather than given an invented identity.
- PdfDocument::page_index_of() maps a page reference back to its index and
returns None for a stranger.
Step 3.2 annotations:
- PdfDocument::page_annotations() reads /Annots and records the real page on
every annotation; the hardcoded page_index: None is gone.
- Annotations expose a typed action() (OpenUri / GoToNamed / GoToPage). The
document reports intent only and never opens anything itself (rule 5).
- contains_point() normalises the rectangle: a PDF /Rect is any two opposite
corners, so an inverted one previously never hit-tested.
Step 3.3 form model:
- /Parent chain inheritance for FT, Ff, V, DV, DA, MaxLen and Opt, with the
child key overriding the ancestor.
- /Ff resolved into concrete types: checkbox vs radio vs pushbutton, combo
vs list box.
- DocumentFormEditor takes typed edits and returns FormError. Read-only,
type-mismatched, over-MaxLen, non-option and unknown-state edits are all
refused, and a refused edit leaves the field untouched and not dirty.
- MaxLen counts characters, not bytes.
- Checking a box uses the on state the widget declares, not an assumed /Yes.
- The field tree walk is depth-bounded so a cyclic /Kids cannot recurse
until the stack dies.
Step 3.4 appearance generation (new appearance.rs):
- Document-level, not widget code. Generates text, multiline, choice and
checkbox appearances as Form XObjects with correct /BBox and /Length.
- /DA parsing resolves font, size and colour, converting gray and CMYK.
- Auto-size (0 Tf) resolves to a size that fits the widget.
- Values are escaped, so a parenthesis in a value cannot terminate the
string and corrupt the stream; a single-line value cannot break out via
newlines; content is clipped to the widget box.
- Unsupported kinds (pushbutton, signature) and degenerate rectangles return
AppearanceError instead of a blank stream that would erase the field.
Wiring: PdfDocument::acroform() parses the form against the real page tree,
which is what makes widget page resolution meaningful.
Tests: adds tests/acroform.pdf, a hand-written two-page AcroForm fixture
with an inherited field, a checkbox with /On and /Off states and a URI link,
all on page index 1 so any code defaulting to page 0 fails. 12 integration
tests assert the Phase 3 exit criterion against that real parsed file.
Validation: TEST_TARGET=pdf ./tools/test-rust-clean.sh
118 tests pass; rustfmt and clippy -D warnings clean.
Replace hardcoded fill/stroke/POI/label draw loops with a configurable
render graph that executes passes in z_order from lowest to highest.
New render_graph module:
- PassType enum: Background, Fill, Stroke, POI, Label, Selection, Debug
- PassConfig with z_order, enabled, min_zoom, max_zoom
- RenderGraph with enable/disable/zoom_range/set_z_order/execution_plan
- PassStats recording for per-frame diagnostics
- 16 unit tests covering ordering, gating, and stats
NigigMapView integration:
- draw_walk delegates to render_graph.should_execute() for each pass
- Records PassStats after each pass (tiles_drawn, features_drawn)
- Public API: render_graph(), enable_pass(), disable_pass(),
set_pass_zoom_range() for downstream configuration
Benefits:
- Easy insertion of new passes (3D buildings, terrain, traffic)
- Per-pass enable/disable at runtime
- Per-pass zoom culling (replaces hardcoded view_zoom >= 13.0)
- Debug visualization (show only one pass)
- Execution plan inspection for diagnostics
Phase 3 (secure repository) is now complete.
3.3 retention and erasure — new retention.rs:
- Data minimisation and financial record keeping pull opposite ways, so the
policy states both: evidence expires 90 days after settlement, settled
records are held for a five-year floor, never-dispatched intents expire in
30 days, and anything awaiting reconciliation is never swept. Deleting an
unresolved payment would destroy the only record of money that may have
left the account.
- erase_payment_on_request honours a user deletion request without
destroying the accounting record: it normally erases the parsed copy of
the user's inbox and retains the financial record, erases fully only past
the floor, and refuses outright while in flight. Refusals carry a reason.
3.4 backup, restore and corruption reporting:
- integrity_check and foreign_key_check surface damage as StorageError
::Corrupt rather than letting a damaged ledger be trusted.
- backup_to uses SQLite's online backup API instead of copying the file, so
a concurrent writer cannot produce a torn copy, and refuses to back up a
database that fails its integrity check.
- restore_from validates the candidate in a separate connection before
touching the live database, so a failed restore cannot destroy a working
ledger. Tested with both a missing and a corrupt backup.
Storage tests 20 -> 36.
Phase 4 (blocking I/O) — 4.1 to 4.3 implemented:
- 4.1 expenses/mod.rs called reload(), a full parse of the store from disk,
as the first statement of draw_walk; scrolling re-read the whole ledger
every frame. Now loaded once, with invalidate()/reload_if_stale() on the
event path.
- 4.2 the SMS scan was a wall-clock check inside draw_walk, so the branch ran
every frame and the scan landed mid-paint. Now a cx.start_interval timer on
the event path, with the interval named rather than inline.
- 4.3 update_summary walked the filtered list and rewrote labels every frame.
Now invalidated only in refresh_filtered and applied in handle_event, fully
out of the draw path rather than guarded inside it.
- 4.4 already satisfied by PaymentStorageWorker since tranche 1.
Supporting domain work: view_state.rs adds PeriodSummary and
TransactionsViewState so the summary arithmetic is testable outside a widget.
The tests immediately pin two things the inlined code got wrong: totals now
report overflow instead of wrapping, and a net outflow returns None rather
than a clamped zero. Money gains Default (zero).
Domain tests 66 -> 75.
Validated on rustc 1.97.1: domain and storage each pass test --locked, fmt,
clippy -D warnings and cargo-deny; storage also --features sqlcipher; domain
benches run; 7 M-Pesa store tests pass; 49 manifests parse.
NOT VERIFIED: the nigig-pay widget edits for 4.1-4.3 are not compiled here.
That crate needs the git-hosted Makepad fork. cargo check -p nigig-pay on a
full developer checkout is required before Phase 4 can be called complete.
Phase 0 of REVIEWS/DART_PDF_VS_MAKEPAD_PDF_GAP_ANALYSIS.md requires a clean
baseline before any feature work. The four pdf crates did not compile at all,
so every test claim about them was unverified.
Compile fixes:
- decode_lzw/decode_run_length returned Vec<u8> where callers expected
PdfResult, so decode_stream_with_params did not type-check.
- interpret_ops called a current_state_mut() method that PdfDevice does not
have; colour-space tracking now goes through explicit device hooks.
- image.rs used miniz_oxide without depending on it; PNG inflate now reuses
the COS crate through a new pdf_cos::filter::inflate_zlib.
Correctness fixes found while making the code build:
- LZW and RunLength silently truncated malformed input and indexed unchecked;
both now return typed errors (rule 6: no silent degradation).
- Tw/Tc/Tz were parsed and thrown away, and the " operator dropped its word
and character spacing, so every advance after them drifted.
- TJ attached each kern to the preceding string instead of the following one
and discarded a trailing kern entirely.
- GlyphWidths::width() fell back to default_width for out-of-range codes;
PDF 32000-1 9.6.2.1 requires /MissingWidth.
- Text advances silently substituted a guessed font_size * 0.6 when no width
table was present; ShowTextWithMetrics now carries advance_is_measured so
callers can distinguish a measurement from an unknown.
Tests: two tests had never compiled and were wrong once they ran (WinAnsi
0x99 is U+2122 not U+2019; q/cm/l/Q records four commands not three). The
document test asserted only that the writer emits a %PDF header; replaced
with real page-tree, out-of-range and malformed-input coverage.
Adds a pdf target to tools/test-rust-clean.sh that tests the three
UI-independent crates bottom-up under rustfmt and clippy -D warnings.
Validation: TEST_TARGET=pdf ./tools/test-rust-clean.sh
72 tests pass; rustfmt and clippy -D warnings clean.
Phase 4 optimization: only recompute visible_tile_keys() when viewport
actually changes, avoiding unnecessary work every frame.
- TileScheduler::update_visible now takes &mut ViewportState
- Checks viewport.dirty before recomputing visible tiles
- Clears dirty flag after computation
- Skips recomputation when viewport unchanged (common case during idle)
- Adds tests for dirty flag behavior
This reduces per-frame CPU work when the map is stationary or during
non-interactive rendering.
Phase 1 Step 4 of map rewrite plan: extract label scratch buffers and
placement methods into dedicated LabelState struct.
- Create label_state.rs with LabelState owning all scratch buffers
- Move place_and_draw_labels, collect_label_candidates, build_label_placement
from view.rs to LabelState
- Wire LabelState into NigigMapView as #[rust] field
- Reduce view.rs from 1497 to 1100 lines (-27%)
- Label logic now isolated and testable independently
- Preserves all existing behavior and performance characteristics
This completes Phase 1 of the map rewrite plan. All subsystem extractions
are now complete: ViewportState, TileCache, TileScheduler, RenderPass,
and LabelState.
Phase 1 of NIGIG_PAY_CONSOLIDATED_REVIEW.md is now closed.
1.1 build governance:
- Declare license = "MIT" on both payment crates. cargo-deny correctly
reported them as unlicensed, which would block any distribution review.
- Version-pin the nigig-pay-domain path dependency; a bare path dependency
is a wildcard requirement.
1.2 quality gates:
- Add deny.toml and a CI job running cargo-deny over both payment crates.
Advisories, bans, licences and sources all pass. The config bans the
makepad-* crates outright and restricts sources to crates.io.
1.3 canonical ownership (ADR 0002):
- Record the domain/storage/platform/UI layering and its one-way deps.
- The review's A5 "fork farm" table is stale: one copy each of parser.rs,
store.rs, pending_store.rs and pay_flow_handler.rs, not three.
- Fix B1: store.rs parsed category, sub_category, status and confidence
from disk then overwrote them with Default::default(), losing every user
categorisation on reload. Persistence also wrote display names, which are
not reversible, so this adds stable storage tokens with a legacy-display
fallback so existing rows still load.
- Fix B4: clean/restore mapped '|' to '~' and reversed every '~', so
"JOHN~DOE" loaded as "JOHN|DOE". Replaced with bijective backslash
escaping covering the separator, newlines and carriage returns.
- B6 (f64 money) deliberately deferred to Phase 6: it is a type change that
ripples into UI consumers.
These were previously recorded as untestable because nigig-core is not a
workspace member. That was wrong: the three files involved need only serde,
chrono, one log! macro and one app_data_dir() helper. The new
tools/test-mpesa-store-clean.sh supplies those shims in a throwaway crate
and runs 9 tests, two of which reproduced the defects before the fix.
1.4 boundary: enforced twice, by a CI manifest/import check and by the
deny.toml ban list.
1.5 shims: three re-exports in nigig-pay/src/lib.rs had zero callers and are
deleted. The remaining four carry a caller count and a named migration
target so they have a deletion plan rather than an open-ended lifetime.
1.6 scope (ADR 0001): accepted that Nigig Pay is a read-only tracker and
launcher, not a payment processor, until an authorised provider integration
exists. This is the decision the review required before further UI work.
SECURITY: a live Cloudflare API token was found committed in README.md,
present since the initial commit and pushed to a public remote. Removed and
recorded as R-SEC-001 in PAYMENT_RISK_REGISTER.md. Redaction does not revoke
it; it remains in history and must be rotated by the owner.
Validated on rustc 1.97.1: domain and storage each pass test --locked, fmt,
clippy -D warnings and cargo-deny; storage also passes --features sqlcipher;
domain benches run; 9 M-Pesa store tests pass. 49 manifests parse.
