nigig-org/REVIEWS/adr/0035-p2p-intel-four-limits.md
andodeki 8bf62e2644
Some checks failed
doc-engine / engine (push) Has been cancelled
doc-engine / coverage (push) Has been cancelled
doc-engine / consumer (push) Has been cancelled
email / gates (push) Has been cancelled
email / email-domain (push) Has been cancelled
email / nigig-email (push) Has been cancelled
email / supply-chain (push) Has been cancelled
nigig-build (CAD) / supply-chain (push) Has been cancelled
nigig-build (CAD) / cad-module (push) Has been cancelled
nigig-build (CAD) / full-crate-check (push) Has been cancelled
nigig-build (CAD) / cad-engine-coverage (push) Has been cancelled
nigig-build (CAD) / doc-workspace-coverage (push) Has been cancelled
nigig-build (CAD) / cad-widget-coverage (push) Has been cancelled
nigig-map / test (push) Has been cancelled
repo hygiene / hygiene (push) Has been cancelled
sms / gates (push) Has been cancelled
sms / robius-sms (push) Has been cancelled
sms / android (push) Has been cancelled
sms / nigig-sms (push) Has been cancelled
sms / supply-chain (push) Has been cancelled
spreadsheet / engine-coverage (push) Has been cancelled
spreadsheet / ui-controller-coverage (push) Has been cancelled
p2p-intel / engine (push) Has been cancelled
p2p-intel / coverage (push) Has been cancelled
p2p-intel / makepad-app (push) Has been cancelled
p2p-intel / exchange-tab (push) Has been cancelled
feat(p2p-intel): the exchange tab becomes spread intelligence, with rails, drift capture, and three of four limits closed
The exchange tab in nigig-mpesa and nigig-pay was a swap mock-up -- Sell/Buy
cards showing hardcoded ETH and AAVE at $38,409.24, wired to nothing -- above
six rate labels, each built from the *first* cached advert for its exchange:

    Binance P2P: USDT BUY @ 129.92 KES  min 1000 max 200000  via BANK

That is a price, not an opportunity. It never compared the two sides of the
book, said nothing about whether the counterparty could be dealt with, and
ignored what it costs to move the money. Against the live KES book it would
have shown a 134.60 advert from a merchant with three completed trades.

Both pages now run the p2p-intel analyzer over the adverts the app already
caches. No new endpoint, no new traffic, api.rs untouched -- the data was
always there, nothing was being asked of it. The two files are
byte-identical and CI now fails if they drift apart, which they already had
once: data.rs differs between the apps by 188 lines of tests one copy has
and the other does not.

**Rails, because a spread alone is a lie.** 29 bps on a 10,000 KES trade is
29 KES of gross margin; M-Pesa Send Money costs 55 KES a leg, 110 round
trip. The trade is deeply negative and the spread says nothing about it.
rail.rs prices every route and ranks by what survives. M-Pesa fees come from
the published Safaricom bands in robius_ussd::mpesa_bands -- real tariff
rows, not estimates, and flat rather than percentage, which is exactly why
the same spread is ruinous at 500 KES and fine at 200,000. Both legs are
charged. An amount outside the tariff reports OutOfRange and an
unconfigured bank reports Unknown; neither reports zero, because zero is a
claim and it is the wrong one. Bank tariffs ship unconfigured for the same
reason: every bank differs and there is no table to default to.

**API drift capture.** These endpoints are internal and undocumented. When
one changes the symptom is an empty panel -- indistinguishable from a quiet
market -- and the response that broke it is gone by the time anyone looks.
Every parse failure is now recorded with the payload excerpt that caused it
and copyable as a plain-text report. Deduplicated, because a 30-second poll
against a changed endpoint fails 120 times an hour and 120 identical rows is
a log nobody reads; the excerpt is excluded from the equality check, since
two responses differing only in advert ids are the same drift. The exported
header states that it carries response excerpts only and never a request,
credential or account number -- the type can only be constructed from a
response body, so that is enforced rather than promised. Cutting the excerpt
is done on character boundaries: Binance really returns names like
BennyBoss and a byte slice would panic mid-character.

The repo's other clipboard code (nigig-build's crdt_widget.rs) answers a
Hit::TextCopy, which is the query-driven path the platform uses for Ctrl+C
on a focused widget. Right for a text editor, wrong for a button exporting a
report the user never selected, so this uses cx.copy_to_clipboard and
confirms in the UI -- a copy button with no feedback is one people press
three times.

On the four limits, the honest scoreboard is two closed, one usefully
sidestepped, one open. ADR 0035 records why, because rounding all four up to
"addressed" would have been the easy write-up and the wrong one.

The poll timer and the host clock are closed outright. cx.start_interval
delivers the tick as an ordinary UI-thread event, so there is no runtime and
nothing to join at shutdown, and the interval reads through
effective_poll_seconds so the fifteen-second floor still applies -- a config
file cannot be used to hammer a rate-limited endpoint, which a test pins by
name. Staleness is now shown, because a price from four minutes ago is not a
price. One ambiguity is recorded rather than hidden: last_scan_ms == 0 is the
"never scanned" sentinel, so a scan whose timestamp genuinely is 0 reads as
never. That only happens when the host clock is broken, which is exactly
when the UI should not claim the data is current.

The headless-backend limit is sidestepped, and the distinction matters
enough to write down. Xvfb is a real X11 server that draws into memory, so
the app gets the display it insists on and tools/test-p2p-app-smoke.sh runs
the real binary end to end. That earned its place immediately: it caught
`Row = <View> { ... }`, which is not valid in this fork's script language
and which cargo build is entirely silent about, because script_mod! is
parsed at *runtime* -- a broken widget tree compiles perfectly and then
renders nothing. The gate greps for [E] in the log for that reason; the
process exits 0. I verified it fails by reintroducing the bad syntax. What
it still cannot do is drive widgets: makepad_test's Selector::id(..).click()
needs the harness to own the event loop, so the six #[ignore]d interaction
tests stay ignored.

