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.
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.
Implements the review phases left open by the previous tranche in the two
pure crates, per REVIEWS/NIGIG_PAY_CONSOLIDATED_REVIEW.md.
nigig-pay-domain:
- evidence.rs: ObservedEvidence/EvidenceLog make SMS and USSD output an
untrusted observation that can raise a reconciliation flag but can never
settle a payment (0.4, B2). Replays are de-duplicated by a deterministic
body digest; only the digest is retained, never the raw body (0.6).
- reconciliation.rs: explicit queue with typed reasons and attributed
resolutions (2.8, 6.3). SafeToRecreate never mutates or re-dispatches the
original intent. The user-facing message says "do not retry" and never
"failed" (U4).
- coordinator.rs: operation_id -> intent index so an unmatched callback
changes nothing (2.7); hard duplicate-dispatch guard where only a
never-started dispatch refunds the budget (2.9, B3);
expire_local_confirmation_window replaces Submitted -> Failed on timeout
and cannot disturb a settled payment.
nigig-pay-storage:
- schema migration 2 adds payment_intent_evidence with a unique digest
constraint and a durable payment_reconciliation_queue (3.2, 3.3).
- legacy import now durably queues unprovable records.
- fault-injection tests for unwritable path, corrupt file, unusable worker
path and audit-insert rollback (3.4); restart-equivalence tests (8.2).
Both crates deny unwrap/expect/panic/indexing outside tests (7.8). That
ratchet caught PaymentStorageWorker::spawn aborting on thread-spawn
failure; it is now fallible and returns StorageError.
Validated in an isolated toolchain over copies of the two pure crates:
cargo fmt --all --check clean, cargo clippy --workspace --all-targets
-D warnings clean, 43 domain tests and 17 storage tests passing, and all
49 workspace manifests parse. The full workspace is not asserted buildable
here: external Makepad/Robius sibling path dependencies are absent, so the
Makepad UI wiring, SQLCipher/Keystore encryption and the Java PIN scrub
remain explicitly unclaimed.
