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.
Replace outdated custom MVT→Overpass→TileBuffers pipeline with latest
makepad build_tile_buffers_from_mvt function that includes:
- Baked fill triangulations (pre-tessellated geometry from MVT)
- Baked painter-cascade faces (z14 tiles carry solved height buckets)
- 3D building support with real heights from detail archive
- Bridge corridor detection and elevation solving
- Road core geometry for 2.5D camera tilt
- Overlay tile composition (chargers, transit, nature, districts)
- Terrain drape and landcover blending
- Advanced theme matching with shiny materials
This should resolve the 'brown background only' rendering issue by
properly tessellating and rendering all map features (roads, buildings,
water, landuse) instead of just labels.
Changes:
- Added tile_makepad.rs (12,422 lines from makepad dev branch)
- Updated tile_disk.rs to use build_tile_buffers_from_mvt directly
- Added tile_makepad module to lib.rs
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
Five of the six scripts under tools/ were committed mode 100644. Every
one of them is invoked with a leading ./ from pay-domain.yml or
pdf.yml, so those steps could only ever fail:
./tools/makepad-native-libs.sh: Permission denied
./tools/test-mpesa-store-clean.sh: Permission denied
Both are real failures from run 349, the first time a runner existed to
execute pay-domain.yml at all. They fail at the job's first substantive
step, so payment-ui-tests did no work whatsoever and
isolated-payment-tests skipped its last seven gates -- including the
dependency audit, the "payment crates must not depend on Makepad"
check, and the mock-gateway-in-release guard.
The mode is a property of the index, so a local chmod that is never
staged does not fix it. Marked all five executable with
`git update-index --chmod=+x` and added a hygiene gate that fails if any
tracked tools/*.sh is not 100755.
repo-hygiene.yml is the right home: it has no path filter, needs no
toolchain, and already exists to validate CI configuration itself.
Gate negative-tested: reverting one script to 100644 fails it with
"tools/makepad-native-libs.sh is mode 100644, expected 100755";
restoring the bit passes.
Option A, as requested. Plus an audit of the whole review against the code.
## The default flips
nigig-pay-ui default = [] (leaf stays off; see below)
nigig-pay default = ["demo"]
nigig-mpesa default = ["demo"]
pageflipnav default = ["native", "demo"]
`cargo run -p pageflipnav` now drives *334#, shows the PIN field and
dispatches. That is the app's primary function and it works out of the box.
A release build opts out:
cargo build -p pageflipnav --no-default-features --features native
This was not one line. A first attempt flipped the four `default =` lines
and the opt-out still leaked: the app crates depended on nigig-pay-ui with
its own defaults, so `--no-default-features` on pageflipnav was silently
re-enabled one level down. Verified with a compile probe rather than
cargo tree, which truncates. The inner deps now carry
`default-features = false`, and the probe confirms both directions:
default -> demo ON, --no-default-features -> demo OFF.
ADR 0007's Play-policy note is untouched. The containment requirement of
review item 0.1 is not dropped — the flag exists, CI exercises both
directions, and a shipped build still cannot dispatch. What changed is
which way it points by default, so development and device testing are not
fighting it.
CI guards inverted to match: they now assert automation is on by default
*and* that the packaging opt-out still works. check-no-pin-capture.sh now
probes the packaging build, since the default legitimately captures a PIN.
## REVIEWS/IMPLEMENTATION_AUDIT.md
Every phase checked against the code, not against the tranche notes. Where
they disagreed the code won. Summary: phases 0-5 and 7 done bar 5.2 and
Keystore provisioning; phase 6 substantially done with the thread_local
session ownership outstanding; phase 8 partly.
## B7 found live while auditing
Month navigation had never been examined. Both copies of the transactions
widget still stepped months with Duration::days(31) and years with
Duration::days(366). Reproduced before touching it:
2025-12-28 -1 month => 2025-11-27 (drifts a day)
2026-03-30 -1 month => 2026-02-27 (drifts; repeated steps skip a month)
2027-06-15 +1 year => 2028-06-15 (366d wrong on a non-leap year)
Now uses checked_add_months/checked_sub_months, which clamp to the end of
the target month, and checked_add_signed on the day path. An
unrepresentable date leaves the view where it was.
Neither claimed done nor flagged open — simply never looked at. That is the
argument for auditing code rather than notes.
## Validation
domain 137 / storage 41 / platform 64 / mpesa 29 / pay-ui 61 pass
cargo check: pay-ui, pay, mpesa, core (default and opt-out) pass
demo-on-by-default probe, both directions pass
no-PIN-capture guard against the packaging build pass
Unrelated and still blocking a full APK: nigig-map fails to compile on
clean HEAD (12 errors, no field center_lat on ViewportState).
Asked in review: "does the latest code have the PIN input field?" It did.
## Hiding was weaker than it looked
pin_input, form_pin and the reveal toggle were all present, and the default
build hid them at runtime in on_after_new. Three problems:
1. The DSL declared the control visible and Rust hid it afterwards, so
anything re-applying the UI definition — a hot reload, a re-instantiated
sheet — brought it back. Defect B11 already names this class.
2. Hidden is not absent. The TextInput stayed in the widget tree, and a
hidden input can still be focused or filled programmatically.
3. form_pin was compiled into every build, so any path reaching it could
populate it.
Item 0.3 asks for removal, not concealment.
## The field no longer exists without `demo`
form_pin, pin_visible, the eye-toggle handler, the text-input handler, both
PIN checks, the request construction and try_build_ussd_request are all
#[cfg(feature = "demo")]. A default build has no field to write to.
The DSL now declares visible: false on the PIN input and its reveal button,
so hidden is the default state rather than a runtime correction; demo
unhides them on init. That closes the reload path in (1).
One runtime call remains as belt-and-braces: if a hot-reloaded definition
surfaces the input, the non-demo arm wipes what was typed. It has no
form_pin to clear, because there isn't one.
## Proving absence rather than asserting it
A grep for form_pin proves nothing — it passes just as happily against a
field still present behind a runtime if. tools/check-no-pin-capture.sh is a
compile probe: it references the field outside any cfg block and requires
the default build to fail with "no field `form_pin`" while demo succeeds.
The script restores the file on every exit path.
Verified both directions: passes on current code, exits 1 when the field is
re-exposed un-gated.
## Validation
cargo check -p {nigig-pay-ui,nigig-pay,nigig-mpesa,nigig-core} pass
cargo check -p {nigig-pay,nigig-mpesa} --features demo pass
nigig-pay-ui: cargo test --lib pass (61)
domain / storage / platform / mpesa harness pass
no-PIN-capture guard: verified to fail on a re-exposed field pass
## What 0.3 still leaves open
The Java-side KEY_PIN scrubbing was already done, so 0.3 is complete for the
default product. A demo build still captures a PIN by design — that path
exists for authorised device testing and is covered by the unresolved
Play-policy decision in ADR 0007. If that decision goes against the USSD
rail, the demo path and its PIN capture are deleted with it.
Phase 6 and item 7.3 of REVIEWS/NIGIG_PAY_CONSOLIDATED_REVIEW.md.
Rationale in REVIEWS/adr/0008.
## The rule, made into a type
Phase 6's exit criterion is that the UI cannot call a transaction
successful without trusted confirmation, or failed without known
rejection. The old code violated it structurally: the sheet set its
status inline in about a dozen places, each from whatever local signal
was nearest, so there was nowhere to put the rule.
The worst instance is U4. A payment whose confirmation SMS had not
arrived in five minutes was marked Failed, the sheet closed, and the user
saw "✗ Payment failed" — with a retry button — while the money was gone.
payment_view_state.rs makes the rule a type. PaymentPresentation has no
Success variant reachable from untrusted evidence and no Failed variant
reachable from a missing SMS. VerificationExpired, Unknown and
UnknownNeedsReconciliation all present as PendingConfirmation: not
success, not failure, and no retry offered. Two defence-in-depth rows:
Confirmed without a provider reference reads as unknown rather than
settled, and an unclassified failure is not evidence nothing was sent.
6.2: ConfirmationSummary makes every required term a mandatory field, so
a confirmation missing the fee or total is not constructible. It is built
from the same quote that gets dispatched. material_digest() binds consent
to the exact terms shown and is re-checked on OK — if anything material
moved in between, the authorisation is void and the screen is shown
again. The modal previously showed only recipient and amount.
6.3: must_stay_open() keeps pending payments visible, and the status line
begins with the exact required wording, asserted by a test.
6.5: evidence rows are exposed and labelled untrusted; pending payments
offer receipt, problem-reporting and data-deletion actions.
## Three defects found by writing the tests
1. Bulk quotes silently under-charged. compute_bulk_costs used filter_map
over the fee lookup, so a contact outside the tariff was dropped from
the fee total and the user was quoted less than they would pay.
2. The batch total could overflow — .sum() panics in debug, wraps in
release. A wrapped total is a quote for the wrong amount.
3. The cost preview showed unknown fees as free via unwrap_or(0).
All three are B5/B6 territory. Item 3 is B5 resurfacing in the preview
path after tranche 1 fixed it in the dispatch path: fixing a defect at one
call site is not the same as fixing the defect.
## A pre-existing failing test, diagnosed rather than deleted
money::tests::ksh_rounds_to_nearest_cent asserted format_ksh(1.005) ==
"1.01" and had been failing on every run — confirmed pre-existing by
stashing this work and re-running clean.
The expectation is impossible, not the formatter wrong: 1.005 has no
binary representation, the nearest f64 is 1.00499999999999989..., so the
correctly rounded result is 1.00. This is defect B6 at its smallest. The
test now says so, with a companion showing Money handling it exactly.
Deleting it would have hidden a live argument for finishing B6.
## 7.3 UI tests in CI
The NIGIG_TEST_PAY gate was already gone; what blocked CI was the Makepad
link step. tools/makepad-native-libs.sh listed the libraries needed to
compile but not libasound2-dev, libpulse-dev and libssl-dev, which are
needed to link a test binary — cargo check succeeds and then
"unable to find library -lasound" appears much later. The helper now
installs and checks them, and a payment-ui-tests job runs the UI tests
plus cargo check on nigig-pay-ui, nigig-pay and nigig-mpesa.
## Validation
domain : 95 tests --locked, fmt, clippy -D warnings, bench pass
storage : 36 + 41 sqlcipher --locked, fmt, clippy pass
platform: 56 + 64 ussd --locked, fmt, clippy, mock guard pass
nigig-pay-ui: cargo test --lib pass (61)
nigig-pay-ui / nigig-pay / nigig-mpesa: cargo check pass
Domain 75 -> 95 tests. UI 55 -> 61, with the long-standing failure fixed.
Every new CI step was run locally before commit.
## Not claimed complete
Phase 6 wiring is partial. PaymentViewState exists and is tested, and the
confirmation path uses ConfirmationSummary, but the sheet's remaining
status strings are still set inline and the thread_local PayFlowHandler
still exists. The type makes that migration mechanical; it does not
perform it. 6.6 bulk stays demo-gated pending ADR 0007's unresolved 5.2.
Phase 5 of REVIEWS/NIGIG_PAY_CONSOLIDATED_REVIEW.md. Design and the one
item engineering cannot close are in REVIEWS/adr/0007.
## Phase 4 is now verified, not just written
Tranche 5 implemented the draw_walk fixes and said plainly that the
nigig-pay widget edits were unbuilt. That blocker was environmental:
installing the packages tools/makepad-native-libs.sh already lists makes
the UI graph compile. Both commands the status doc listed as required
now pass:
cargo check -p nigig-pay pass
cargo check -p nigig-pay-ui pass
No code changed for this; the claim is now evidence rather than assertion.
## Phase 5: new nigig-pay-platform crate
ADR 0002 reserved this crate and marked it "not yet created".
5.1 One crate owns the seam. It is the only crate in the payment stack
that may name a platform SDK. CI enforces both directions: payment crates
may not import Makepad, and domain/storage may not import jni or the
robius platform crates.
5.3 Correlation is mandatory and single-flight. SessionRegistry admits an
event as Accepted, Duplicate or Ignored; PlatformEvent cannot be built
without a CorrelationId. Closed session ids are retired permanently, so
an abandoned session's confirmation cannot settle the payment that
replaced it. There is a test named after exactly that scenario.
2.8/5.3 Progress decides retry safety, not error kind. classify_failure
takes the failure and the DispatchProgress reached before it, and
progress is the authority. The same TemporarilyUnavailable is safely
retryable before the dial and ambiguous once the menu is being driven —
the distinction the old code could not make, which is defect B3's
mechanism. A property test asserts across the whole failure space that
nothing which may have reached the provider authorises a fresh attempt.
5.4 Fakes cannot ship. MockGateway is cfg-gated, is a compile_error! in a
release build unless allow-mock-in-release is named explicitly, and
stamps every session id with MOCK-. CI asserts the release build fails.
5.5 No unsafe, no PIN. The crate is #![forbid(unsafe_code)] so the JNI
surface stays in robius-ussd. UssdGateway is !Send/!Sync by construction,
making the main-thread requirement a compile error. The adapter leaves
the pin field empty and a test asserts it.
5.6 The web claim is withdrawn. No browser API can drive USSD and a
Daraja credential must never reach a browser, so WebGateway refuses every
call and maps to Fatal — "never sent" — which owes no reconciliation.
7.5 Adversarial SMS corpus. StrictMpesaSms is the payment-boundary
reader, deliberately separate from nigig-core's permissive tracker parser
(ADR 0007 explains why this is not the duplication ADR 0002 forbids). It
requires an exact 10-char code, exact sender-ID match so MPESA-REFUNDS
and FAKE-MPESA are refused, rejects fractional shillings instead of
rounding, and caps body length. Corpus covers spoofing, forged code
shapes, out-of-range amounts, unicode and NUL injection, and replay. The
closing test asserts the honest limit: a well-crafted forgery is still
only evidence, because the output type has no settled state to reach.
## 5.2 is not done and is not closeable here
The AccessibilityService Play-policy review is a business decision. ADR
0007 records it as blocking, states the termination exposure, and names
what must happen before the rail is enabled. USSD dispatch stays behind
the default-off demo feature. If the review fails, ADR 0001's
tracker/launcher position applies and only the dispatch adapter is lost.
## Validation
platform: 56 tests, 64 with --features ussd, fmt, clippy -D warnings
(both feature sets), cargo-deny, mock-in-release guard
asserted to fail pass
domain: 75 tests --locked pass
storage: 36 + 41 tests --locked, incl. sqlcipher pass
nigig-pay, nigig-pay-ui: cargo check pass
cargo-deny reports advisories/bans/licenses/sources ok. The isolated
runner gained a `platform` target and it runs in CI on every push that
touches the crate.
Phase 6 of REVIEWS/DART_PDF_VS_MAKEPAD_PDF_GAP_ANALYSIS.md: "Real PDFs. Real
regressions. No test theater."
Step 6.1, corpus. 27 fixtures across basic, fonts, forms, annotations,
images, edge and malformed, in the layout the review specifies. They are
produced by tests/corpus/generate.py rather than committed as opaque blobs,
because a corpus you cannot read is a corpus you cannot trust; CI regenerates
them and fails if they differ. Hand-rolled rather than library-produced,
since fixtures for a parser must contain constructs a library refuses to
emit.
Step 6.2, corpus tests (pdf-document/tests/corpus.rs, 27 tests). Text,
vectors, Flate, multipage, CID fonts, every form field type, inherited field
keys, link actions, hidden annotations, XObjects, inline images with
embedded EI bytes, rotation, crop boxes, nested CTMs and content arrays.
Step 6.3, robustness (pdf-document/tests/robustness.rs, 3 tests) plus five
cargo-fuzz targets. cargo-fuzz needs nightly and libFuzzer so it cannot gate
a stable CI run; the harness covers the same ground deterministically by
mutating the real corpus with a fixed-seed PRNG, so a failure is reproducible
from the seed rather than only from a saved artefact. The fuzz targets remain
the deeper coverage-guided search and run on a schedule.
Three crashes on untrusted input, all found by this work and all previously
reachable from a malformed file:
- collect_pages_ref recursed forever on a /Kids cycle. Stack overflow aborts
the process; it cannot be caught. Now tracks visited nodes and bounds depth.
- PdfDocument::resolve and the COS lexer recursed once per nesting level, so
a file of 5000 open brackets overflowed the stack. Both are now bounded.
- decode_85_group multiplied an accumulator that a malformed group can
overflow, and subtracted below zero on a digit outside the valid range.
Both panic in a debug build. Now saturating.
Step 6.4, CI (.forgejo/workflows/pdf.yml). An engine job that runs the
corpus and robustness suites under rustfmt and clippy -D warnings; a separate
makepad-integration job so a missing system library is not reported as a PDF
regression; and a scheduled fuzz job. The engine job also enforces the two
architectural rules mechanically rather than in prose: no Makepad dependency
or import in the engine crates, and no process or URL launching anywhere in
them.
Also fixes .gitignore: the blanket *.pdf rule silently excluded all 27
fixtures, which would have left CI unable to run them on a fresh clone.
Validation:
TEST_TARGET=pdf ./tools/test-rust-clean.sh (233 tests)
TEST_TARGET=pdf-ui ./tools/test-rust-clean.sh (268 tests)
Both rustfmt and clippy -D warnings clean; all five fuzz targets compile.
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.
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.
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 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.
No new phase was started. This finishes every item tranches 1-2 left
partial, so Phases 1, 3, 7 and 8 are complete for the two pure crates or
explicitly blocked on crates that cannot be built here.
Phase 1 (build governance):
- Check in Cargo.lock for both payment crates and build with --locked.
.gitignore excluded them, which would have made item 1.1 a false claim.
- tools/test-rust-clean.sh now reads the channel from rust-toolchain.toml
instead of a hardcoded default that had already drifted, and gates on
fmt, clippy -D warnings, the sqlcipher feature and the benchmarks.
- CI enforces lockfile presence/freshness and a Makepad-boundary check that
inspects manifests and use/extern lines rather than prose (1.4).
Building on the declared 1.97.1 toolchain surfaced five lints 1.85 missed;
all are fixed. The two Default impls are annotated rather than derived
because each encodes a security or state-machine decision.
Phase 3 (secure repository):
- 3.1 encryption at rest: new encryption.rs and an opt-in sqlcipher feature.
PRAGMA key is applied first and verified by a forced read, so a wrong key
fails as KeyRejected rather than as corruption. DatabaseKey redacts its
Debug and zeroes on drop. No key is ever derived or persisted here, and
there is no unencrypted fallback. A test asserts the recipient MSISDN is
absent from the raw database bytes.
- 3.5: preferences.rs replaces the ANDROID_DATA marker file whose existence
was the value; every field fails safe.
- 3.6: redact.rs. PaymentIntent's derived Debug leaked a customer MSISDN
into any log line; it now masks phone, name and ids, with a regression
test. StorageError no longer prints a full intent id.
Phase 7:
- 7.7 benchmarks over money, validation, fees, the intent lifecycle and
evidence handling, using the stable harness so they run on the pinned
toolchain. BASELINE.md records measured output.
- 7.1/7.2/7.3 verified already satisfied; the review text is stale.
Phase 8:
- 8.1 simulator.rs with a scripted gateway and biometric, no clock or I/O.
- 8.2 property tests over every permutation of a representative event set:
no ordering dispatches twice, untrusted events never settle a payment,
ambiguity never becomes failure, Confirmed is terminal, duplicate
confirmations are idempotent, and replayed SMS cannot fake a conflict.
Validated on rustc 1.97.1 outside the incomplete workspace graph:
domain 66 tests, storage 20 tests, storage+sqlcipher 25 tests, fmt clean,
clippy -D warnings clean on both crates and both feature sets, benches run.
49 manifests parse; git diff --check clean.
Still unclaimed and blocked on uncompilable crates: UI wiring, Keystore key
provisioning, Phase 4 draw_walk I/O, Phase 5 adapters and legal review, the
Java PIN scrub, and B1/B4/B6 in nigig-core.