OS notifications stay open. There is no robius-notification crate beside
robius-sms and robius-ussd, and Makepad exposes none on any target; building
one means NotificationCompat on Android, UNUserNotification on iOS and a
D-Bus call on Linux. What shipped is the seam plus one implementation named
UnavailableNotifications -- named for what it is, because a DefaultNotifier
that silently discarded every alert would read like a working feature at
every call site. is_delivering() returns false and the UI shows the reason,
so the user is told that alerts stop with the window instead of assuming
they are covered. A no-op reporting success is the exact "declared versus
delivered" failure ADR 0017 exists to prevent.

223 tests, coverage 97.03% with fifteen per-file floors -- seven of them new
and all measured, after I noticed an earlier edit had silently failed to
apply and the new files were being counted in the total but gated by
nothing. CI gains the Xvfb smoke job and an exchange-tab job that builds
both host apps and diffs their pages.
2026-09-01 16:07:43 +00:00

5 KiB

ADR 0035 — The four p2p-intel limits: two closed, one sidestepped, one open

Status: accepted Date: 2026-09-01

Context

The p2p-intel README named four limits rather than glossing them:

  1. In-app alerts need the app open — no OS notification.
  2. Scans are manual; poll_interval_seconds is validated but unused.
  3. Timestamps are zero, pending a host clock.
  4. The Makepad widget is compiled but never executed.

This records what happened to each, because two of them turned out to be cheap, one had a route nobody had tried, and one is genuinely somebody else's work. Rounding all four up to "addressed" would have been the easy write-up and the wrong one.

Decision

2 and 3 are closed outright

The poll timer is cx.start_interval, which Makepad delivers as an ordinary event on the UI thread. No runtime, no thread, nothing to join at shutdown. The interval reads through Config::effective_poll_seconds, so the fifteen-second floor still applies and a config file cannot be used to hammer a rate-limited endpoint — pinned by a_too_fast_poll_cannot_be_used_to_bypass_the_rate_limit.

The clock is SystemTime::now() in the app and offline_store::offline_now_ms() in the exchange pages. Staleness is shown in the header, because a price from four minutes ago is not a price.

One deliberate ambiguity is recorded rather than hidden: last_scan_ms == 0 is the "never scanned" sentinel, so a scan whose timestamp genuinely is 0 reads as never. That only happens when the host clock is broken, which is exactly when the UI should not claim the data is current. a_scan_at_the_unix_epoch_still_reads_as_never pins it as a decision.

4 is sidestepped, and the distinction matters

The blocker is real and unchanged: windowing_backend.rs in the Makepad fork knows only X11 and Wayland, defaults to X11, and dies with no display. Six ui.rs tests across this repo are #[ignore]d for it.

Xvfb is a real X11 server that draws into memory. So the app gets the display it insists on, and tools/test-p2p-app-smoke.sh runs the real binary, parses the real widget tree, creates a real GL context, and renders.

This is worth having and is not what it is often mistaken for:

  • It can catch a malformed script_mod! block, a missing widget id, a shader that will not compile, and a startup panic. It earned its place the day it was written by catching Row = <View> { … }, which is not valid in this fork's script language. cargo build is silent about it, because the block is parsed at runtime — a broken widget tree compiles perfectly and then renders nothing. The smoke test greps for [E] in the log precisely because a non-zero exit code would not have appeared.
  • It cannot drive widgets. makepad_test's Selector::id(..).click() needs the harness to own the event loop. That is fork work and it is not done, so the #[ignore]d interaction tests stay ignored.

The gate was verified to fail: reintroducing the bad syntax made it exit 1 with the parser error, and reverting made it pass.

1 is open, and now says so at runtime

There is no OS notification backend in this repository. No robius-notification crate exists beside robius-sms, robius-contacts and robius-ussd, and Makepad's platform layer exposes none on any target. Building one means NotificationCompat plus a channel on Android, UNUserNotification on iOS, and a D-Bus call to org.freedesktop.Notifications on Linux. That is a robius-sized crate.

What was built is the seam: NotificationSink, with UnavailableNotifications as the only implementation. It is named for what it is, because a DefaultNotifier that silently discarded every alert would read like a working feature at every call site.

The part that earns its place is is_delivering() and unavailable_reason(). The dashboard renders the reason, so the user is told that alerts stop when the window closes rather than assuming they are covered and missing a spread. A no-op that reports success is exactly the "declared versus delivered" failure ADR 0017 exists to prevent; this reports its own absence.

Consequences

  • Adding a real notification backend touches one impl and no alert logic. the_seam_is_object_safe_so_a_backend_can_be_swapped_in fails at compile time if that stops being true.
  • CI gains two jobs: the Xvfb smoke test, and a build of nigig-mpesa and nigig-pay with a check that their exchange pages have not drifted apart.
  • The honest scoreboard is two closed, one usefully sidestepped, one open with a seam and a runtime disclosure. The README says the same.

Alternatives rejected

Shipping a no-op notifier that returns true. It would have let the README claim four of four. It would also mean a user who closed the window believed they were still being alerted. The whole point of this tool is that its numbers can be trusted.

Claiming Xvfb closes the headless-backend gap. It closes the "never executed" half. Saying it closed the interaction half would misrepresent what the six ignored tests need, and the next person would waste a day finding out.