b92659fe92218c74c8e541aab83a46835fa94950
93 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Kgothatso Ngako
|
b92659fe92 |
docs: record Phase 4 of the profiles pull, the migration to version 20
759199f2 landed as
|
||
|
Kgothatso Ngako
|
ae8cde889e |
docs: record Phase 3 of the profiles pull, and the one resolution
Four commits landed as |
||
|
Kgothatso Ngako
|
d6d87ed208 |
docs: record Phase 0 and Phase 2 of the profiles pull
Phase 0 needed one thing this plan did not predict in detail: the worktree's library checkout sat at 01489b8, an older master whose object store did not even hold the pinned 84cc44c, while its nested chain was already at the pinned four. One fetch from the main checkout's submodule and a detached checkout made the tree clean. The rewrite was rebuilt from the fork point and produced the same forty-nine rewritten commits as the dry run, hash for hash: filter-branch with the normaliser is deterministic for the same input, so the dry run's numbers are the run's numbers rather than an estimate of them. Pushing the removal to origin/mantra is the user's step and is recorded as still open. Phase 2 landed a5ff2641 and 3008137e as |
||
|
Kgothatso Ngako
|
d957ee8de5 |
feat(identity): a seed's key is a credential, and the seed is the wallet attached to it
Phase 1 of docs/multiple-profiles.md. No library change: the file format, the encrypted writer and the manager all exist, and the app already wrote the credentials file from the seed writer -- to delete a public entry a seed superseded. This changes what it writes there, and what the listing believes. Until now a seed's nostr key was never in nostr-credentials.dat. It was derived from the words at listing, to know which npub to show, and from the running node at activation, to know which key to sign with -- so a seed-backed profile existed only as a derivation, and the app had to start a Lightning node to find out who it was. Both sign-in plans made "one key, one file" an invariant, and it was the wrong one: it said a wallet is a profile. Now the credentials file is the list of profiles and a seed is a wallet attached to the entry its key derives. writeMnemonic writes two things, credential first: a Secret for the derived key under its x-only pubkey, replacing a public entry where there is one, and then the seed. Credential first because a crash between the two leaves a bare-key profile the phrase completes, which is a valid thing to hold and says the model out loud -- the profile exists, then a wallet is attached to it. The one refusal it drops is the phrase of a key held as a bare secret, SeedAlreadyExists under the nsec plan: the device did not have that wallet, so this is the profile acquiring the wallet that derives it. The entry stays a secret for the same key, the id becomes the wallet's, and the bare key's preference files go with the old id -- the profile's preferences are the wallet's now, fresh, which is right since there is a new secret to back up. The same seed twice is still refused, by wallet id, and is the only way a profile with a wallet attached is offered its phrase again. SeedCredentials.reconcile is the repair for every seed already on a device, beside migrateFromNostrKeys in listIdentities and shaped like it: a named, idempotent write, one file write for however many seeds are missing, nothing at all on a device with none or one already repaired. A failed write is a result, not a throw, and carries the map that was read: the listing goes on with it and merge derives the key of a seed that has no credential, so a seed this could not repair is still listed. A failed write must never hide a wallet. The plan had the repair running before the credentials file was read; it runs after, and hands the listing what it returns, so the file is decrypted once -- the doc now says so. StoredIdentity.merge inverts: the credentials are the list, and each seed is attached to the Secret its key derives -- listed once, as Mnemonic under the wallet's id, carrying the credential's key -- where before the seeds were the list and a secret for a seed's key was listed twice under two ids. Mnemonic gains privateKey. A Public for a seed's key is skipped with a log line, since the next repair upgrades it; a seed with no credential is listed by derivation, with a log line. IdentityKind.Mnemonic's doc changes to what the kind now means: the name records the attachment, not the source. setActiveWallet takes the StoredIdentity.Mnemonic and builds the identity from the credential's key, with the node's derivation as a cross-check -- a check(), because a node disagreeing with the credentials file is the one corruption worth refusing to run under, and it cannot fail for a file the repair wrote. The node still starts for a profile with a wallet attached: not for the key any more, but for what startNewBusiness does besides -- metadata, preferences, last-used build, and on Android the channel watcher a restored Phoenix phrase may need. Making it lazy is now one branch and is named as its own decision. forgetNostrCredential refuses a second thing: a key a seed derives. The seed would derive it again and the next repair would write it back, so a forget that succeeded would undo itself. NotACredential becomes WalletAttached in the writer's result and ForgetIdentity's outcome, since that is now the only reason a signing profile cannot be forgotten -- a key not in the file at all can, since the repair, only be a seed's key the repair could not write. The two sign-in docs' tables each gain a row pointing here for the invariant this supersedes. Tests: SeedCredentialsJvmTest against a device from before -- seed.dat written directly, no credentials -- writes exactly what is missing in one write; a device already repaired is not written to again, checked by the file's bytes since a rewrite would carry a fresh iv; a public entry for a seed's key is upgraded; bare keys are untouched; and a write that fails, with the key store locked, is reported with the map that was read and the wallet is still listed. IdentityWriterJvmTest drives the callback-shaped seed writer through a CompletableDeferred on a real Main dispatcher, since it reports after a real one-second delay: a phrase writes both files, its nsec and npub are then duplicates and its forget is WalletAttached; the same phrase twice is refused; the phrase of a bare key attaches, under the wallet's id, with the bare key's preferences gone; the phrase of a key held read-only attaches and the entry becomes a secret. StoredIdentityJvmTest lists a secret-plus-seed once with the credential's key, a seed without a credential by derivation, and a public entry for a seed's key as the wallet only. Not under test: the activation's cross-check, because a PhoenixBusiness cannot be built without a node. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Pulled-From: curated/curated@3008137e3f |
||
|
Kgothatso Ngako
|
44324c902b |
docs: plan several profiles on a device, starting from what a profile is
The switch already exists as a state transition -- switchToWallet, resetToSelector, a null identity sent to startup with popUpTo(0), every collector a child of collectLatest -- and is reachable from nowhere: Landing shows only when the device holds no identity, so a second can never be added, and the profile tab's "change account" is a pending route. This plan builds the two entrances and fixes what the transition gets wrong. Before either, it changes one rule the two sign-in plans share. A seed's nostr key is not written to the credentials file; it is derived from the words at listing and from the running node at activation, and the writers keep one key out of two files. That puts the wallet where the profile should be: the list is a merge of two files with opposite ideas of what a row is, and the node has to run for a profile to know its own key. Phase 1 makes a profile a credential and a seed a wallet attached to one -- the credential written when the seed is, repaired into the file for every seed already on the device, merge inverted to list credentials and attach seeds by pubkey, the identity's key read from the credential with the node's as a cross-check, and a second refusal on forget for a key a seed derives. The node still starts for a profile with a wallet attached, for the channel watcher rather than for the key; making it lazy is now one branch and is named as its own decision. The other decision is that a switch is a restart of the signed-in graph, not a swap under it: every route carries the key it was pushed for. That settles the switcher as a pushed screen behind one tap, the previous node stopped, the last-used profile as the one that opens on launch, and a profile added from inside switched to. Nine phases: the credential; the switch, with the relay observer that never cancelled its predecessor and the node that kept running; the startup precedence, which put "show me the list" above "open this one" and never saved a default; the switcher, showing the nostr profile rather than "Default name" and labelling a row with a wallet attached; the add rows, where a profile created from inside is a bare key and not a second wallet, and "end this" -- which wipes every profile's database -- goes; an owner on the two fetch queues and an inbox sweep on activation, because a gift wrap fetched under the other profile's key is stored and never opened again; the exits; a round trip; rollout, with the one downgrade that lists a seed's profile twice. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Pulled-From: curated/curated@a5ff264164 |
||
|
Kgothatso Ngako
|
5f2414dcb5 |
docs: plan the second pull from the fork, with its dry run already done
The upstream moved the day the first pull landed: ten commits on curated/curated, 86cb876b..29027f2b, that let a device hold several profiles and move between them, and with them the fork's first schema migration, version 20. The first plan named them the next pull and said the schema change earned a decision of its own. This is that plan, and unlike the first it was written after the dry run rather than before it, so its numbers are measured: rewrite from the fork point in 103 s, ten commits replayed onto |
||
|
Kgothatso Ngako
|
776455ecb0 |
refactor(groups): take out the group's nostr identity and the curated lists, and keep broadcast unreachable
Lines B, D and H of docs/curated-to-mantra.md, pulled from the Curated fork this
morning, go again this afternoon: the group's nostr profile (kind 0), its four relay
lists (NIP-65, NIP-17, NIP-50, NIP-51), its posts (kind 1), the curated schemas it
publishes (31889), the suggestions it reads (31888) and the entries it accepts
(31890). Eighty-two files, twelve screens, the `nostr/curated/` package, the readings
(`GroupNostrProfile`, `GroupRelayList`, `GroupPost`, `GroupCuratedSchema`,
`GroupCuratedEntry`, `CuratedSuggestion`), the `applyInnerEvent` arms for all eight
kinds, the `ChatRepository` and `NostrRepository` reads that fed them, 226 strings,
and every test that came with them. A translation collective has no list to curate
and no reason to describe itself under a key no member controls, and five rows
saying so on every group's screen were five rows about somebody else's product.
**The pasted-event proposal goes with them, because it was theirs.**
`GroupEventProposal.ACCEPTED_KINDS` was exactly kinds 0, 1, the four relay-list
kinds and 31889 -- "the kinds the group's screen has a place for", in its own words --
so with the five rows gone it would have refused every paste. Keeping it as a
generic "sign any event" was considered and rejected: the screen's whole argument
was that a member goes to the row to see the event landed, and there is no row. The
`ProposedEvent` summaries for the same kinds go too; the one test of its pre-existing
fallback ("Event of kind N") is kept, as the only line of `ProposedEventTest` that
was about code this repository had before the pull.
**Broadcast stays, and nothing opens it.** `BroadcastGroupSignedEventScreen`, its
route, view model, state and `BroadcastButton` are kept, on the decision that a way
to send a group-signed event to relays is worth having against the day something
wires a button in -- the button's only call sites were the five removed screens.
Two things had to change for it to compile against a tree with no relay lists:
`defaultRelaysFor(kind)` is now the app's own publish set for every kind, since the
General list it preferred, the Blocked list it filtered by and the schema relays it
widened to no longer exist; and `relayUrlOrNull` moves into the view model's
companion from the deleted `GroupRelaySet`, unchanged. The hint on the screen says
"the relays this app publishes to" rather than "where the group has said it lives".
Its two tests are rewritten around the new seed: the screen test answers for the
first two seeded relays and counts the rest as asked, and empties the list by hand
to see the empty state, since the seed always has something in it. Deleting
broadcast outright -- the tidier tree -- was the recommendation and was declined.
**Every pre-existing file is back at its pre-pull content plus the kept lines'
hunks, and nothing else.** Eleven files -- `ChatMessage.kt`, `NostrEventDao.kt`,
`NostrEvent.kt`, `LocalChatRoom.kt`, `Member.kt`, `ProposedEvent.kt`,
`ChatTranscript.kt`, `ProfileAvatar.kt`, the group screen's view model and state,
and `m3-title-case.py` -- were touched by no kept commit and are restored from
|
||
|
Kgothatso Ngako
|
0fa807a6fb |
docs: record what the pull built, and the seven places it chose differently
Phases 0 to 5 of docs/curated-to-mantra.md are landed on this branch: thirty
commits pulled from curated/curated, each carrying a `Pulled-From` trailer
naming the commit it came from, on top of Phase 0's two native ones. The plan
is kept as written and its status line now says what happened, with the table
the house keeps for a plan that has been built -- what it said against what
it turned out to be -- and the README's sentence about it follows.
**Seven differences, and none of them are corrections to the plan's
decisions.** The three decisions -- drop the brand line but keep its
reasoning, keep Mantra's sections by dropping 808a3459, take the curated
lists -- all held, and the measured predictions about them (the screen's
order, the tests, the audit) came true to the number. What changed was
mechanics: the phased replay is a cherry-pick per commit rather than a rebase
of each cut, because a recreated merge cannot reach a parent that was replayed
in an earlier phase; the library pin follows the commit rather than staying
put, because the compiler said the nsec line does not build against 84cc44c;
the join files are taken from upstream's own merges rather than unioned, for
three reasons that each took a failed attempt to learn; and the exactness
residual is seventeen files rather than sixteen plus a logo, because Mantra's
own side had moved too. Each is written in the table with the reasoning
beside it, so the next pull starts from what happened.
**The numbers are per phase, so a regression later can be placed.** jvmTest
736, 864, 905, 1,007, 1,039 and testDebugUnitTest 403, 486, 495, 530, 530
across Phases 0 to 5; both compilers clean and every m3 audit budget met at
each; the final jvmTest executed rather than restored from the build cache,
1,039 tests in 46 seconds of test time, 0 failures. The plan's own dry run
predicted 1,036; the three extra are 39fb64b6's pin of the library's
nostrPublicKey() against NIP-06.
**What is not done is named rather than implied.** Phase 6's two reverse-pulls
--
|
||
|
Kgothatso Ngako
|
89e364c1bd |
chore(library): base the credentials branch on the library's master
The Phase 2 library commit was made on a branch cut from 59c11ed -- the nsec key-store commit, which is what the app's curated branch pins -- but that commit is not the library's default branch's tip: master is at e51e3ae, the merge of PR #1 that brought 59c11ed in. The two trees are identical, so the rebase is content-free; what changes is that claude/nostr-credentials is now one commit ahead of master rather than a sibling of its tip, and the app's pointer follows it to 84cc44c. The plan said the nsec library commit "sits on a remote branch called detached", which was true of where it was found and false of where it is: it reached master through the PR. What is still true, and still the rollout's one hard step, is that it is untagged -- the library has no tags at all -- and JitPack consumers resolve by tag. The two sentences that said otherwise are corrected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Pulled-From: curated/curated@00c36ec509 |
||
|
Kgothatso Ngako
|
042c5db5f4 |
docs: record what the npub sign-in plan built, and the twelve places it chose differently
Phase 8 of docs/npub-sign-in.md is the rollout, which is process rather than code; what is left to write down is what the seven phases before it turned out to be. The header moves from "Not built" to "Built", with the table the other phased plans keep: what the plan said against what the implementation did, twelve rows, each a decision worth reading before touching the code it describes -- the startup branch one phase early because the sealed type asked for it, the migration's three outcomes, the DAO guard that was belt-and-braces on paper and load-bearing for two phases, a null identity answering "can sign" with true because nobody is not read-only, and a round trip run against a real database with a contrast case so that "nothing was signed" is known to be a claim the harness can refute. One finding that belongs to no phase is recorded with it: a compose test looking for a floating action button's label has to search the unmerged tree, or its "does not exist" is vacuously true. And the one rollout fact that is not process: the library commit is on claude/nostr-credentials at 01962f3, bumped in by Phase 2, and like the nsec plan's before it has to be pushed and tagged before any of this leaves the machine. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Pulled-From: curated/curated@f866b9d17b |
||
|
Kgothatso Ngako
|
10df01f4b7 |
docs(npub): one credentials file, not two, and the write that makes the upgrade atomic
The plan's first design kept public keys in a plaintext file beside
nostr-keys.dat, app-side, so that nothing touched the library. Review asked
why the key file was not simply made a credentials file with a read-only
kind, and the answer is that it should be. The upgrade -- pasting the nsec
of a key held read-only -- was the one operation spanning both files, so it
was two writes with a crash window between them and a "secret wins" rule in
merge to repair it; with one file keyed by pubkey it is a single write that
replaces {type: public} with {type: secret}. Forget collapses to one path
with it. And the advantage that paid for the two-file design was worth less
than it looked: the library commit that introduced nostr-keys.dat is
untagged, so a credentials format rides the tag that work already owes.
Phase 2 is rewritten for nostr-credentials.dat: a typed entry per pubkey in
the same envelope, renamed rather than versioned in place because an older
build's listIdentities returns on SerializationError before it publishes
anything and would list no wallets at all; a migration that reads the v1
file once and deletes it, since a frozen copy would resurrect a forgotten
key on a downgrade; and the one upgrade that still spans two files -- a
recovery phrase over a public credential -- with its order stated. The
header, Phases 3, 6, 7 and 8, the estimate and the appendix follow, where
the two-file design now sits as the rejected alternative with the reason.
One overclaim of the revision's own is corrected in it: the library does not
use a class discriminator anywhere -- its cloud payloads pick a variant by a
version field -- so the plan says the discriminator is chosen here, not
inherited.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Pulled-From: curated/curated@dc8a1091d3
|
||
|
Kgothatso Ngako
|
69050bb743 |
docs: plan npub sign-in, starting from what a read-only identity is for
The nsec plan's out-of-scope note said a read-only mode is a product, not a branch. This plan takes that at its word: what a public key can see here is thin -- a profile card, its follows as search results, no feed, no rooms, since every room is MLS or a gift wrap to the key that was not pasted -- so the first section decides what such an identity is for before anything is designed. It is a preview: the app as your own profile, before you paste a secret into it. That one word settles the Messages tab (an empty state that offers the upgrade), pasting the nsec of a read-only key (an upgrade in place under the same id, not "already on this device"), sign out (real for this kind only), and not-found (try again or a different key, never set one up). Eight phases: a nullable key on Identity, with the note that the compiler will be silent about it; a plaintext list beside the two key files, app-side through the public getDatadir and AtomicFileWrite so nothing needs a JitPack tag; the third startup branch, a read-only KeyPair built in one place, and only the two pumps that read; the sign-in screen, where hex stays a secret because an x coordinate is almost always also a valid scalar; a LocalCanSign capability and an inventory of every write entrance one tap from the three tabs; two exits; two round trips; rollout. Two traps found on the way are recorded where they bite: quartz's KeyPair(privKey = null) generates a fresh key rather than meaning "no key", and decryptGiftWrapSeal forwards exactly that; and signInToProfile plants a second kind 0 on a second call, which nothing reached until the upgrade path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Pulled-From: curated/curated@78807fe956 |
||
|
Kgothatso Ngako
|
b127647145 |
feat(groups): the queue of a list the group curates, read off the relays it named
`930d37c8` gave a group the schema: the definition of a list, signed by the room's
own key, for strangers to answer. This is the answers. Under every schema card on the
group's screen there is now a "View suggestions" button, and it opens the list's
*queue* -- every kind 31888 anyone has published in reply to the schema's coordinate,
newest first, whoever signed it, with a mark on each one the group has already taken
up into the list. It is the second half of the curated-list NIP in this app, the
read side of it: suggestions (31888) and canonical entries (31890) are now read and
checked here. They are still not written; that is the third half.
The screen is bitcoin.mov's `/suggestions` page, which does the same thing for that
site's one list: one row per suggestion event rather than one per film, so that a
reader can see what is waiting before anything is added and whether the curator has
taken it up. `SuggestionList.tsx` and the store behind it, `useVideos.ts`, are the
reference for everything the screen decides -- one row per event, the newest
version per coordinate, "Curated" where a canonical entry shares the `d`, and a
timer standing in for an answer that never comes.
**The button is behind no gate, and it is per card.** Every other button in the
identity block is hidden from a member holding no share of the key, because each of
them proposes a signature by the group. The queue is the one thing in the block the
group did not write. A schema exists to be answered by strangers, and reading the
answers takes no share of anything, so the button is there for whoever is looking
-- `GroupNostrProfileSectionJvmTest` puts a member with no share in front of a
schema and finds it. It sits under its own card rather than once for the section,
in the same `item {}` as the card, because each list has its own queue and a button
between two cards would say nothing about which one it opened.
**The protocol is ported rule for rule, and the fixtures are the NIP's own.**
`nostr/curated/CuratedEntryEvent` is the reference module's `verifyEntry`,
`verifyCuratedCanonical`, `checkValue` and `valuesOf`, in the NIP's order: the kind
is the one the schema names; every required field is present; no field without
`repeat` appears twice; every value matches its field's type and config; every
`require-any` group is met; the `a` root names this schema and no other; and the
author is somebody the visibility admits. A canonical entry gets two rules on top --
its author is the schema's author, because curation is the one power a schema does
not delegate, and a source pointer, when there is one, is a well-formed suggestion
coordinate, because a malformed one credits the wrong person. `CuratedEntryEventTest`
takes *The Rise and Rise of Bitcoin* as `eebb74ab…` suggested it in the NIP's
example, and the curator's sign-off on it, and checks both against the bitcoin.mov
schema from the same document; then the doc's own minimum suggestion, which has an
IMDb link and no watch link and satisfies `require-any` that way; then every
rejection the doc lists and says why it matters -- an unknown type, a year outside
1900--2100, a non-https poster, a `javascript:` link, a title over 200 characters, a
`d` with a space in it -- plus the reply rules, the repeat rule, the closed-list
rule and the two canonical rules.
**Rejected, not repaired, and read by field.** The NIP says a client MUST verify an
entry against its schema and MUST reject one that does not satisfy it rather than
showing it with the bad parts blanked out; relays carry malformed, partial and
hostile events from other applications. So `suggestion` and `canonical` return the
entry whole or return null, and `verifySuggestion` and `verifyCanonical` return the
reasons as typed problems -- a field name and a `Reason`, the way `CuratedSchema.
problems` is an enum rather than a sentence -- so that a form later can show them
inline. What comes back is a `CuratedEntry` keyed by the schema's *field names*
rather than by tag, because a field is what a form and a screen know an entry by and
the tag is only where it was kept: the two `r` tags of the example land on
`watchUrl` and `imdbUrl` by their markers, the two `t` tags on `hashtags`, the body
on `description`. The `director`, `i` and `lang` tags on the NIP's example are on no
field of the NIP's schema and are not in the reading -- "tags the schema does not
define MAY be present and MUST be ignored".
**A pattern this platform cannot compile counts as not matched.** The reference
builds `new RegExp('^(?:' + pattern + ')$')` and would throw out of its verifier on
a bad one; JavaScript and Kotlin regex dialects also differ at the edges. Ignoring
an uncompilable pattern would accept entries the site refuses; refusing them is the
conservative reading, and it is documented at the check.
**A coordinate splits on the first two colons only.** A `d` may itself contain
colons -- `imdb:tt2821314` does, and it is the identifier the NIP's example uses --
so `CuratedCoordinate.parse` is the reference's regex rather than a `split(":")`,
and the test pins the identifier surviving its own colon and the pubkey being
lowercased on the way through.
**The queue is a reading over stored rows, with three things to close and one to
add.** `CuratedSuggestion.queueOf` takes whatever the local table holds for the
coordinate -- kinds 31888 and 31890 with an `a` tag naming it -- and reads the
queue out of it. Closed: an event that does not satisfy the schema; a kind 31890
signed by anybody but the group, which is somebody else's list and marks nothing;
and an older version of an entry its author has since replaced, since both kinds
are addressable and different relays hold different latest versions, so the merge
that realises an edit happens here, newest per `kind:pubkey:d` with the event id
breaking ties. Added: `isCurated`, true where a canonical entry the group signed
shares the suggestion's identifier -- the coordinate both land on, and the
reference page's rule. Newest first, because that is the order a queue is read in.
**A stranger is named by their profile, and by their key until one arrives.** The
indexer writes a placeholder profile named "LOADING..." the moment a pubkey is
first seen, and a queue full of that would be a queue of nobody.
`suggesterName` reads the profile only when it is a real one -- `createdAt` past
`GENESIS_AT` -- and otherwise the short key, which is at least stable and
distinguishing. The row's avatar is tinted from the key like every avatar here, so
two suggestions by one stranger read as one stranger.
**Read from where the schema says, and the screen says where.** The schema's
`relay` tags are signed into the event for exactly this: a client that finds the
list anywhere knows where its replies live and cannot be sent elsewhere by an
unsigned config. A schema naming none -- the NIP allows it and lets a client pick
-- is read from the group's general relay list, the *read* relays of it, since this
is a read; and failing an agreed, non-empty one, from this build's own relay, the
same fallback the schema editor seeds from. Spellings are normalised so one relay
written two ways is one request. The header names the relays asked, because a
queue read from the wrong relays and a list nobody has written to look exactly
alike from here, and a member should be able to tell them apart.
**Nothing here talks to a relay; it is a pull like every other one-off read.**
`CuratedSuggestionListViewModel` queues one `SynchronizeNostrEventRequest` per relay
for the sync pump to drain, carrying the NIP's two queries as one request's two
filters -- everything anyone published in reply to the coordinate, and what the
*group* published in reply to it, the second scoped by `authors` to spare the relay
the work `queueOf` does regardless -- and watches the local table for what comes
back. That watch needed a way to observe an arbitrary NIP-01 filter, which the
repository did not have: `observeNostrFeed(filter)` recognises a handful of shapes
and falls back to text notes for the rest, and none of its shapes is "these kinds
with this `a` tag". `observeNostrEventsMatching` applies the filter whole through
`NostrEventFilterQuery`, the builder negentropy already uses, so the `#a` match is
anchored and escaped rather than a substring scan; the DAO method behind it names
its observed tables explicitly, the events and the profiles joined onto them, so a
row whose author's kind 0 arrives after the row does re-emits with the name filled
in. What the screen shows is therefore what this device has *stored* for the
coordinate -- which is also what it shows with no network at all, and what it
showed last time until the relays answer again. `NostrEvent.toEvent` is the small
conversion the reading needs to hand a stored row to a verifier written against
the wire shape, beside `GroupSignedEvent.toEvent` which does the same for the
group's own.
**"Still looking" is bounded by time, because nothing else bounds it.** The pump
marks a request sent when the REQ goes out and processed when an event comes back.
A relay holding nothing for the coordinate answers with an EOSE the pump does not
record, so there is no signal for "asked and empty", and a screen that showed the
local table's first, empty read would call every list empty for the second before
its rows arrived -- which teaches a member not to believe it. So the queue is
"being asked" from the request going out until either something lands or ten
seconds pass, and only after that is an empty queue empty. The reference store does
the same with a six-second timer; ten here because the request queues behind the
pump's four subscription slots before it goes out. Rows arriving end it early, a
late arrival after it is still shown, and `withQueue` -- the one state merge --
pins all three in `CuratedSuggestionListViewModelJvmTest`. The empty state offers
"Ask again", which re-queues the same requests: the one thing the watch cannot do
on its own is make more arrive.
**The row shows what an entry is, not where to find it.** The screen is driven by
a schema it has never seen, so what goes on a row is a decision rather than a
layout: the title as the headline, since every list has one; a glance line of the
form fields that say what the entry is, as `Label: value`, because a bare
"2014 · en" says nothing to somebody who does not know the list's fields; the body
where the list has one; and who suggested it, when. Links are left to the sheet --
a URL on a row is a line of noise. "Curated" sits on the row as the one thing on it
a member might act on: it says the group already took this up, and re-curating it
would revise the entry rather than add one.
**The sheet has the whole entry and the event it came as.** Every field the entry
answered, labelled as the schema labels it and in the schema's order, so it reads
as the form would have; the title not repeated, since it is the heading; tokens and
links monospaced because those are compared and copied rather than read. Then the
signed event, byte for byte, with a copy button -- the way the key state sheet
shows the group's own statement and for the same reason: a prettier rendering is a
different string to the one whose id was hashed. It is also the thing a canonical
entry is built from, when that half arrives.
**Four states, and the transition between them.** Loading while the room and the
schema are read; an error, with nothing to retry, for a schema this device does
not hold, since the identifier came from a navigation argument and reading it
again fails the same way; and a loaded state that is either the rows, or looking,
or empty with something to do. `ScreenStateTransition` wraps the `when`, which is
the Scaffold's whole content.
**The audit's proper-noun list grows by two films.** The preview suggests *The Rise
and Rise of Bitcoin* and *Magic Money*, both title case for the correct reason, and
`m3-title-case.py` excludes sample data by name rather than by pattern.
**Still nothing has reached a relay, and it bites once more.** Group-signed events
are not yet published, so a group's schema has not been where a suggester could
find it, and its queue is empty until it has. The screen reads the coordinate
`31889:<group>:<d>` all the same and shows whatever a relay holds for it; the
suggestions on bitcoin.mov's relay reply to *that* curator's coordinate and are
correctly not a group's. `GroupCuratedSchema`'s caveat now says so and points at
the reading.
32 new tests. `CuratedEntryEventTest` (15) works from the NIP's example suggestion
and canonical entry against the NIP's schema: the reading by field, the minimum
suggestion, the source pointer and its absence, curation not being delegated, a
malformed source, the wrong kind, the reply rules, a missing required field, the
repeat rule both ways, every value rule the doc lists, `require-any`, the closed
and private lists, `checkValue` on durations, patterns and https, and the
coordinate split. `CuratedSuggestionTest` (4) reads a queue out of stored rows:
verified suggestions newest first with the broken, the foreign and the wrong-kind
ones dropped, an edit collapsing to its newest version, the curated mark placed by
the group's canonical entry and not a stranger's, and the naming of a suggester
with a profile, a placeholder and nothing. `CuratedSuggestionListViewModelJvmTest`
(6) covers the relay order and the read-only filter, one request per relay carrying
both queries and the watch covering both kinds, the three outcomes of `withQueue`,
an `initiate` run end to end against fakes -- the relays asked, the filter watched,
the rows shown, the looking ended -- and a missing schema being an error rather
than an empty queue. `CuratedSuggestionListScreenJvmTest` (6) renders the header,
the row's glance line with the link kept off it, the mark on the right row and only
that row, looking against empty, and the sheet with its fields, its link and its
event. One new case and one extended in `GroupNostrProfileSectionJvmTest` put the
button under each card in turn and in front of a member with no share.
1398 tests pass -- 893 in `:composeApp:jvmTest`, 505 in `:composeApp:testDebugUnitTest`
-- `:composeApp:compileDebugKotlinAndroid` is clean, and `m3Audit` meets every budget.
Replayed onto Mantra by docs/curated-to-mantra.md: strings.xml: taken as the original merge c8de3a1f left it, this being the branch join.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Pulled-From: curated/curated@1e52fc8f25
|
||
|
Kgothatso Ngako
|
14ffa04e7c |
docs: record what the nsec sign-in plan built, and the twelve places it chose differently
Phase 8 of docs/nsec-sign-in.md is the rollout, which is process rather than code; what is left to write down is what the seven phases before it actually turned out to be. The header moves from "Not built" to "Built", with the table the other phased plans keep: what the plan said against what the implementation did, twelve rows, each one a decision worth reading before touching the code it describes -- the not-found exit as a screen state rather than a route, input problems on the field rather than through ErrorState, the kind 0 written over the placeholder rather than beside it, forget taking the account rows too. Two findings that belong to no phase are recorded with it: the android source set's file-name collision that put the failure classifier in its own file, and DataStoreManager's process-global preferences cache, which only a test reusing a key across directories could have found. And the one rollout fact that matters: the library commit is on claude/nostr-key-store in the submodule, bumped in by the Phase 3 commit, and has to be pushed and tagged with this branch. The docs index now says the plan has been built and points at the table. Replayed onto Mantra by docs/curated-to-mantra.md: README.md: line-set three-way merge, both sides' additions kept and this commit's deletions applied. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Pulled-From: curated/curated@2326839e48 |
||
|
Kgothatso Ngako
|
c49914818a |
docs: plan nsec sign-in, and the ten call sites that make it small
An nsec is a BIP32 leaf of the seed at m/44'/1237'/0'/0/0, and the derivation runs one way, so an nsec can never have a wallet behind it. What makes the feature tractable anyway is that nothing in the app reads the wallet except the nostr key -- ten sites, all the same expression -- so the plan puts an Identity in front of the wallet and gives an nsec an identity with no node behind it. Eight phases: the identity type; a sibling key file in the library under the one keystore alias Android will accept; listing and starting both kinds; the sign-in screen for a phrase or an nsec, deduped on pubkey rather than wallet id; indexer relays and a not-found exit for the sync that today asks one relay and never finishes; recovery for a key with no phrase; tests; rollout. Two findings along the way are recorded as pre-existing rather than new: loadNostrProfile(startupRoute) keys off a Profile row a placeholder account does not have and answers Landing while observeProfile answers the sync screen, and the library's LocalKeyManager.nostrPublicKey() returns the 33-byte compressed key, not a nostr key. Replayed onto Mantra by docs/curated-to-mantra.md: README.md: line-set three-way merge, both sides' additions kept and this commit's deletions applied. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Pulled-From: curated/curated@daae63d5c5 |
||
|
Kgothatso Ngako
|
ce951bb5d0 |
docs(scripts): the pin follows the commit even when git fast-forwards the gitlink
Found landing Phase 3 for real, one commit after the trial had passed it. In the trial worktree the submodule directory was an empty placeholder, so when 366b0177 moved the pin from 01489b8 to 59c11ed against a branch already at 84cc44c, git could not compare the three and reported a conflict, and the pin rule set 59c11ed as intended. On the real branch the submodule is populated, git can see that 59c11ed is an ancestor of 84cc44c, and it resolves the gitlink by fast-forward -- cleanly, silently, and to the newer pin, which is the one the commit does not compile against. So the pin rule no longer waits for a conflict: after any pick of a commit the table names, the gitlink is set to what the table says, and the note says git had fast-forwarded it. The same code path now covers 5efeae76's mapping of the unfetchable 01962f3 onto 84cc44c, which used to be a special case. Phase 3 was reset to the Phase 2 tip and is landed again with this in place; nothing else about those eight commits changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
2a02fbc14f |
docs: record the day's dry run, and the three things it changed in the plan
Phase 1 of docs/curated-to-mantra.md, run on 2026-09-13 on top of Phase 0, on a scratch worktree that landed nothing. The plan asked for exactly this -- redo the dry run the day you land, because the numbers drift -- and it was right to: the upstream had moved, the pipeline as written could not do phased landings, and the library pin turned out to have to move backwards. All three are now in the plan, in the Phase 1 section, as a record beside the plan rather than a rewrite of it. **The replay lands by cherry-pick in topological order, not by `rebase --rebase-merges`, and the reason is a property of git rather than a preference.** The plan's phases are cuts through a graph with four merges in it. Rebasing a cut whose merges have one parent in an earlier cut recreates each such merge against the *pre*-replay parent -- the commit in the rewritten source branch, not the one already landed -- and drags the unrebased lineage in beside the rebased one. A single whole-range rebase, which is what the plan's dry run measured, never meets this because every parent is inside the range. So: the ordinary commits are picked one at a time, the merge commits land as nothing, and their content, which is only ever conflict resolutions, is folded in where the conflicts actually surface. The driver that does it is docs/scripts/curated-replay.py, added here; every commit it lands carries a `Pulled-From: curated/curated@<sha>` trailer stamped by the rewrite, and a paragraph naming any resolution made on the way in. **At a branch join the join files are taken exactly as upstream's own merge left them, and a union was tried and rejected three times before that rule was reached.** A union -- keep both sides of the conflict -- is the obvious resolution for two branches appending strings to the same file, and it was wrong three ways, each caught by the exactness check against the rewritten tip and none of them visible in a passing build. It duplicates lines both sides already hold when a conflict hunk widens (thirteen string keys, twice). It never applies the other side's deletions, so the two copy-suggestion strings that fcc19f95 removes survived. And, the one that took longest to see, a join resolved in a different *order* from upstream's merge leaves every later commit that edits the block unable to find its base, so its deletions fail silently while its additions land -- which is why the second fix still left the same two strings behind. The rule that survives: files whose lines are unique by construction (string keys, imports, route registrations, table rows) get a line-set three-way merge over diff3 hunks -- ours, minus what theirs deleted from base, plus what theirs added that the file does not already hold anywhere -- and never at a join, where the file upstream's merge produced is the answer. The driver's docstring says all of this so the next reader does not rediscover it. **The library pin goes back to 59c11ed for Phases 3 and 4 and forward again in Phase 5, and the compiler was asked before deciding.** The nsec line was written against lightning-kmp-app 59c11ed and writes through its `NostrKeyManager`; 84cc44c, Mantra's pin, renamed that class to a read-only `LegacyNostrKeysFile`. Reasoning said the Phase 3 cut would not compile against 84cc44c; the trial worktree was put at that cut and built against it, and produced eight `Unresolved reference 'NostrKeyManager'` errors in IdentityWriter.kt. So 366b0177 moves the pin to 59c11ed, whose nested lightning-kmp -> bitcoin-kmp -> secp256k1 chain is the same commit as 84cc44c's and rebuilds nothing native, and 5efeae76 brings it to 84cc44c -- not to the 01962f3 it named upstream, which is a branch commit since rebased onto the library's master and no longer fetchable, but whose tree 84cc44c reproduces exactly. Every commit on the branch will compile against the pin it records, which is what upstream's history had and a fixed pin would have thrown away. Alternatives rejected: adapting the Phase 3 commits to the renamed class (rewriting upstream's work on the way in, and inventing an intermediate state nobody built) and landing Phases 3 to 5 as one unit whose inner commits do not build (bisect would hate it, and so would review). **The upstream moved, and the new line is the next pull rather than part of this one.** curated/curated went from 86cb876b, which the plan was written against, to 29027f2b: ten commits for several profiles on one device, one of which (759199f2) adds schema version 20, the fork's first migration. They are recorded and left out on purpose; a migration landing on Mantra's database deserves its own decision, and the plan's own principle -- pull the whole non-brand tree so the next pull is a replay -- says how that decision goes once it is made. The trial, with the committed driver: 30 commits replayed (4, 8, 6, 12), 21 clean and 9 with a resolution note, 0 stuck; the driver's own rerun from the Phase 0 tip reproduces the tree and improves on the hand-run trial by the one README line the old union had wrongly kept; residual diff against the rewritten 86cb876b exactly the known set -- Phase 0's prose, 39fb64b6's files, 808a3459's three, this document and its scripts, the logo. :composeApp:compileDebugKotlinAndroid and :composeApp:compileKotlinJvm clean; :composeApp:jvmTest 1,039 tests, 0 failures; :composeApp:testDebugUnitTest 530 tests, 0 failures; :composeApp:m3Audit all budgets met. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
ba0830a3d4 |
docs: say why the two hash tags and the relay host must never be renamed
Phase 0 of docs/curated-to-mantra.md, item 3. Three strings in this tree spell "mantra" for reasons that have nothing to do with the brand, and until now nothing beside them said so: `SharedKeyDerivation.TWEAK_TAG` and `ChillDkgRitualManager.HOST_KEY_DERIVATION_TAG` are inputs to hashes, and `Relays.ephemeral` is a relay that is running. Each now carries a comment saying what renaming it would cost, and docs/shared-key-derivation.md gets the paragraph that ties the three together. **The comments come from the fork, and are rewritten rather than pulled.** The Curated fork found out what these strings were the hard way: it renamed the app twice, and each time had to decide which of thousands of "mantra" tokens were the brand. Its rebrand commits (3bc8be53, e6aee792) left these three alone and wrote down why, and run through the pull's name-rewrite those commits collapse to almost nothing but those comments. They were not taken as commits, because what survives the rewrite is a sentence like "has survived two rebrands -- Mantra to Curated, Curated to Mantra", which in this repository describes rebrands that never happened. The fact they state from this side is different and worth stating plainly: the fork keeps all three byte for byte, so a Mantra member and a Curated member of one group derive one key and talk to one relay, and a rename *here* would split them as surely as a rename there. **Why comments at all, when the derivation note already has the rule.** The note's one rule is about the path a key is derived along; it never said that the tag string itself is part of the derivation, and the failure mode of renaming it is silent -- every room orphaned, every partial signature aggregating to nothing that verifies, and nothing on screen to say so. A `v2` tag is the shape a deliberate change would take, and the comments say so, so that the next person to grep for the brand finds the answer before the diff. **`ComposeAppCommonTest` moves from `press.auxiliary` to `press.mantra`.** It is the KMP template's `1 + 2 == 3`, the last file under a package the app vacated two brands ago, and the fork relocated it rather than deleting it so the source tree has one root package instead of an orphan under an empty one. Same here; it is moved with `git mv` so its history follows. No behaviour changes. :composeApp:jvmTest 736 tests, 0 failures; :composeApp:testDebugUnitTest 403 tests, 0 failures; :composeApp:m3Audit all budgets met. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
4681ac1a03 |
refactor: retire the brand before mantra, and keep the three records of it
Phase 0 of docs/curated-to-mantra.md, item 2. The app has been Mantra since
before this repository had a docs/ directory, and four things still said Torch,
the brand before it: the theme composable, `TorchTheme`, at 116 sites; the
user agent, `UserAgent.APP_NAME` and `CLIENT_NAME`; two error strings a user
could actually be shown ("tell X to use Torch", "finished setting up Torch");
and iOS, where `Config.xcconfig` built `PRODUCT_NAME=Torch` under the bundle id
`ac.aux.compose.Aux` and the project's product reference was still `Aux.app` --
two brands ago. All four now say Mantra.
**This is done natively, before anything is pulled from the fork, so that the
fork's theme maps onto a name that means something.** The Curated fork retired
Torch itself in d26cf6c7 (`TorchTheme` -> `CuratedTheme`, later `CurareTheme`).
The pull rewrites every fork commit into this repository's names, and until
now the only honest target for `CurareTheme` was `TorchTheme` -- the brand
before last -- which would have had every pulled screen wrapping itself in a
name the app stopped using long ago. `MantraTheme` is what the rewrite maps to
from here; docs/scripts/curated-unbrand.py's theme, `UserAgent` and iOS rules
are changed in this commit for that reason, and the comment that said "change
here if Phase 0 renames it" goes with them. Retiring Torch as part of the pull
instead -- by taking the rewritten d26cf6c7 -- was rejected because that commit
would then be a fork commit renaming something the fork never had, and because
the iOS half of the debt was never the fork's to pay.
**The user agent went last, and the reason it could go at all is written where
the next reader will look.** `CLIENT_NAME` is the `client` tag on every relay
list this app publishes, which is why it was left alone when the top bar was
fixed: it goes on the wire, so it read as a network identity question. It is
attribution and nothing derives from it, which is the difference between it
and the two `mantra/` hash tags that must never move; the comment on the
`HomeScreen` bar and the paragraph in material-design-conformance.md that both
said "still says Torch" now say that, in order, rather than describing a state
this commit ends.
**Three records keep the old name on purpose.** material-design-conformance.md:278
records a finding -- the top bar once read "Torch" while the window title read
"Mantra" -- and changing it would falsify the record. The paragraph at :654 is
rewritten rather than deleted because it was a statement of current state, and
the current state changed. `NavigationRoutingTest`'s "Introducing... Torch"
fixture is the string a stranded user actually saw, and the test is about the
stranding. Nothing else in the tree says Torch.
**iOS is unverified.** The build gates the apple targets behind `isMacOsX` and
this was built on linux; the xcconfig and pbxproj edits are the same shape the
fork's went through, and this is the first time this repository has named
itself there. Build it on a Mac before believing it.
:composeApp:compileDebugKotlinAndroid and :composeApp:compileKotlinJvm clean;
:composeApp:jvmTest 736 tests, 0 failures; :composeApp:testDebugUnitTest 403
tests, 0 failures; :composeApp:m3Audit all budgets met.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
b796a32be3 |
docs: plan the pull of Curated's work back into Mantra, and the tool that makes it mechanical
Curated (remote `curated`, curated/curated-kmp) forked from this repository at |
||
|
Kgothatso Ngako
|
0be31803f2 |
docs: record phase 10, and the deferred decision it carries out
`docs/subgroups.md` was written as ten phases of reasoning kept in the order they were argued, and phase 4 spent forty lines on why the child's ceremony was *not* held in the parent's Marmot room -- explicitly so the decision would not be re-litigated without its price attached. That section is now a shopping list that has been carried out, so it keeps its argument and gains a pointer forward, and the three costs it enumerated are checked off one by one in a new phase 10. The parts of the note that state the old arrangement as present-tense fact are updated rather than annotated: the three-ceremonies table now reads one room, three ceremonies, two quorums, and says the thing that needs saying twice -- an MLS message reaches the whole tree, so a ceremony in the parent's room has to name who it is with. Phase 10 itself is written the way the others are, around what fails silently: - `DkgSession.chatRoomId` stopped identifying a ceremony, and the place that matters is `completedKey`'s last fallback, which every member welcomed after a group's own ceremony lands on; - `signingPath` had to admit a Marmot room, which widens the one function whose contract is that a path never comes off a proposal; - the p-tags had to stay on both transports, which is the opposite of what `FrostSigningManager` correctly does. The two sections that argued the old collision -- "The collision this buys" and "Why a subgroup cannot be the whole group was withdrawn" -- keep their reasoning and gain the end of it: `(room, parent)` stopped telling two subgroups of one parent apart, so the lookup moved to `(room, parent, admins)`, and the permanent half of the refusal disappeared with the derived room. The limitation and the appendix entry are struck through rather than deleted, since what they were weighing is why the phase exists. `docs/shared-key-ceremony.md` no longer says a ceremony runs over a NIP-17 chat. The participant set is the proposal's p-tags on both transports, and that distinction is the whole reason it is stated that way rather than as "the group". `docs/mls-skipped-keys.md` keeps `proposeRitual` in its table of reliable triggers and now says what changed about it: it reached that table on gift wraps, where the bug does not apply, and a subgroup's ceremony now rides group events. It is the entry with the worst consequence -- a ChillDKG cannot finish until every participant takes part, so one lost round-1 message stalls it permanently for everybody rather than costing one member a line of chat. That is the thing the quartz fix in that note is now load-bearing for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
45cc80b538 |
feat(marmot): put a # in front of every group's name, and retire (#admins)
A device's room list holds two unrelated kinds of room and nothing on a row said
which. A NIP-17 room is a conversation between the people in it. A Marmot room is
a *group* -- an id its key derives, a membership baked into an MLS tree, admins
who can act for it, a signature anyone holding the id can check -- and the two
behave differently enough that guessing is a mistake.
`"Ekklesia (#admins)"` was an attempt at saying so, and it marked the wrong half.
Only the admin room got it; a subgroup got no marker at all, so as soon as a group
had one child, half the Marmot rooms on the device were unmarked. It also sorted
nowhere near the group it belonged to, and a truncated row drops a trailing suffix
first -- so the marker was missing exactly where the list is crowded enough to
need it.
**The rule is `MarmotGroupName.of`, and it runs where a room is minted rather than
where it is drawn.** The name is baked into the epoch-0 `MarmotGroupData` every
member is welcomed with, so a `#` added at display time would be a name this
device alone could see. `#Ekklesia` marks both kinds of group room, and marks them
at the front.
**Three mints, because there are three ways a Marmot room comes into existence.**
`MarmotGroupCreation.create` is the funnel for two of them -- the admin room a
group opens after its ceremony, and a subgroup -- and normalising there means
neither caller has to remember. The third, `SelectChatRoomTypeViewModel`'s
convenient room, has a random id rather than a derived one, so it has no key state
to adopt and no admin set to bake in and does not pass through that funnel; it
applies the rule itself.
**Idempotence is load-bearing, not tidiness.** A subgroup's name is derived twice
from the same bare ceremony-room subject, by two callers that never see each
other: `SubgroupManager.proposeBirthCertificate` normalises the name the parent's
quorum is asked to sign, and `MarmotGroupCreation` normalises the name the room
carries. Those two have to be the same string, or the subgroup is not called what
its parent certified -- and a certificate is a signature over the name, so a
verifier comparing them would see a real mismatch. `of` being idempotent is what
makes them agree by construction rather than by both sites being kept in step.
**The ceremony room keeps the bare name.** It is a NIP-17 room -- where a subgroup
is made, not the subgroup -- and prefixing it too produced two identically-named
rows, which spends the mark to say nothing. `Translators` (the ceremony) now sits
beside `#Translators` (the group it stood up), which is the distinction the `#`
exists to draw. Its subject is trimmed, so the bare name and the two normalised
ones cannot differ by whitespace.
**The `#` is drawn beside the name field, not pushed into its state.** `name` in
`SelectSubgroupAdminsViewModel` stays bare and the M3 `prefix` slot shows the
convention, because normalising on every keystroke moves the caret out from under
somebody halfway through a word. The coordinator still reads the name they are
about to get.
Four strings lose the old name -- "Create the #admins group" becomes "Create the
admin room", and the three about what "the #admins room" will sign with now say
"the admin room". Their keys are renamed with them, since the keys in this
catalogue are derived from the text. Around twenty comments, two screen previews
and seven test fixtures follow.
Docs: the ceremony note states the convention and what it replaces, and the
subgroups note's name-field section is rewritten -- it had been arguing from the
`"${parent.subject} (#admins)"` synthesis that no longer exists.
`docs/mls-skipped-keys.md` keeps its `"Frosty (#admins)"`: that is a captured
debugging log, and rewriting it would falsify a record.
Three tests. `MarmotGroupNameTest` pins the rule, idempotence included.
`MarmotGroupCreationJvmTest` pins the funnel -- a bare name in, `#Ekklesia` on both
the room row this device draws and the group data every other member reads.
`SubgroupManagerJvmTest` pins the pair that has to agree, by reading the proposed
event's tags back out of the signing session: the name the parent is asked to sign
is the name `MarmotGroupCreation` will give the room. That last one needed the
signable-parent fixture to seed host keys, since a ceremony's signer ids are
derived from them rather than stored.
**Rooms that already exist keep their names.** The name lives in the epoch-0 group
context, so renaming one is an MLS commit every member has to process -- a
different change from a naming convention, and not made here.
403 common tests, 726 jvm tests, `m3Audit` meets every budget with 0 title-case
strings and 0 dp literals.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
1930d6aaef |
fix(subgroups): a subgroup may be the whole group
"A subgroup cannot be the whole group. Leave at least one member out." That rule shipped in Phase 8 and it was wrong twice. **It refused something legitimate.** A subgroup is a logical division -- a group deciding that some of its work belongs to a differently-keyed room -- not a group carving out a smaller membership. Every member being in it is an ordinary case, and no guard here had any business deciding otherwise. **And it was a proxy, not a check.** The thing it stood in for is real: a ceremony room is derived from its admins, so a subgroup over everybody lands in the room the group's *own* ceremony was held in, and `proposeRitual` handing back that ceremony would quietly make the child the parent. But set size does not detect that. A parent whose membership has changed since its own ceremony derives a different room -- so the sizes can match with no collision, and differ with one. **Sharing the room was never the problem; sharing a ceremony was.** `DkgSession.parentChatRoomId` already told two ceremonies apart, so the fix is to scope the lookup by it rather than to forbid the selection. `DkgSessionDao.getLatestSessionFor(room, parent)` replaces `...ForChatRoom` at the three places that decide whether a ceremony already exists: `proposeRitual`'s one-at-a-time guard, `refuseCeremonyRoom`, and the two repository observers the ritual screen follows. A room may now hold the group's own ceremony and a subgroup's at once. Nothing below that lookup had to learn about the second one. A ceremony's messages, approvals and transcript are already keyed by session id; only the question "what is this room's current ceremony" was ever room-scoped, and that question was always really "for what purpose". One collision survives and it is degenerate: the same parent, over the same admins, twice. Those two have nothing left to distinguish them -- which is another way of saying they are one subgroup asked for twice, and that is what the message now says. `a subgroup that is the whole group is refused` becomes `a subgroup may be the whole group`, and two new cases pin the scoping: the group's own ceremony sitting in the derived room does not block a subgroup there, and the same parent asking twice over the same admins still does while a different admin set is untouched. `docs/subgroups.md` keeps the withdrawn rule struck through in the refusals table with a section saying why, rather than quietly deleting it -- the reasoning that led to it is the reasoning somebody would repeat. 397 common tests, 713 jvm tests, `m3Audit` meets every budget. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
0180425904 |
docs: record what the subgroups plan built, and the six places it chose differently
All nine phases are built, one commit each. The phases are kept as written -- they are the reasoning, and the code reads better against the argument it came from than against a summary of itself -- with a table of where the building disagreed with the plan. Six worth reading. `openCeremony` was never built, because a wrapper over two repository calls the picker already makes would be a third name for one act. `MarmotGroupCreation` is reached through the repository rather than called from a view model, because view models here talk to repositories and managers take the database. The guards' tests are in jvmTest rather than pure, because every refusal reads the database and a pure version would test less. Phase 4 added two tags rather than one, the second fixing a bug older than subgroups -- every robust group has been arriving nameless on every device but its creator's. `stateFrom` needed a third reader and a wrapper, because "tag present and unreadable" looks identical to "absent" through a parser, and "refused" has to be distinguishable from "none claimed". And Phase 8's two capability refusals short-circuited the tests already written, which is how it came out that the fixtures had never had a parent that could sign. Two the plan got right and worth keeping if this is ever rewritten: the key-package check moved to the picker on review, before a line was built, and it is the difference between a subgroup failing in a second and failing after three ceremonies; and the founding-roster rule has a test whose job is to fail the day somebody adds the comparison that looks obviously missing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
6fb0af1147 |
docs: close five gaps in the subgroups plan, one of which wasted three ceremonies
A review pass over the plan committed in
|
||
|
Kgothatso Ngako
|
c1ce262f3e |
docs: plan subgroups, and phase the four ceremonies a group needs to make one
A group can make another group, and the child can prove where it came from. This
is the plan for that, in nine phases, written against the code at
|
||
|
Kgothatso Ngako
|
113eda9f4d |
feat: sign a group's key state before its room exists, and put FROST on NIP-17
A room's `GroupKeyState` was the new #admins room's first application message: the coordinator created the room, added the members, and only then asked the group to agree what it signs with. The order is now reversed. The group agrees it while it is still just a ceremony and a NIP-17 chat, and the room is created already knowing. **Two things were wrong with the old order, and neither was cosmetic.** The room's founding fact was settled after the founding, so a session that never reached a quorum left a live room whose every member fell back to rederiving -- which works, but only at the one path the constant names, and says nothing about which ceremony a device should take its share from. And the members who had to sign it were exactly the ones the room had just been created to hold: a member whose key package could not be found was excluded from the room *and* from a decision they held a share of, while `createAdminGroup` refuses to create the room at all in that case. Agreeing first makes the state a precondition of the room rather than an afterthought. **Signing therefore has to work in a NIP-17 room, and `broadcast` is the only place that knows.** In a Marmot room a signing message stays an ordinary inner event, encrypted to the group and addressed to nobody, because who is in the group is the MLS tree's business. In a NIP-17 room it goes out as one sealed gift wrap per member and has to name them all, or the members it left out never hear. Neither shape lets a recipient list decide anything -- the signer set comes from the ceremony's host keys either way -- so tagging somebody does not put them in it and failing to tag somebody only stops them hearing. Everything above `broadcast` is the same protocol; `NostrDao` dispatches the 3032x kinds off the gift-wrap path beside the DKG's, and the outbound path needed no change because `sealGiftWrapPayload` already seals to the room's participants and already refuses MLS rooms. **`signingPath` gains the one case that cannot be self-checked.** Every other candidate is right exactly when walking it reaches the room, which makes the resolution self-checking rather than trusting. A NIP-17 room's id is an aggregation of its members' keys, so no path reaches it and nothing can be checked that way. What the group signs as there is the room it is about to make: the ceremony's key at the app's admin path. That is admitted only when the ceremony is *this room's own* -- `key.chatRoomId == chatRoomId`, read from this device's database -- and the path is the constant rather than anything off the wire, so a proposer still chooses nothing. Naming some other ceremony this device holds a share for gets no path at all, and `completedKey` will not even find a key for a NIP-17 room that did not host one, so such a room cannot open a session; both are tested. **A state's subject is now its own `d` tag, not the room it arrived in.** Those used to be required to agree, and a mismatch was dropped -- the right rule while a state was made in the room it described, and the wrong one now that the two differ by design. Nothing is given up. The check that drop was standing in for is still made and made against the *named* room: `GroupKeyState.verifies` has to rederive it, and `isSignedByGroup` has to find a signature by the key that rederivation reaches. A state can therefore only ever be about a room it derives, whatever room it turned up in, so nobody can point one room at another room's key by putting it through the wrong door. The arrival room survives only as the fallback for a state carrying no `d` tag at all. **`record` holds what it cannot file; `adopt` files it when there is a room.** `GroupKeyState.chatRoomId` is a foreign key, so a state signed before its room exists has nothing to hang on -- which is now the normal case rather than an error. `record` says so and keeps the signed event; `adopt` reads it back off `GroupSignedEvent` and files it the moment a room appears. Both ways into a room end there: the member who creates it, in `createAdminGroup` and before the members are added, since filing is local and doing it while the room is certain to exist beats doing it after a step that can partly fail; and the member who arrives on a Welcome, in `NostrDao`, off the same event they were already holding because it was signed in the room they were already in. Nothing goes on the wire in either case. A member who was not in the ceremony holds no such event and gets nothing, which is right -- they hold no share either, so there is nothing for them to pick the wrong one of. **The screen watches the signed event, not a state row, and that is not interchangeable.** There is no row until there is a room, so the only thing that can say the agreement was reached is the event. `observeSignedGroupKeyState` is a flow over `GroupSignedEvent` by kind for the same reason the button it gates exists. Gating on the session's own items instead was rejected twice over: `complete` writes `stage = COMPLETE` *before* `recordSignedEvents`, so a collector woken by the session row can read before the event lands; and an item can hold a signature that has not been verified yet -- `complete` is where each one is checked against its id and author, and throws if it is not. **The button is one control and two steps, in the order they have to happen.** "Agree the group's signing key" until a quorum has signed, "Create the #admins group" after. Offering both at once would be the old order still available, and `createAdminGroup` refuses it in the view model as well, since the screen not drawing something is not a guard. A failed session re-offers the propose button and nothing else does, because a retry has to be a *new* session: the failed one's nonce seeds have already been published against an aggregate, and reusing one produces two partial signatures under a single secret nonce, which is how a share is extracted. `propose` mints a fresh session id every time, so tapping it is the safe retry by construction. **One bug found in review, which the tests now pin.** `replayStoredMessages` read only `marmotInnerEventDao`, so in a NIP-17 room a message arriving before the proposal it belongs to -- routine on a fresh sync, where a relay hands over a backlog in whatever order it likes -- was stored in the gift-wrap payloads and never read back. It now reads whichever store the room's transport writes to, which has to be the same reading `broadcast` makes. `a nonce arriving before the proposal is replayed out of the gift wraps` fails against the old code. **One wart, taken deliberately.** `GroupSignedEvent.chatRoomId` means the room a signature was made in, which for every event but this one is also the room whose key signed it. The key state is filed under the ceremony's room and authored by the #admins room, so `GroupSignedEvent.verifies` cannot pass on that row -- check it with `GroupKeyStateEvent.isSignedByGroup`, which asks the question the row cannot. Both columns are documented to say so. Re-filing the row under the #admins room once it exists was the alternative and buys nothing: a key state is not chroniclable, so no reader wants it there, and moving a row to keep one helper honest is worse than saying where the helper stops. `ChronicleManager` and `docs/member-chronicle.md` both argued for the `isChroniclable` filter from "every room signs a `GroupKeyStateEvent` as its first act", which is no longer true of any Marmot room. The filter stays and the argument is restated: what it stops is a member replaying any group-signed statement *about* the record as though it were work, and `applyPage` refuses the same kinds coming the other way. The two are a pair and neither is safe to drop on the strength of the other. `ChronicleAssemblyJvmTest` now puts its key state on file by hand, which makes that test sharper rather than hypothetical. `SignedGroupKeyStateTest`'s harness flattens the two transports into one `Queued` shape and each device declares whether its room has MLS state, so every existing test keeps testing the Marmot path and the seven new ones read the same. `GroupKeyStateTest`'s "a state naming another group's key is dropped" splits in two: one holding the room fixed and varying the key, which is still a drop, and one varying both, which is another group's true statement and is now attributed to that group's room rather than refused. 649 jvm tests and 373 common tests pass; `m3Audit` meets every budget. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
a56295b0e2 |
docs: record what phase 8 built, and what is left for a person across all nine
The plan's last phase becomes a record, and the document gains a closing status: every count the audit was written to move, from the state in "Where this app stands" to what `m3-audit.sh` reports today, and a gathered list of what a person still has to look at — the eight screens with competing filled buttons, the two list-detail families the pane work did not reach, the container transform, desktop keyboard traversal, and the avatar picker's selected state. **The audit caught the previous commit.** `ThemeGallery` added eight string literals in composables, taking the count 39 -> 47, which is exactly the drift the budget exists to notice. They are sample text — the words are chosen to be words, so that colour pairings can be looked at — and putting them in the catalogue would add eight entries no screen shows and a translator would have to be told to ignore. So the audit grows a third exemption marker beside `m3-color-exempt` and `m3-spacing-exempt`: `m3-string-exempt`, per file rather than per line, because the exemption is a property of what the file is for and eight markers down one gallery would say less than one at the top of it. Back to 39, and the report now says how many files are exempt so the mechanism cannot be used quietly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
b6aa2111ac |
docs: record what phase 7 built, including the API the plan named that does not exist
Phase 7 becomes a record. Three things in it are corrections to the plan rather than notes on it, and all three are the kind that only surface once somebody tries: - `MotionSchemeKeyTokens`, which the plan says every spec should come from, is `internal` to material3 and not addressable from an app. `MaterialTheme.motionScheme` is the public surface and gives the same six specs. - "every state change in the app is a hard cut" was true of screen states and not of navigation, whose default is a 700ms fade in navigation-compose's internals. Still worth replacing -- three times M3's duration, and a literal in a dependency -- but for a different reason than the one written down. - Android has no reduce-motion setting. It has "Remove animations", which zeroes the animation duration scales, and Compose ignores those scales entirely. Also what was deliberately left: the container transform between a list item and its detail screen. It is `SharedTransitionLayout` work, and above the expanded breakpoint the detail is already beside the list, so there is no container to transform -- doing it before the pane split settles means writing it twice. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
16775bf6c7 |
docs: record what phase 6 built, and give the audit a floor to defend it
The plan's phase 6 becomes a record rather than a proposal, in the shape the earlier phases took: what was built, what was decided and why, what a person still has to look at. Two decisions in it were the product owner's rather than the code's -- promoting search and profile to navigation destinations, and doing chat alone rather than all three list-detail families -- and both are named as such with the date. **The audit learns two things.** It counted `NavigationBar(`, `NavigationRail(` and friends, and reported **zero** for an app that had just grown a navigation bar: `NavigationSuiteScaffold` is what chooses between them per breakpoint, and the concrete component never appears in the source. It now counts the scaffold and its items. And it grew a `floor()` beside `report()`. Every other budget in the file is a ceiling that ratchets down as a phase lands, which is the right shape for literals, hardcoded colours and untriaged nulls -- things a careless edit *adds*. The adaptive work is the opposite: a screen that stops reading the breakpoint still compiles and still renders, and the count goes down. So `--check` now also fails when the adaptive API count drops below 12 or the navigation component count below 2. **Two `contentDescription = null` that the audit caught in this phase's own work** -- the navigation item's icon and the new-chat button's -- now say `Decorative`. Same null, and the same convention phase 3 established: recording that somebody looked is the whole point, and a budget of zero only holds if new code obeys it too. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
043d725599 |
feat: draw the expressive loading indicator, and stop shouting the sign-out button
Phase 5, second step, of docs/material-design-conformance.md. Two smaller pieces, and a
correction to the plan.
**41 loading states stopped being a gold spinner.** `LoadingDataIndicator` wraps every wait
in the app, and it drew a `CircularProgressIndicator` hardcoded to 80dp in
`colorScheme.secondary` -- the brand gold, which reads as a warning rather than as a wait,
on a component that has a size of its own. It now draws `LoadingIndicator`, which is M3's
component for an indeterminate wait with no progress to report and the one
`MaterialExpressiveTheme` expects to be paired with. One wrapper changed; 41 call sites
follow.
**The profile screen had two maximum-emphasis buttons, and one of them was Sign out.**
Seven actions in one list: five `TextButton`s (edit profile, key packages, change account,
profile keys, network relays) and two filled `Button`s. A filled button is M3's highest
emphasis and is meant for one action per screen, so this was two competing primaries -- and
the more prominent of the pair was the list's most destructive item.
Sharing is now `FilledTonalButton`: it is the useful action, at medium emphasis rather than
maximum. Signing out is a `TextButton` in the error colour, which is not a new pattern --
it is how leaving and deleting a group are already treated in `ChatRoomDetailScreen`.
Screenshot verified on emulator-5554: one tonal button, one red text button, five plain
ones, and a hierarchy a reader can follow.
**The plan was wrong about disabled FABs, and the code was right.** It said five screens
should stop hand-computing a container colour from a `can…` flag and pass `enabled`
instead. **No `FloatingActionButton` overload in material3 1.10 takes `enabled`** -- checked
in the source, zero matches for `enabled: Boolean` in FloatingActionButton.kt -- because
the spec's own position is that an unavailable FAB should not appear at all. Hand-computing
is the only way to show a disabled one.
More to the point, the existing code is already better than the plan assumed: it pairs the
colour with `Modifier.semantics { disabled() }` and a comment saying "looking unavailable
is not being unavailable: without this a screen reader still announces a button it is happy
to press." Left alone, and the plan corrected.
**Eight screens are left for a person.** LandingScreen puts "Sign in" beside "Create
profile", SocialPreconditionScreen puts "Invite a friend" beside "View invites", and six
others do the same. Both members of each pair are filled buttons. Which one is primary is a
product decision about what the screen is *for*, and picking wrong quietly weights a choice
the user is supposed to make freely -- so this is listed in the plan rather than guessed at
here.
**Tests.** 949 pass, 600 jvm over 73 classes and 349 android over 44, unchanged. Both
changes are composition-time rendering, which this repo has no UI test infrastructure to
assert; the device screenshot stands in for it. `:composeApp:compileDebugKotlinAndroid`
builds and the apk runs.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
44bf2a01f0 |
feat: give the app somewhere to report an outcome, and every dead-end error a way out
Phase 5, first step, of docs/material-design-conformance.md. Two absences, both structural.
**Sixteen copies of the same dead end.** The tree held sixteen instances of
Column(horizontalAlignment = CenterHorizontally) {
Spacer(Modifier.height(48.dp))
Text("Something went wrong")
}
and five of the same shape saying "No events were found". **Not one of the sixteen offered
a retry.** Every failure in this app named no cause and had no way forward but the back
button.
`ErrorState` and `EmptyState` replace all 21. Deliberately plain -- an icon, a line, and
for errors an action when the caller has one to give. `ErrorState`'s `onRetry` is nullable
so that passing null is a *decision* a reader can see, rather than the absence of a
parameter nobody thought about.
`EmptyState`'s message is **required**, with no default, and that is the point of the
change rather than a detail. "No events were found" was shown for five different absences:
nobody you follow, nobody following you, an empty feed, no replies, no search results. A
shared default would have preserved exactly that. They now read "You aren't following
anyone yet.", "Nobody is following you yet.", "Nothing in this feed yet.", "No replies to
this yet." and "Nothing matched that search." -- and `no_events_were_found` is deleted.
**Zero snackbars across 43 Scaffolds.** No `Snackbar`, no `SnackbarHost`, no
`SnackbarHostState` anywhere. Every transient outcome -- an invite failing, a key package
published, a message not sent -- had nowhere to be reported, so the code either said
nothing or navigated away and hoped.
`LocalSnackbarHostState` is a composition local rather than a parameter because of where
the reporting happens: a view model coroutine finishing a call is several composables below
the `Scaffold` that owns the host, and threading the state down would be the same plumbing
repeated 43 times and forgotten on the 44th. One host is provided in `MantraApp`; only one
Scaffold is composed at a time under a NavHost, so the message renders on whichever screen
is on top.
It **throws** rather than defaulting to a detached `SnackbarHostState()`. A default would
make `notify(...)` a silent no-op on any screen that forgot the host, which is precisely
the failure this file exists to end.
**Wired to a real action, not left as infrastructure.** `publishNewKeyPackage` and
`rotateKeyPackage` were fire and forget: you tapped, a coroutine ran, and nothing on screen
changed -- indistinguishable from a tap that missed. Both take an `onDone` and the screen
reports it. Verified on emulator-5554: tapping Publish shows "Key package published" and
the count goes 2 -> 3.
**Externalising the strings made four copy problems visible, which is the argument for
having done it.** With 364 strings in one file rather than scattered through 60
composables, `%1$s Key Packages`, `replying To %1$s` and **three surviving mentions of the
old product name** were sitting in plain sight. All corrected. (They had been fixed once
already and lost: the previous commit reverted the tree to fix an unrelated import bug and
re-ran the extractor over the original text. Worth recording, because it is what a
revert-and-redo costs when a script is the thing being iterated on.)
**And it made the title-case checker stop covering anything.** `m3-title-case.py` scanned
`.kt` files, so when phase 4 moved the strings out it went on reporting zero while the four
above sat in `strings.xml`. It now reads the catalogue too, and that path is verified by
flipping one entry to "Try Again" and watching it fail. Externalising narrows what a source
scan can see; the check has to follow.
**Tests.** 949 pass, 600 jvm over 73 classes and 349 android over 44, unchanged. The state
composables and the snackbar host are composition-time behaviour and this repo has no
Compose UI test infrastructure; what stands in for it is the device run above.
`m3-audit.sh --check` exits 0.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
2117e22d48 |
refactor: make the 40 interpolated UI strings format strings, and assert the argument order
Phase 4, third step, of docs/material-design-conformance.md. `Text("Add chapter to
${uiState.artifact.name}")` becomes a resource holding `Add chapter to %1$s` and a call
passing the expression. 49 call sites. Literals in composables go 76 -> 39;
`stringResource` goes 374 -> 424.
**A silent bug in the previous commit's extractor, found by this one.** Imports were
tested with `statement in source`, and the generated accessors are named after their
strings -- so `import mantra.composeapp.generated.resources.translate` is a *prefix* of
`...resources.translate_into_which_dialect`. The substring test decided the import was
already there, and the compiler reported "Unresolved reference 'translate'" in a file
whose imports looked complete. Both extractors now match whole lines, and the helper
carries the explanation.
**Four filters, each earned by something the dry run got wrong.**
*A template that is only interpolation has nothing to translate.* `Text("$name")` would
have become a resource holding `%1$s` -- longer, slower, and no more localisable than the
code it replaced.
*A leading or trailing space means it is being glued to a neighbour.* " \\u00b7 %1$s" is a
separator. The test has to be on the format string rather than on the literal halves: a
template opening with an interpolation leaves the first part empty and the second starting
with the separating space, which makes "%1$s Key packages" look like a fragment when it is
a whole label.
*`\\uXXXX` and `\\"` are Kotlin syntax, not XML.* Left alone they would have shipped as the
six visible characters of the escape. They are decoded into the resource, which is UTF-8
and can hold `·` directly. `\\n` is **not** decoded, because
StringCatalogueJvmTest shows Compose Resources processes that one and a real newline in an
XML value would be reflowed by the parser.
*A term of a `+` concatenation is still not a string.* Same rule as the plain extractor.
**Three copy problems surfaced only here, because interpolated strings had never been
checked.** `m3-title-case.py` excludes anything containing `$` -- an interpolation is not a
literal -- so `"$count Key Packages"` had been invisible to every pass so far, as had
`"replying To ${…}"`. And a third instance of the old product name, in
`"...once they're on Torch."`. All three fixed. Worth noting as a gap in the checker rather
than a one-off: title case inside a template is still unchecked, and there are 83
concatenation fragments left where it could hide.
**Two new assertions, on the two things a compiler cannot see.** Argument *order* is
decided by where each `${…}` sat, and a transposition compiles and reads plausibly --
"Recovered 3 of 12" against "Recovered 12 of 3" -- so a two-argument and a three-argument
string are asserted end to end. The three-argument one doubles as the check that `·`
was decoded rather than passed through.
**What is deliberately left.** 83 literals that are terms of a `+` concatenation.
Reassembling `"a " + x + " b"` into one format string means deciding what the whole
sentence is, and several are pluralisations -- `(if (n == 2) "event" else "events")` --
which want a real plural resource rather than a format argument, and that is an API choice
rather than a rewrite. `m3-extract-formatted.py --remaining` lists them.
**Tests.** 949 pass, 600 jvm over 73 classes and 349 android over 44, up from 947/598/349.
`:composeApp:compileDebugKotlinAndroid` builds; the debug apk installs and runs on
emulator-5554 through onboarding, the message list and a chat room with its text intact.
`m3-audit.sh --check` exits 0.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
419504c982 |
refactor: move 315 UI strings into the resource catalogue, and prove the escapes survive
Phase 4, second step, of docs/material-design-conformance.md. 251 distinct strings, 315
call sites, from literals inside composables to `stringResource(Res.string.…)`. Literals
in composables go 332 -> 76; `stringResource` goes 0 -> 374.
**The extractor took four attempts, and each failure is why it is checked in.**
*A bare `text = "…"` is not a Compose string.* `text` is an ordinary parameter name and
this tree uses it on data classes: `NavigationUIState.Loading(text = "…")` is not a
composable, and rewriting it failed with "@Composable invocations can only happen from the
context of a @Composable function". So `Text(`/`BasicText(` calls are brace-matched and
only literals genuinely inside one are touched.
*A regex over quote pairs is not a Kotlin lexer.* Matching `"[^"]*"` over a whole file
pairs one string's closing quote with the next string's opening quote, so "literals" came
out as several lines of Kotlin. Restricting the body to one line fixed that and left a
subtler version: `"a ${if (n == 1) "chunk" else "chunks"} b"` has two inner literals
belonging to an outer template, and left-to-right matching lifts them out as strings of
their own. The script decided `"chunk"` and `"note"` were UI text worth translating. It now
scans properly -- on an opening quote, walk forward tracking `${` depth, recursing over
nested literals, and stop at the closing quote at depth zero.
*A fragment is not a string.* `"a " + x + " b"` is one sentence in three pieces, and " b"
is not something a translator can work with -- word order differs between languages. Three
filters, because the fragments hide in three shapes: adjacent to a `+`, leading or trailing
whitespace or no letters at all (", " and ":"), and -- the one that needed a fourth pass --
a pluralisation where the *parenthesis* is adjacent to the `+` and neither literal is:
(if (proposal.eventCount == 2) "event" else "events") +
Testing the line rather than the literal catches those four sites while leaving a genuine
either/or alone: `if (session == null) "Start key ceremony" else "Try again"` has no `+`
and both branches are whole strings.
**Compose Resources is not aapt, and that was a bug this commit nearly shipped.** The
first version escaped apostrophes as `\'` and doubled `%`, which is what android's resource
compiler requires. Compose Resources does neither. `getString(Res.string.don_t_sign)`
returned
Don\'t sign
backslash included, and there are 30-odd apostrophes in this catalogue. Every one of them
would have rendered with a visible backslash, on screens nobody opens often.
What makes this worth a permanent test rather than a fixed script: escape handling is
**partial**, not absent. The same run showed `\n` *is* processed --
"Currently no messages have been shared.\nBreak the ice." comes back with a real newline.
So there is no family rule to rely on, and the next escape somebody adds needs checking on
its own.
`StringCatalogueJvmTest` asserts all three cases through `getString`, which is the
non-composable reader for the same resources and needs no composition. It found the bug
before a device did.
**Names are derived from content**, snake_cased and truncated at a word boundary:
`something_went_wrong`, `add_artifact_to_the_group_library`. The conventional shape for an
automated extraction, with a known cost -- rewording the copy leaves the name slightly
stale. The alternative, naming by *purpose*, needs somebody to read 315 call sites, and a
name asserting the wrong purpose is worse than one that is a little dated.
**1101 dead strings out, 251 live ones in.** The catalogue previously held the phoenix
wallet fork's entire string table with nothing referencing it; it now holds this app's own,
plus `app_name`.
**What is left, and why.** 76 literals: 46 interpolated, which need format placeholders and
an argument order decided per site, and 30 concatenation fragments, which need their
sentences reassembled first. Both are the next commit, and both are jobs where a script
should not guess.
**Tests.** 947 pass, 598 jvm over 73 classes and 349 android over 44, up from 944/595/349 --
three new assertions in one new class. `:composeApp:compileDebugKotlinAndroid` builds, the
debug apk installs and runs on emulator-5554 with its text reading correctly through
onboarding and the message list. `m3-audit.sh --check` exits 0.
`ChronicleApplyJvmTest` failed once during this commit's verification and passed on rerun;
it is the pre-existing 1-in-8 flake filed during phase 3, and nothing here touches
chronicle code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
0304aca62a |
fix: sentence-case every UI string, settle the product name, and empty the dead catalogue
Phase 4, first step, of docs/material-design-conformance.md. M3's style guide is
unambiguous: "All text, including titles, headings, labels, menu items, navigation
components, app bars, and buttons should use sentence-style capitalization. ... Don't use
title case capitalization." The tree was title case throughout.
**100 occurrences across 60 distinct strings**, in two passes, and the second pass is the
interesting one.
The first pass matched `[A-Z][a-z]+( [A-Z][a-z]+)+` in a `text =`, `Text(` or
`contentDescription =` position and found 41 strings, 73 occurrences: "Add Chapter",
"Sign In", "Key Package Management", "Publish New Key Package". Then the audit reported
zero and the app still had "Invite a Friend" on its first screen.
Two holes. The pattern required every word after the first to be capitalised, so anything
with an article in it survived -- "Invite a Friend", "Add to Group", "Name of Artifact",
"Sign in to Npub". And it read one line at a time, so a `Text(` whose literal sat on the
next line was invisible. A whole-file scan allowing lowercase articles found 19 more
strings, 27 occurrences.
**Sample data is deliberately left in title case.** "Steve Biko", "John Doe", "Frank
Talk", "To Kill a Mockingbird", "Man With A Plan", "Woman Of Few Words" are people and
titles of works, and title case is how those are written. The first audit swept them up
and reported 67 offenders where the real number was 41, which is the kind of number that
teaches a reader to ignore the tool.
Also untouched: the KDoc reference to iOS's own "Increase Contrast" setting, which is
Apple's capitalisation of Apple's setting, and `logger.d("Queried Sync")`, which is
written for whoever is reading logcat.
**Two strings changed meaning rather than just case.** "Sign in to Npub" became "Sign in
with an npub" -- npub is a protocol term, lowercase everywhere else in this app, and you
sign in *with* one rather than *to* it. "Lightning Bolt", a content description, became
"Lightning payment": M3's rule for a description is to name the purpose rather than the
picture, and "bolt" is the picture.
**The product has one name now, and it is Mantra.** The launcher label, the desktop window
title, the landing screen and the package all said Mantra; the home screen's app bar said
"Torch" and `composeResources`' `app_name` said "Machankura". The app bar is fixed.
`UserAgent.APP_NAME` still says "Torch" and is left alone on purpose -- it goes on the wire
to relay operators, so it is a network identity question rather than a content one, and a
comment at the call site says so.
**The two destructive actions now say what they do.** "Leave group" and "Delete group" are
`TextButton`s that fire immediately, with no confirmation step and nothing stating the
consequence. M3: "Tell users what will happen if they take an action and how they can undo
it."
Read out of the repository rather than guessed, because saying the wrong thing about a
destructive action is worse than saying nothing. `leaveChatRoom` sets `leftGroupAt` and
posts a line to the room; `softDeleteChatRoom` sets `deletedAt` on the local row and
nothing else. So: "Posts a line to the room saying you left, and lets you delete it from
this device afterwards", and "Removes the room from this device. The messages stay on the
relays and with the other members." The second matters most -- a button labelled "Delete
group" with no qualifier invites the belief that the messages are gone, which is the
opposite of true.
**1101 dead strings deleted.** `composeResources/values/strings.xml` held the phoenix
wallet fork's whole catalogue -- notification channels, electrum settings, swap timeouts
-- and **nothing referenced any of it**. The tree's only two `stringResource` calls are
both commented out, and one of them names an `R.string`, which does not exist in a Compose
Multiplatform resource set at all. Keeping them made the file look like the app's
catalogue while the app's actual 332 strings sat in composables. It now holds `app_name`
and a note about what happens next.
A trap for the next person, recorded in the file: the compose resources plugin reports an
XML comment containing a double hyphen only as "XML file ... is not valid. Check the file
content." XML forbids `--` inside comments, and this commit hit it while writing that
note.
**The audit's check is now a script, for the reason the second pass exists.**
`docs/scripts/m3-title-case.py` scans whole files, allows articles, excludes sample data by
name and skips logger calls. Budget ratcheted to 0. The grep it replaces was wrong in three
ways and reported success anyway, which is worse than not checking.
**Tests.** 944 pass, 595 jvm over 72 classes and 349 android over 44, unchanged. The debug
apk installs and runs on emulator-5554. `m3-audit.sh --check` exits 0. The 332 literals
themselves are the next commit.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
1f24aaf4bb |
fix: give the two single-field screens their initial focus, and check 200% text on a device
Phase 3, final step, of docs/material-design-conformance.md. The tree had **zero** uses of
`FocusRequester`, `LocalFocusManager` or `focusProperties`, so no screen defined where
keyboard focus starts.
**Two places get it, and only two.** M3's flow guidance asks for an initial focus per
screen and, for a dialog, that "focus is set to the dialog component, likely to a specific
interactive element within the dialog such as a text input field":
- `StartDirectMessageToNpubOrNip05Dialog` -- one field and two buttons. Without this the
dialog opens with nothing focused, so a keyboard or switch user tabs in from wherever
focus happened to be.
- The desktop `PassphraseGate` -- the first screen of the desktop app, whose entire
content is one field, and where there is no tap to give it focus. Somebody who opens
the app and starts typing should not have to reach for the mouse first.
The other seven text-field screens deliberately do **not** auto-focus. Requesting focus
raises the software keyboard, and on a screen that leads with content somebody wants to
read -- AddArtifact's chapter list, WriteNewNote's reply preview -- that covers the thing
they came for. M3 asks for the initial focus to be *defined*, not for a field to be
grabbed; on those screens the definition is "the top of the content".
**Large text verified on a device rather than reasoned about.** Two passes:
A static one first, since the failure mode is a fixed height around text. All 23 fixed
vertical dimensions outside `Spacer`s are icons, images and progress indicators -- 12 to
40dp `.size()` calls, a 200dp image, a 180dp `heightIn` cap. Nothing wraps text in a fixed
box.
Then at `font_scale 2.0` on an API 36 emulator, three screens: onboarding, the message
list, and a chat room. All reflow without clipping. The chat room is the useful one --
system messages wrap to two lines and their timestamps and chevrons stay aligned, the
composer keeps its full width, and the transcript stays readable. `font_scale` was put
back to 1.0 afterwards.
The physical device attached to this machine was left alone. `font_scale` is a
system-wide setting and changing it on somebody's actual phone to test an app is not a
reasonable thing to do; a fresh emulator was booted for it instead.
**An unrelated flaky test, measured and left alone.** `ChronicleApplyJvmTest > an answered
catch-up leaves one line, whatever it took to deliver` failed once during this commit's
verification with
expected:<[chronicleRequested, chronicleReceived]> but was:<[chronicleReceived, chronicleRequested]>
and reproduces at **1 failure in 8** consecutive `--rerun` invocations on this tree. Both
transcript lines are written within the same second and the DAO's ordering has no
documented tie-break, so either order can come back. That is chronicle and database code;
nothing in this branch touches it. Whether it is a test bug or a real one -- two lines
swapping places in a user's transcript on reload would be a defect -- wants deciding by
somebody in that code, so it is filed rather than patched here.
**Tests.** 944 pass, 595 jvm over 72 classes and 349 android over 44, unchanged. Focus and
window insets are properties of a running composition; there is no Compose UI test
infrastructure here, and a test asserting the modifier is present would restate the diff.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
831d1c0ad4 |
fix: give every tappable element a real target, and every icon a decided description
Phase 3, second step, of docs/material-design-conformance.md. Two accessibility rules the
tree had no way to hold: M3's 48x48dp touch target and 44x44dp pointer target, and its
requirement that a decorative visual be *annotated* as decorative rather than merely left
undescribed.
**Nineteen `.clickable` chains had no minimum size, and three were text-sized.**
`ArticleCard` and `LiveStreamCardContent` each make an author's name tappable -- a
`labelMedium`, around 16dp tall -- and `LinkPreview` does the same to a `bodyLarge` url
with 2dp of vertical padding. The other sixteen are cards, rows and full-screen boxes that
are already far larger.
`minimumInteractiveComponentSize()` is applied to all nineteen rather than to the three,
because it is a no-op on anything already 48dp and that makes the rule checkable by a
script instead of by measuring. Worth being precise about what it does, since the modifier
is easy to describe wrongly: it reserves 48x48dp of **layout**, not of touch handling --
touch expansion happens at the input layer regardless. Layout is what keeps adjacent
targets from overlapping, what satisfies M3's 8dp separation, and what a mouse pointer on
the desktop build actually has to land on.
**`Clickable.kt` had it built in and moved house.** The vendored ACINQ helper defaults to
`RectangleShape` and `PaddingValues(0.dp)`, so a `Clickable` is exactly as big as its
content -- and its call sites wrap a 20dp emoji and a row of wallet text. It now applies
the modifier unconditionally, before `.padding(internalPadding)`, since a size modifier
after it would re-impose the smaller constraint.
It also stopped declaring `package com.machankura.compose.ui.composable.widgets.buttons`
while living under `press/mantra/`. That is the second of the three package namespaces the
UI was spread across; `Type.kt` was the first.
**Eighteen `contentDescription = null` were indistinguishable from eighteen oversights.**
`null` is the *correct* API -- M3 asks that decorative visuals be "annotated as decorative
in order to hide them in code", and null is how that annotation is spelled in Compose. The
problem is that it reads identically whether somebody decided or never looked.
So `Decorative` is introduced -- a `String?` that is null -- and fifteen sites now say
`contentDescription = Decorative`. Same bytes, same behaviour, and the difference between
a decision and a gap is now visible in the source and countable by the audit. Each of the
fifteen has adjacent text saying what the icon says: a lock beside "Private to Ada", a
check beside "The group has a shared key.", an icon inside a button whose label is right
there.
**Three were not decorative and now carry their state.**
- `DkgRitualScreen`'s participant list -- a filled or empty circle beside each member.
The name says who; only the icon says whether they have contributed. Now "Contributed"
/ "Not yet contributed".
- `DkgRitualScreen`'s round header -- the title says which round and the count says how
far along; only the icon says whether it finished. Now "Complete" / "In progress".
- `ProposalListScreen`'s leading icon, which is the one this commit could not have left
alone: the previous commit took the red away from the failure state on the highlighted
card, because `error` is 2.67:1 there. The shape is now the only cue a sighted user
gets and the description is the only cue anyone else gets. Now "Awaiting your
signature" / "Signed" / "Failed" / "Waiting on others".
Descriptions follow M3's rule -- name the purpose, not the picture, and never the role.
"Contributed", not "green check", and never "Contributed icon", since the role is added
automatically and a screen reader would say it twice.
**Two new checks, replacing one that was asking the wrong question.**
`docs/scripts/m3-touch-targets.py` finds `.clickable` chains with no minimum size,
including chains broken across two lines. The audit used to count `.clickable` outright,
which is not a defect count: a clickable `Card` is fine and a clickable `Text` is not, and
only the modifier tells them apart. The audit also now separates `contentDescription =
null` (untriaged, budget 0) from `Decorative` (decided, reported at 15).
Both budgets ratcheted to 0, dated in the file.
**Tests.** 944 pass, 595 jvm over 72 classes and 349 android over 44, unchanged -- these
are layout and semantics properties, and this repo has no Compose UI test infrastructure to
assert them against a running composition. What stands in for it is the two scripts, which
check the property that *can* be checked statically: that the modifier and the decision are
present at every site. `:composeApp:compileDebugKotlinAndroid` builds, `m3-audit.sh
--check` exits 0.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
4fe47c7d46 |
fix: derive every call-site colour from its container, ending nine contrast failures
Phase 3, first step, of docs/material-design-conformance.md. The generated palette was
already sound -- every `onX`-on-`X` pair in all six schemes clears 4.5:1 -- and every
failure in the app came from a colour reached for at the call site instead of derived from
what it sits on.
**The worst one made the app's most important rows invisible.** `ProposalListScreen` put a
`ListItem` inside a `Card` and overrode only the card's container:
Card(colors = CardDefaults.cardColors(containerColor = primaryContainer)) {
ListItem(colors = ListItemDefaults.colors(containerColor = Color.Transparent),
`cardColors(containerColor = …)` does derive `contentColor = contentColorFor(…)`, so
`LocalContentColor` inside the card was correct. `ListItem` does not read
`LocalContentColor`. Its headline comes from `ListTokens.ItemLabelTextColor`, which is
`onSurface`, and in the light scheme `onSurface` and `primaryContainer` are both `#1B1B1B`.
Measured on that card:
headline (onSurface) 1.00:1 invisible
leading icon (primary) 1.22:1
supporting (onSurfaceVariant) 1.84:1
"could not be read" (error) 2.67:1
onPrimaryContainer 4.61:1 the only one that worked
Four of five below the floor, and the card is applied to exactly `proposal.awaitsYou` --
the proposals waiting on your signature. Dark was fine throughout, because there
`primaryContainer` is black, so this only ever showed in the light scheme.
The card's colours are now computed once and everything inside derives from
`cardColors.contentColor`: the six `ListItemColors` slots, the leading icon tint, the
"Review" label, and the unreadable-count line. `primaryContainer` is kept as the highlight
so this stays a fix rather than a restyle -- `secondaryContainer`, the brand gold, would
read more like "this needs you", and that is a design call recorded in a comment rather
than taken here.
On the highlighted card the failure state loses its red, because `error` is 2.67:1 there.
The signal survives in the icon and in the sentence "could not be read", which is the more
robust cue anyway and the only one available to somebody who cannot distinguish the red.
**`HomeScreen`'s top bar lost its override entirely.** `containerColor = primaryContainer`
with `titleContentColor = primary` is `#000000` on `#1B1B1B`: **1.22:1**, a black title on
a near-black bar. `TopAppBarDefaults` gives `surface`/`onSurface` and needed no help.
**Three of the four `alpha = 0.5f` sites were not text, which changes what they failed.**
The audit called them caption text; they are `CircularProgressIndicator` colours, so the
threshold is 3:1 rather than 4.5:1. At 2.49:1 they fail either way, but the plan said the
wrong thing and is corrected. The one that really is text -- `ArticleCard`'s published-at
timestamp at `alpha = 0.7f`, 3.96:1 -- is the fourth. All five now use `onSurfaceVariant`
at full opacity, 7.25:1, which is the role for secondary text and needed no alpha to
become one.
**The LIVE badge was a hand-mixed red.** `Color(0xFFE53935)` with a white label is 4.23:1,
under the floor for `labelSmall`. `error`/`onError` is the role for a red that has to be
read and is 6.46:1.
**The avatar picker used a content colour as a background.** `onSurface` at 50% composited
to a mid grey 2.49:1 from the unselected cells beside it -- so which emoji was selected was
close to unreadable. Now `secondaryContainer`, M3's role for a selected item. Worth being
straight about the limit: that role is 1.65:1 against the surface in this palette, which M3
accepts because its own selected states carry a second cue, an outline or a checkmark. This
grid has neither. Adding one is component work, and the comment and the plan both say so
rather than leaving it looking finished.
**Three colours stay hardcoded, and each says why at the site.** A new
`// m3-color-exempt: <reason>` marker, matching the spacing convention from phase 2, and
the audit honours it:
- `QRCodeView` -- a QR code is read by a camera. Scanners need maximum luminance
contrast between the modules and their background, and under dynamic colour
`onSurface`/`surface` could be two mid tones and unscannable.
- `FullScreenImageViewer`'s close button -- it floats over an arbitrary photograph, so
no role is safe behind it. A translucent scrim with white on it is M3's own
full-screen media treatment and the only pairing that holds over both a white sky and
a black one.
- `LoadingAsyncImage`'s spinner, but only when a blurhash placeholder is behind it. With
no placeholder the surface is known and the role is used.
Exemptions belong at the call site: the reason travels with the code and a reviewer sees it
in the diff that adds it, rather than in a list of file names in the audit script.
**Two colours were tokenised without moving a pixel.** `Color.Black` on the blank route's
`Surface` and on the image viewer's backdrop are both `scrim`, which is `#000000` in every
one of this app's six schemes. Same bytes, and the value now travels with the theme.
**A new assertion for the case the others structurally cannot catch.** A translucent
container has no contrast ratio of its own -- it has one only once composited -- so
`ColorSchemeContrastTest` grows an eleventh test that composites the two remaining tinted
containers over `surface` and measures the result, in all six schemes, naming the call site
in the failure. The pairings this commit *fixed* are not restated: once the proposal card
derives its colours, the pair it produces is `onPrimaryContainer` on `primaryContainer`,
which the first assertion already walks.
**Audit budget for hardcoded colours ratcheted 9 -> 0**, dated in the file.
**Tests.** 944 pass, 595 jvm over 72 classes and 349 android over 44, up from 942/594/348.
`:composeApp:compileDebugKotlinAndroid` builds, `m3-audit.sh --check` exits 0.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
1bd5ad960f |
docs: record what phase 2 built, and why the audit changed shape
The plan's phase 2 was written before the migration ran and assumed the work was mostly a sweep for off-grid numbers. It was not: 10dp and 20dp dominate the tree and both are already on the M3 scale, so only 89 of 527 were off-grid at all. The section now says what was actually built -- the scale, the two migrations, and the reason the audit's value-based classification had to become a shape-based one -- along with the 0.03% pixel diff that shows the sweep moved nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
8cd0f3f44d |
refactor: take the last 353 spacing literals onto the scale, and reach zero
Phase 2, final step, of docs/material-design-conformance.md. Every `.dp` literal in a
spacing position in the UI tree is now a token. 431 reads of `MaterialTheme.spacing.*`, one
reasoned exemption, and `m3-spacing-positions.py` exits 0.
**Shape decides the token, not just the value.** The migration script grew a per-shape
mapping because the same number means different things in different positions: 8dp of
padding is `compactPadding`, 8dp of gap is `itemGap`, and 8dp under a `Spacer` is neither
of those and stays `space100`. Where the pair determines the meaning the semantic name is
used, and nowhere else:
padding + 8dp -> compactPadding 10 sites
padding + 16dp -> containerPadding 10
gap + 4dp -> relatedGap 8
gap + 8dp -> itemGap 10
That is 38 of 353. The rest take the raw stop, and deliberately: assigning a semantic name
needs somebody to have read what the container *is*, and a name that asserts a meaning the
code does not have is worse than a stop that asserts none. `screenMargin` in particular is
unassignable mechanically -- it is 16dp of padding, exactly like `containerPadding` -- so
it has no call sites yet and gets them when someone reads the screens.
**Two spacers were standing in for zero.** `WriteNewNoteScreen` renders
`Spacer(Modifier.height(1.dp))` twice, in the `LazyColumn` item that shows a reply preview
when there is one. There is nothing to show and the item still has to render something;
1dp was the placeholder. Now `space0`, with a comment, because a 1dp gap that nobody
intended is the kind of thing that gets copied.
**One value is exempt, and says so at the site.** `SovereignWalletStartupScreen`'s
`Spacer(Modifier.height(128.dp))` is room to scroll the last wallet clear of the bottom of
the window -- reserved space, not a step in the rhythm. The scale tops out at `space900`
(72dp) and rounding to it would put the row back under the edge.
Rather than exempt it in the script by value, the classifier now honours an inline
`// m3-spacing-exempt: <reason>` comment on the lines directly above. Exemptions belong at
the call site: the reason travels with the code, a reviewer sees it in the diff that adds
it, and the tool stops accumulating a list of numbers that mean nothing on their own -- the
mistake the first version of this audit made with `DIMENSION_EXEMPT`.
**Where the tokens landed.** `space125` (10dp) 128 times and `space250` (20dp) 107 -- the
two values that already dominated the tree, now named. `space600` (48dp) 52 times, which is
the empty-state spacer from the previous commit. The long tail is 2, 4, 6, 12, 14, 16, 24,
32, 40 and 64dp.
**Verified that nothing moved.** The landing screen was captured on emulator-5554 before
and after and compared pixel by pixel on a 4px grid: **47 differing samples out of
162,000, 0.03%**, and they are the status bar clock. The sweep is a rename.
**Budget ratcheted 353 -> 0**, dated in the file. Phase 8 wires `--check` into CI, at which
point a new `.dp` in a `padding()` fails the build.
**Tests.** 942 pass, 594 jvm over 72 classes and 348 android over 44, unchanged --
`SpacingScaleTest` already asserts the scale, and there is nothing to assert about a
call site having been renamed that the compiler does not.
`:composeApp:compileDebugKotlinAndroid` builds, the debug apk installs and runs,
`m3-audit.sh --check` exits 0. 75 files, 432 insertions, 348 deletions.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
f7a732d68a |
refactor: move the 89 off-grid spacing values onto the M3 scale
Phase 2, second step, of docs/material-design-conformance.md. 77 of the 89 literals that
were off M3's spacing scale sat in spacing positions and now read
`MaterialTheme.spacing.spaceNNN`; the remaining 12 are dimensions and are out of scope.
One drifted corner moved onto the shape scale.
**The mapping, and why each is the nearest stop rather than the nicest number.**
5.dp x10 -> space50 (4dp) padding and gaps in dense rows
15.dp x14 -> space200 (16dp) card and dialog padding, two gaps
30.dp x1 -> space400 (32dp) the spacer under LoadingDataIndicator's spinner
50.dp x52 -> space600 (48dp) the spacer above an empty or error message
Nearest-stop throughout, so the largest move is 2dp and most are 1. `5.dp` is equidistant
between `space50` and `space75`; it goes to 4dp because `spacedBy(4.dp)` is already the
idiom elsewhere in the tree and a scale with two answers for the same input is not one.
The 52 at 48dp are the same three lines copied into 16 files -- a `Spacer` pushing
"Something went wrong" down the screen. Phase 5 retires them into a shared empty-state
composable; migrating them first means that composable inherits a token rather than
another literal.
**One shape, and it is the argument for having a scale at all.**
`RoundedCornerShape(30.dp)` in `TextNoteEventDetail` was the only hand-written corner off
the M3 scale, at 30dp against `extraLarge`'s 28. Two units: invisible beside any single
other card, and exactly the drift that happens when the value is a literal. It is now
`MaterialTheme.shapes.extraLarge`, the first call site for the scale `Shape.kt` documented.
**Rewritten by a script that reads call shapes, not values, and it is checked in.**
`docs/scripts/m3-migrate-spacing.py` brace-matches three call shapes -- `padding(...)`/
`PaddingValues(...)`, `Arrangement.spacedBy(...)`, and a `.height()`/`.width()` whose
enclosing call is `Spacer(` -- and rewrites only literals that fall inside one. A
`.size(18.dp)` icon, a non-Spacer `.height()`, a `RoundedCornerShape` or a `BorderStroke`
can never be caught, which a regex over `\\d+\\.dp` would have done to all of them. It
inserts the two imports where they are missing and skips comment lines. Dry run by default.
**The audit was measuring the wrong thing, and this is where that showed.** It split
literals by value against a hardcoded `DIMENSION_EXEMPT` list -- and the split is not a
property of the value. `16.dp` is a spacing stop *and* a plausible icon size. `50.dp` was a
`Spacer` height in 52 places and a divider width in one, and no list of numbers separates
those. `docs/scripts/m3-spacing-positions.py` replaces it with the same brace-matching
parse the migration uses, so the audit and the migration agree by construction; the audit
now reports **353 spacing literals** left and 76 dimensions out of scope, and the exemption
table is gone.
That reframes phase 2's acceptance criterion into something checkable: spacing positions to
zero, dimensions untouched. The script exits 1 while any spacing literal remains.
**What is left off-scale, and why none of it is a defect.** Twelve dimensions: avatar sizes
at 35, 55, 70 and 75dp, icon sizes at 18 and 22dp, and a 50dp divider width. Avatar and
icon sizing is a component-spec question rather than a spacing one -- M3 gives icons 18/20/
24/40/48 and says nothing about avatars -- and the plan puts per-component specs after the
adaptive phase. They are reported rather than exempted so the number stays visible.
**Tests.** 942 pass, 594 jvm over 72 classes and 348 android over 44, unchanged --
this commit adds no assertions, and the ones it could add (`SpacingScaleTest`) landed with
the scale. `:composeApp:compileDebugKotlinAndroid` builds, `m3-audit.sh --check` exits 0.
Pixels move by at most 2dp, in 30 files.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
47bdb6f976 |
fix: promote the two pill colours to extended roles, fixing both contrast failures
Phase 1, step 5 of docs/material-design-conformance.md. `BluePill` and `RedPill` were raw
`Color` values in `Color.kt`, paired at the call site with `Color.White` and
`Color.DarkGray` by eye. Both pairings were below the 4.5:1 floor, and one of them was not
the colour it looked like.
**This step could not leave the pixels alone, and it is the only one so far that changes
them.** `Color.DarkGray` on `BluePill` measures **2.90:1**. `RedPill` was
`Color(230, 32, 32, 191)` -- the four-Int constructor, whose last argument is alpha, so it
is `#E62020` at 0.749. Opaque, white on it is 4.57:1 and passes; composited over the
surface as it actually renders, it is **3.50:1** and does not. Any correct version of these
two buttons is a visible change, so "adds, does not restyle" does not apply here and the
plan already said the call sites would move in this step.
**What M3 asks for here is an extended colour**, not a literal: a brand colour promoted to
a full role family -- `color` / `onColor` / `colorContainer` / `onColorContainer` -- so
that contrast is a property of the family rather than a decision repeated at each use.
`ColorFamily` was already declared in `Theme.kt`, unused, alongside an
`unspecified_scheme`; Material Theme Builder emits both, and this is what they are for.
**Derived by the same rule as the gold palette**, which the fixed-roles commit established
and verified: maximum in-gamut chroma at the source colour's Lab hue, sampled at M3's role
tones. BluePill's hue is 277.0 and RedPill's is 36.3.
role light dark blue light red light
color tone 40 tone 80 #0060AB #C00012
onColor tone 100 tone 20 #FFFFFF #FFFFFF
colorContainer tone 90 tone 30 #D7E2FF #FFDAD3
onColorContainer tone 10 tone 90 #001C39 #390C00
The buttons take `color`/`onColor`: 6.46:1 for the red pill and 6.44:1 for the blue, from
2.90 and 3.50.
**A side effect worth having.** At tone 40 the two pills are the same lightness, so they
now read as a matched pair. Before, `#E62020` sat beside `#5D8DD6` -- a saturated red next
to a soft periwinkle -- and the blue looked like the lesser option. On a screen whose whole
content is "commit, or wipe and leave", weighting one choice by accident is a defect of its
own.
**They travel on a composition local, not on `isSystemInDarkTheme()`.** `ColorScheme` has
no slot for extended colours, so `LocalExtendedColors` is provided by `TorchTheme` from the
same `darkTheme` it chooses the scheme with. Reading `isSystemInDarkTheme()` at the call
site would have been one line shorter and subtly wrong: it ignores a caller who passed
`darkTheme` explicitly, so a preview forcing dark would show light pills. The local
defaults to the light families rather than to `unspecified_scheme` -- nothing composes
outside `TorchTheme` today, and an invisible button is a worse way to discover that than a
light-themed one.
**No medium- or high-contrast variants, deliberately.** The entire surface is two buttons
on one screen, and the light family's weakest pair is 6.44:1 -- clear of the floor by more
than the contrast schemes would add. 32 more values for that would be out of proportion,
and the comment in `Color.kt` says so rather than leaving the omission to be read as an
oversight.
**`QRCodeView` lost its constructor default.** `QRCodeBackgroundPainter` defaulted
`backgroundColor` to `BluePill` -- a colour picked outside the theme for a surface that is
almost never seen, since at the default `padding = 0.dp` the logo painter covers the rect
it fills. The default is gone and the one call site passes it, so the choice is visible
rather than buried.
**Two new assertions, one of which is about the constructor.** `ColorSchemeContrastTest`
grows to 9. The first checks both pairs of every extended family at 4.5:1. The second
checks that every extended role is **opaque**, because `RedPill`'s alpha is what made the
first assertion insufficient: a translucent container has no ratio of its own -- it has one
only once composited -- so a contrast test would have measured a colour the user never
sees. That is the bug this commit fixes, and it would have passed a naive contrast test.
**The audit stopped counting its own commentary.** Fixing these call sites left a comment
*explaining* what `Color.White`/`Color.DarkGray` had been, and `m3-audit.sh` counted it as
a hardcoded colour -- so the file stayed in the report after being fixed. The script now
drops comment lines before counting. Budget ratcheted 11 -> 9: the two real sites, plus the
false positive the filter removes.
**Tests.** 930 pass, 588 jvm over 71 classes and 342 android over 43, up from 926/586/340.
`:composeApp:compileDebugKotlinAndroid` and `:composeApp:compileKotlinJvm` build,
`m3-audit.sh --check` exits 0. The nine remaining hardcoded colours are phase 3's, and are
listed by the audit.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
52b57769e1 |
feat: adopt MaterialExpressiveTheme, and give shape, type and motion a named home
Phase 1, steps 3, 4 and 6 of docs/material-design-conformance.md. `TorchTheme` passed
`MaterialTheme` a colour scheme and a typography and nothing else, so shape and motion were
whatever the library defaulted to and there was nowhere to write down what any of it was
for.
**Expressive, by decision rather than by drift.** The plan deliberately left
`MaterialExpressiveTheme` vs `MaterialTheme` open, because it changes component defaults
app-wide and is a product call. Put to the product owner on 2026-09-08 and answered
expressive. The pinned material3 1.10.0-alpha05 ships the whole set -- `ButtonGroupKt`,
`SplitButtonKt`, `FloatingToolbarKt`, `LoadingIndicatorKt`, `ShortNavigationBarKt`,
`WideNavigationRail`, `MaterialShapesKt` -- and the tree already opts into
`ExperimentalMaterial3ExpressiveApi` in 66 places, so this makes explicit what the imports
had already assumed.
**All four slots are passed explicitly, and that is the point.**
`MaterialExpressiveTheme` defaults its colour scheme to `expressiveLightColorScheme()` and
its shapes and typography likewise -- Material's values, not this app's. Leaving any slot
to that default is the same class of accident as the twelve unassigned fixed roles fixed
two commits ago: it compiles, it renders, and it renders somebody else's design.
**No visual change on the screens checked, and that is worth stating rather than
assuming.** Measured on the API 36 emulator: the "Invite a Friend" button is byte-identical
before and after -- same fill `#4E5E8B`, same 357px box at the same y -- because the
expressive default for a `Button` at default size matches the baseline in this version.
What expressive actually buys is elsewhere: `LocalUsingExpressiveTheme` gating component
behaviour, the three increased shape steps, the fifteen `...Emphasized` type roles, and the
components phases 5 to 7 are built on.
**`MantraShapes` is baseline `Shapes()`, on evidence.** The corners hand-written across the
tree already land on the M3 scale --
RoundedCornerShape(4.dp) x3 = extraSmall
RoundedCornerShape(12.dp) x11 = medium
RoundedCornerShape(16.dp) x2 = large
RoundedCornerShape(30.dp) x1 ~ extraLarge (28dp)
-- so overriding the scale would restyle the app for no reason. What is wrong is that they
are literals, which is how the last one drifted two units off the scale and why none of
them can move per breakpoint later. `Shape.kt` documents the eight steps and what each is
for; migrating those seventeen call sites is a later phase, and this is what they migrate
onto. Declaring it explicitly rather than relying on the default gives the note somewhere
to live.
**`MotionScheme.expressive()` is wired and unused.** Nothing in the app animates today --
one `animateScrollToPage`, no `AnimatedVisibility`, no navigation transitions -- so this
buys nothing yet. It is here so that when the motion phase starts, every spec comes from
the scheme rather than from a literal `tween`, and the app's feel is one decision instead
of forty.
**`Type.kt` left `com.example.ui.theme`.** It has been declaring that package while living
under `press/mantra/compose/ui/theme/`, one of three namespaces holding live UI code in
this tree. The move is mechanical; the doc comment on it is not. It records what each type
family is *for* -- `display*` for a screen's identity, `headline*` for section tops,
`title*` for headers and list headlines, `body*` for anything read as a sentence, `label*`
for **component text only** -- because the audit's finding is not that the scale is wrong
but that 92 of 240 reads are `label*` while `display*` and `headline*` carry 9 between them
across 43 screens. A UI at one pitch. The file stays baseline; the rule now has a home for
the sweep that fixes the call sites.
**The desktop unlock screen renders in the app's theme for the first time.**
`PassphraseGate` sat in the `else` branch beside `MantraApp`, which applies `TorchTheme`
itself -- so the gate composed under the default `MaterialTheme` and its
`colorScheme.error` and `typography.headlineSmall` were baseline M3. It is the first screen
a desktop user sees. `TorchTheme` now wraps both branches.
That wraps the unlocked branch twice, deliberately. `MantraApp` keeps its own `TorchTheme`
because android and ios enter through it and would lose the theme entirely if it moved out;
a second application of identical values costs one `CompositionLocalProvider` composition.
The comment says so, since the redundancy looks like an oversight.
**Dynamic colour stays on, by decision.** Also put to the product owner: on Android 12+
`dynamicColor = true` wins unconditionally, so the six schemes are used only below Android
12, on ios and on desktop, and a modern phone paints the wallpaper palette. Answered keep
as-is. A comment on the selection in `TorchTheme` now says this outright, because otherwise
the next person to change `Color.kt` and see nothing happen on their phone will assume the
change did not work.
**Tests.** 926 pass, 586 jvm over 71 classes and 340 android over 43, unchanged -- this
commit adds no assertions, because what it changes is either a library default (nothing to
assert that the compiler does not) or a doc comment. `:composeApp:compileDebugKotlinAndroid`
and `:composeApp:compileKotlinJvm` build, the debug apk installs and runs on emulator-5554
under the expressive theme, `m3-audit.sh --check` exits 0.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
21d57eba54 |
feat: honour the platform's contrast setting, reaching four schemes that were dead code
Phase 1, step 2 of docs/material-design-conformance.md. `Color.kt` has carried medium-
and high-contrast variants of both themes since it was generated -- 156 colour values,
wired into `lightColorScheme`/`darkColorScheme` in `Theme.kt`, and never selected.
`TorchTheme` chose between `darkScheme` and `lightScheme` and nothing else, so a user who
turned contrast up in Accessibility settings got no change at all.
M3's accessibility foundation leads with *honour individuals*: "supporting varying
preferences and choices that allow individuals to address how their changing conditions,
individual knowledge, and varying needs are met." The work to do that was already done and
disconnected.
**The expect/actual boundary moved, because it was in the wrong place.** `themeColorScheme`
took four arguments and did two unrelated jobs -- decide the contrast-free light/dark
scheme, and decide whether to prefer a wallpaper palette. Adding contrast to it would have
meant passing six schemes across the boundary and repeating the selection table in three
actuals. It splits instead into `platformThemeContrast()` and `dynamicColorScheme()`, each
answering one narrow platform question, with the six-way table as a plain function
`appColorScheme(darkTheme, contrast)` in common code. `dynamicColorScheme` returns null
rather than falling back internally so the fallback stays in one place.
**Android reads the setting and listens for changes.** `UiModeManager.getContrast()` is
API 34; the app's minSdk is 26, so below that the answer is Standard. The float is snapped
to the nearest of the platform's three documented positions rather than matched exactly, so
a future finer-grained slider degrades to the closest scheme this app has instead of
falling back to Standard.
The `ContrastChangeListener` is the part that is easy to leave out and matters most. A
contrast change does not restart the activity and does not arrive as a `Configuration`
update, so without it the new setting would take effect on the next cold start -- which is
precisely the case the setting exists for. `context.mainExecutor` rather than
`ContextCompat.getMainExecutor`: it needs API 28, this branch is already gated on 34, and
composeApp does not declare androidx.core -- it only arrives transitively through
activity-compose, which is not a dependency to lean on.
**iOS observes the notification for the same reason** --
`UIAccessibilityDarkerSystemColorsEnabled` plus
`UIAccessibilityDarkerSystemColorsStatusDidChangeNotification`. It is a boolean, not a
slider, so iOS reports High or Standard and never Medium.
**Desktop is honest rather than complete.** Windows publishes high contrast as the
`win.highContrast.on` AWT desktop property and fires a property change when it is toggled,
so that path is real and live. macos "Increase contrast" and the linux desktop equivalents
do not reach AWT, and reading them means a native call per platform, so on those two the
answer is Standard and the file says so. This is the right place for a user-overridable
preference later; a desktop app cannot always see what the desktop was told.
**Verified on an emulator, at the pixel.** API 36, dynamic colour temporarily switched off
(see below for why that is necessary), sampling the `onPrimaryContainer` pixel of the "Skip
for now" label as `settings put secure contrast_level` moved:
standard (0.0) #848484 onPrimaryContainerLight
medium (0.5) #A7A7A7 onPrimaryContainerLightMediumContrast
high (1.0) #D0D0D0 onPrimaryContainerLightHighContrast
The three declared values exactly, and **the app was not restarted between them** -- only
the setting changed, four seconds apart. That is the listener working end to end. The probe
that switched dynamic colour off is reverted in this commit; the emulator's contrast_level
is back at 0.0.
**A finding that came out of the verification, and is not fixed here.** `TorchTheme`
defaults `dynamicColor = true`, and on Android 12+ dynamic colour wins unconditionally --
so on essentially every current Android device **none of the six schemes is used at all**
and the app renders in whatever the user's wallpaper produced. The first screenshot of this
session shows the onboarding screen in Material lavender; switching dynamic colour off
reveals the black-and-gold brand for the first time. Nobody on a modern Android has been
seeing this app's palette.
That is a product decision, not a conformance one, so it is recorded in the plan's "What
this plan does not cover" rather than changed. It does bound what this commit buys: on
Android 14+ with dynamic colour on, contrast is honoured by the platform anyway (the
`system_*` resources shift with it, confirmed on the same emulator -- buttons went
slate-blue to near-black navy). What this commit reaches is Android below 12, Android 12-13,
iOS, and desktop.
**Three new assertions.** `AppColorSchemeSelectionTest` covers the table itself, because its
failure mode is silent and specific: a scheme wired to the wrong cell still renders a
complete, plausible UI, and somebody who turns contrast up and gets the medium scheme back
cannot tell it apart from a high-contrast scheme that is not very high. It asserts each of
the six cells by identity, that all six are distinct objects (a copy-paste leaving two cells
on the same scheme would pass the first test only if it also mislabelled one), and that
`onSurface` on `surface` never *falls* as contrast rises -- the one direction that must
hold, and deliberately not the full monotonicity assertion that ColorSchemeContrastTest
explains is false.
**Not compiled: the iOS actual.** The ios targets are declared only on macos (see
docs/jvm-target.md), so `Theme.ios.kt` is written against the UIKit and Foundation bindings
rather than checked by a compiler. Its file comment says so. The android and jvm actuals of
the same two functions are compiled, and the android one is verified on a device.
**Tests.** 926 pass, 586 jvm over 71 classes and 340 android over 43, up from 920/583/337.
`:composeApp:compileDebugKotlinAndroid` and `:composeApp:compileKotlinJvm` build,
`m3-audit.sh --check` exits 0. The 54 existing `TorchTheme { }` call sites are untouched --
the new parameter is defaulted.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
86c9628eee |
fix: assign every ColorScheme role, so no component can fall back to Material lavender
Phase 1, step 1 of docs/material-design-conformance.md. `Theme.kt` assigned 36 of the
49 roles `androidx.compose.material3.ColorScheme` declares. The other thirteen took
`lightColorScheme()`/`darkColorScheme()` defaults, and for twelve of them that default
is the Material baseline palette: `primaryFixed` -> `ColorLightTokens.PrimaryFixed` ->
`PaletteTokens.Primary90` -> **#EADDFF**. Lavender, in an app whose primary is
`#000000`, in both themes, in all six schemes.
Nothing in the tree reads a fixed role today, which is why nobody has seen it. That
also means it could not have been found by looking at the app -- it springs the first
time an expressive component reaches for one, and it will look like a rendering bug
rather than a missing assignment.
**The tones were computed, not chosen.** M3 defines the family by tone: `xFixed` =
tone 90, `xFixedDim` = 80, `onXFixed` = 10, `onXFixedVariant` = 30, and ColorLightTokens
and ColorDarkTokens carry identical values for all twelve -- theme-independence is what
"fixed" means. Tone is CIE L*, so for a chroma-0 palette a tone is exactly the sRGB grey
at that L*, and inverting L* -> Y -> sRGB reproduces this palette's own greys **to the
byte**:
tone 0 #000000 primaryLight
tone 10 #1B1B1B primaryContainerLight, onSurfaceLight
tone 20 #303030 onPrimaryDark, inverseSurfaceLight
tone 40 #5E5E5E inversePrimaryDark
tone 80 #C6C6C6 primaryDark, inversePrimaryLight
tone 90 #E2E2E2 onSurfaceDark, surfaceContainerHighestLight
tone 95 #F1F1F1 inverseOnSurfaceLight
tone 100 #FFFFFF onPrimaryLight
Eight independent hits. The primary and tertiary palettes are the standard M3 neutral
tonal palette at chroma 0, so their fixed families are derived rather than invented.
**The secondary palette is gold at Lab hue 87.5 degrees, and its dark half is maximum
in-gamut chroma at that hue.** Generating tones off that ramp regenerates
`onSecondaryDark` (#3D2F00, tone 20) and `secondaryLight` (#745B00, tone 40) byte for
byte, which is what licenses using it for tones 10 (#241A00) and 30 (#584400).
Its tones 90 and 80 are **reused rather than regenerated**. The palette already ships
#FFDE82 at tone 90 (as `secondaryDark`) and the brand gold #EFBF04 at tone 80 (as
`secondaryContainer`, identical in light and dark -- someone hand-set it, no generator
emits that). Regenerating would have produced #FFDF99 and #F1C100: a second gold two
units from the one already on screen, indistinguishable in isolation and wrong beside
it. A near-duplicate brand colour is worse than none.
**Sanity check on the whole derivation.** The four ratios these families produce land
within 0.1 of M3's own baseline fixed family --
onFixed on Fixed 13.30 (baseline 13.32)
onFixedVariant on Fixed 7.17 (baseline 7.23)
onFixed on FixedDim 10.08 (baseline 10.08)
onFixedVariant on Dim 5.44 (baseline 5.47)
-- because tone, not hue, sets the ratio. Two palettes with nothing in common landing
on the same four numbers is the check that the tone mapping is right.
**Containers hold across the contrast setting; content darkens.** That is the move
`Color.kt` already makes everywhere else -- `onSurfaceLight` goes #1B1B1B -> #111111 ->
#000000 while `surfaceLight` stays #F9F9F9 through all three -- so the fixed family
follows it: content tones 10/30, then 5/20, then 0/10. The weakest pair ladders
5.44 -> 7.73 -> 10.08. Shifting the containers instead would have moved the brand-visible
half for a setting that is about legibility.
**`surfaceTint` is the thirteenth, and it was never a defect.** Its default is `primary`,
which is correct: `surfaceColorAtElevation` composites it over `surface` at 2-8% alpha,
so an elevated light surface darkens toward primary and an elevated dark one lightens --
M3's own behaviour, and this app sets no elevations anywhere, so nothing reads it. It is
assigned explicitly anyway, with that reasoning in a comment, so that "every role is
assigned" is a property a reader can check by looking rather than by knowing which
omissions were deliberate. m3-audit.sh reports the two kinds apart for the same reason.
**Three new assertions, and the two that matter cannot be satisfied by accident.**
`ColorSchemeContrastTest` grows from 4 to 7:
- both content roles on both fixed containers at 4.5:1, across all six schemes;
- the fixed roles are the same colour in light and dark, which is the definition and
would otherwise only fail on a screen that puts one beside a themed surface;
- no role is left at the Material baseline palette -- the twelve baseline hex values
read out of `PaletteTokens.kt` and asserted absent.
Verified by deleting `primaryFixed = primaryFixed,` from `lightScheme` alone: two tests
fail, naming the role and printing back `Color(0.917, 0.866, 1.0)`. Reverted.
**Audit budget ratcheted 12 -> 0**, dated in the file. Per the header's contract that is
the only direction a budget moves, and the commit that lowers it is the one that earns it.
**Tests.** 920 pass, 583 jvm over 70 classes and 337 android over 42, up from 914/580/337
-- three new assertions counted once per target. `:composeApp:compileDebugKotlinAndroid`
builds, `m3-audit.sh --check` exits 0. No visual change: every role that had a value keeps
it, and the thirteen that gain one were rendering baseline defaults nothing reads yet.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
2b0ce8d73b |
test: measure M3 conformance instead of asserting it, with a budgeted audit and a contrast test
Phase 0 of docs/material-design-conformance.md. Every count in that document was produced by hand, which makes the eight phases after it opinions rather than work with acceptance criteria. This is the harness that turns them back into numbers. **`docs/scripts/m3-audit.sh` regenerates the whole audit, and can fail a build.** Plain invocation reports; `--check` exits 1 when a budget at the top of the file is exceeded. The budgets are the tree as it stands -- 11 hardcoded colours, 33 bare `.clickable`, 18 null content descriptions, 12 unassigned colour roles -- and the contract written into the header is that they ratchet **down**, in the same commit that earns the reduction, and are never raised. Counts a phase has not reached yet are `-1`, which reports but never fails. Phase 8 wires `--check` into CI, at which point a raised budget is the diff a reviewer is looking for. Verified both directions: `--check` exits 0 on the clean tree, and appending a single `Color(0xFF00FF00)` to LoadingScreen.kt makes it exit 1 naming the budget. **Two counts are reported apart from each other on purpose.** Thirteen ColorScheme roles are never assigned in Theme.kt, and reporting that as one number would overstate it. Twelve are the `*Fixed*` family, which default to `ColorLightTokens.PrimaryFixed` -> `PaletteTokens.Primary90` -> `#EADDFF`, so a monochrome app renders Material baseline lavender the moment anything reads one. The thirteenth is `surfaceTint`, whose default is `primary` -- correct, and not a defect. The script labels the first group "lavender" and the second "not a defect". The `.dp` histogram splits three ways for the same reason. 527 literals: 419 on the M3 spacing scale, 19 dimensions rather than spacing (a 1dp hairline, an avatar, an image height), and 89 genuinely off-scale. The naive split reported 101 off-scale by counting 1dp borders as bad spacing, which would have sent phase 2 chasing hairlines. `DIMENSION_EXEMPT` is deliberately short and the header asks for a justification in the commit that lengthens it. **`ColorSchemeContrastTest` walks the real schemes, which cost a visibility keyword.** Four assertions over all six declared schemes: every content role on its container at 4.5:1, `onSurface` on each of the seven tonal surfaces at 4.5:1, `outline` against every surface it is drawn on at 3:1, and `primary`/`error` against `surface` at 3:1. WCAG relative luminance from first principles -- the 0.03928 knee and the 2.4 exponent, not a gamma-2.2 approximation, because the approximation moves borderline pairs by enough to change a verdict and the tightest pair in this tree is 4.56:1. `Theme.kt`'s six schemes went from `private val` to `internal val` so the test can see them. The alternative -- rebuilding the schemes inside the test from `Color.kt`'s public values -- keeps production visibility untouched and was rejected: it would assert the palette and miss the wiring, and the wiring is the half that fails silently. `surfaceContainerHigh = surfaceContainerHighestLight` is a one-character slip, compiles, and reads fine in review. A comment above the first scheme says this, so the keyword is not quietly widened back. **Verified that it bites.** Nudging `onSurfaceVariantLight` from `#4C4546` to `#9C9496` -- a plausible "soften the secondary text" edit that nothing else in the build would object to -- fails with `light: onSurfaceVariant on surfaceVariant is 2.29:1`, naming scheme, pair and ratio. Reverted; the committed value is unchanged. **Monotonicity across the contrast ladder is deliberately not asserted.** The obvious invariant -- high-contrast beats medium beats default for every pair -- looks right and is false. Ten pairs move the other way, and correctly: in the light high-contrast scheme `surfaceContainerHighest` goes darker to separate it from `surface`, which drops its ratio against `onSurface` from 13.30 to 12.29 while raising the separation that the change exists for. `onErrorContainer on errorContainer` drops 7.24 -> 5.19 from default to medium for the same kind of reason. Asserting the ladder would have meant either a red test or nine exemptions; the floor is the real invariant and every one of those values is comfortably above it. The test's doc comment records this so the next reader does not add the assertion. **Also not asserted: `outlineVariant`, and the call sites.** `outlineVariant` reads 1.61:1 against surface, which looks alarming and is not a defect -- M3's own baseline sits in the same range and the role is a decorative divider, so `outline` is what gets the 3:1 assertion. The seven call-site pairings that are genuinely below threshold, including the 1.00:1 one in ProposalListScreen, belong to phase 3; adding them now would mean checking in a red test. **Doc reconciled to the script rather than the other way round.** Three hand counts were wrong and are corrected in docs/material-design-conformance.md: 520 `.dp` literals -> 527 (the earlier figure omitted the exempt dimensions), 90 `label*` typography uses -> 92 (it missed `labelSmallEmphasized` and `labelLargeEmphasized`, which are label roles too), and 101 off-scale -> 89. The phase 0 section is rewritten from a plan into what was built, including what was decided against. **Tests.** 914 pass, 580 jvm over 70 classes and 334 android over 42 classes, up from 906/576/69 and 330/41 -- the four new assertions, in one new class, counted once per target because commonTest flows into both. `:composeApp:compileDebugKotlinAndroid` builds. No app behaviour changes: the only production edit in this commit is `private` -> `internal` on six vals. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
1c02b25a07 |
docs: measure the UI against the M3 foundations, and phase the work that follows
A plan, not a change: what m3.material.io/foundations asks for as of its May 2026
revision, what these 43 screens actually do, and eight phases ordered so that each
one makes the next mechanical rather than judgemental.
**The spec was read, not remembered.** m3.material.io is a client-rendered SPA --
WebFetch returns an empty `<main>` and the tab URLs 404 on direct navigation -- so
the numbers here came out of a real browser session clicking through the tab
controls. That mattered: the May 2026 revision renamed window size classes to
**breakpoints** and there are now five of them rather than three (compact / medium
/ expanded / large / extra-large, at 600 / 840 / 1200 / 1600dp), renamed responsive
design to adaptive design, and published the spacing system as tokens on an 8dp
scale where `space100 = 8dp`. Writing this from memory of older M3 would have
produced a plan against a vocabulary the current spec no longer uses.
**The palette is fine; the call sites are not.** Every `onX`-on-`X` pair in all six
declared schemes clears 4.5:1, the tightest being `onPrimaryContainer` on
`primaryContainer` at 4.61:1 light and 4.56:1 dark. So the generated scheme is not
the problem and this plan does not propose a repalette. What fails is colour
decided locally, seven pairings of it, and the worst is not visible to a reviewer:
Card(colors = CardDefaults.cardColors(containerColor = primaryContainer)) {
ListItem(colors = ListItemDefaults.colors(containerColor = Color.Transparent),
`cardColors(containerColor = ...)` does derive `contentColor = contentColorFor(...)`,
so `LocalContentColor` inside the card is correct. But `ListItem` does not read
`LocalContentColor` -- its headline comes from `ListTokens.ItemLabelTextColor`,
which is `onSurface` -- and the call site overrides only `containerColor`. In the
light scheme `onSurface` and `primaryContainer` are both `#1B1B1B`. That is
**1.00:1**, and it is applied exactly to `proposal.awaitsYou`, so the proposals
waiting on your signature are the ones rendered invisible. `HomeScreen`'s
`titleContentColor = primary` on `containerColor = primaryContainer` is the same
mistake at 1.22:1. Ratios were computed rather than eyeballed; the script is in the
Phase 0 deliverable.
**Twelve colour roles fall through to Material baseline lavender.** `Color.kt`
never assigns `primaryFixed`, `primaryFixedDim`, `onPrimaryFixed`,
`onPrimaryFixedVariant` or the secondary/tertiary equivalents, so
`lightColorScheme()` defaults them to `ColorLightTokens.PrimaryFixed` ->
`PaletteTokens.Primary90` -> `#EADDFF`. Nothing reads them today, which is why it
has never been noticed; the trap springs the first time an expressive component
does. Read out of the pinned `material3-desktop-1.10.0-alpha05-sources.jar` rather
than assumed.
**Four of the six declared schemes are unreachable.** The medium- and high-contrast
variants are written out in full in `Color.kt` -- 78 colour values -- wired into
`lightColorScheme`/`darkColorScheme` in `Theme.kt`, and then never selected:
`TorchTheme` chooses between `darkScheme` and `lightScheme` only. The work to
honour a platform contrast setting is already done and disconnected.
**10dp and 20dp are not the problem they look like.** They are the two dominant
spacing values (132 and 115 uses) and both are *on* the M3 scale, as `space125` and
`space250`. The plan says so rather than proposing a sweep that would change
nothing. What is wrong is that none of the 520 `.dp` literals records whether it is
padding, a gap or a margin -- the three categories the spec gives different rules
to -- so nothing can be adapted per breakpoint later. About 101 are off-scale
(50dp x 53, 15dp x 14, 5dp x 10 and so on), and `Modifier.height(50.dp)` appears 49
times as the same copied spacer above the same copied error message.
**Findings that were measured and then dropped.** `outlineVariant` reads 1.61:1
against surface and `secondaryContainer` 1.65:1, both of which look alarming and
neither of which is a defect: M3's own baseline sits in the same range, and the 3:1
rule the spec gives is for clustered interactive containers, not dividers or tonal
surfaces. `onSurface.copy(alpha = 0.38f)` is the specified disabled opacity and the
spec exempts disabled states from contrast entirely. Reporting these would have
padded the count and cost the reader trust in the rest.
**The rest of the audit, in counts.** 334 string literals in composables against 2
`stringResource` calls, with title case throughout ("Edit Profile", "New Chat") where
the style guide asks for sentence case. Zero `Snackbar` across 26 `Scaffold`s. 16
copies of `Text("Something went wrong")`, none of which offers a retry. 90 of 240
typography reads on `label*` roles, which are for component text, while `display*`
and `headline*` carry 9 uses between them across 43 screens. 33 bare
`Modifier.clickable` with no minimum target, two of them text-height. Two
`BoxWithConstraints` and no window-size handling at all, on a project with a desktop
target whose own entry point already says so in a comment.
**Eight phases, ordered by what each unblocks.** 0 baseline harness, 1 theme,
2 spacing tokens, 3 accessibility floor, 4 content, 5 states and feedback,
6 adaptive layout, 7 motion, 8 guard rails. Tokens come before the call sites that
consume them; the accessibility floor comes before the adaptive work that would
otherwise double the surface to fix; guard rails come last so they lock in real
state rather than aspiration. Phase 6 is the only one that cannot be done
mechanically and the only one marked not reversible alone.
**What it deliberately does not decide.** Whether the target is
`MaterialExpressiveTheme` or `MaterialTheme` -- the pinned material3 ships the full
expressive set and the code already opts into `ExperimentalMaterial3ExpressiveApi`
in 66 places, but it changes default component shapes and sizes app-wide, so it is a
product call and Phase 1 raises it rather than answering it. Also out of scope:
whether the monochrome palette is right, the per-component specs, iOS (which only
builds on a mac, and whose HIG asks 44dp where M3 asks 48dp), and the three package
namespaces the UI currently lives across.
No code changes. `docs/README.md` gains the row and the closing paragraph's note on
how this one relates to the others.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
98f766fcd3 |
fix: put the invite in the room, so a stuck one can be seen
Inviting a member to a group that already had members put nothing whatsoever in
the transcript. Not "put it in late" -- nothing, and nothing ever if the invite
did not complete. So the one failure the user is best placed to notice, an
invite that never reached the person it was made for, was the one the app kept
to itself.
The line existed. It was written by `MarmotOutboundDao.deliveryWelcome`, which
is the wrong place for it, and the reason is the two paths through
`inviteMember` that docs/marmot-membership.md already describes. A group that is
still only its creator has nobody to inform, so its Welcome goes out immediately
and `deliveryWelcome` runs inside the invite. A group that has members must
broadcast a commit first, and its Welcome waits for a relay to acknowledge it --
`DatabaseNostrRepository.broadcastProcessed` picks the stored `MarmotCommitResult`
back up and delivers then. Every invite after a group's first therefore wrote
its transcript line a relay round trip away from the invite, if at all.
**Four separate silences, not one.** Worth listing because only the first is
about the deferral, and fixing that alone would have left the other three:
1. The deferred path wrote nothing until the ack, and nothing ever without one.
2. The write hung off `getMarmotKeyPackageById(...)?.let { getProfileByPublicKey(...)?.let { ... } }`.
Those two lookups were there to *name* the invitee, and a miss on either cost
the whole line rather than just the name.
3. `deliveryWelcome` wraps its body in `catch (e: Throwable) { logger.e(...) }`
and returned Unit, so a Welcome that could not be built reached the log and
no further.
4. `inviteMemberToChatRoom` is `@Transaction`. An invite that threw -- no MLS
state for the room, a credential identity that does not match the peer --
rolled its line back with everything else, which is right, and left no
account of the refusal anywhere durable.
And the line it did write was `messageType = "message"`, `isUserMessage = true`,
so it rendered as a chat bubble: "Invited Bob to chat", attributed to the
inviter as something they said.
**Three membership types, and the line moves to invite time.**
`ChatMessage.MEMBERSHIP_TYPES` -- `memberInvited`, `memberInviteSent`,
`memberInviteFailed` -- rendered by the transcript as system notices through
`RitualNotice`, the way the ceremony, signing and chronicle lines already are.
`memberInvited` is written by `inviteMember` and by `addMembersToChatRoom`'s
batch path, *when the invite is made*, and deliberately **inside** the caller's
transaction. Both halves of that matter and they pull opposite ways: written any
later and an invite waiting on an ack that never comes shows nothing, which is
the bug; written outside the transaction and an invite that does not survive
`addMember` leaves the room claiming one was made.
`memberInviteSent` is written by `DatabaseNostrRepository` alone. It is not
written on the immediate path, and that is not an oversight: there the Welcome
goes out in the same breath as the invite, so one line is the whole truth. It
would also be a line the transcript could not order -- `MantraConverters` stores
`Instant` as `epochSeconds`, the room query is `ORDER BY createdAt DESC`, and two
rows written in the same second tie. Only the deferred path separates the two
events in time, so only it owes a second line.
`memberInviteFailed` carries the reason, because it is the only copy the user
gets. `deliveryWelcome` now returns `Boolean` and files this line from its own
catch before returning false -- its callers had no other way to see a failure it
had already swallowed, and on the deferred path there is no invite screen left
to fail back to. `addMembersToChatRoom` reads that answer instead of a
`runCatching` that could never catch anything.
**The refusal is written from outside the transaction that rolled it back.**
`DatabaseChatRepository.inviteMember` catches, calls `announceInviteFailed`, and
rethrows. The throw is what puts a message on the invite screen now; the line is
what is still there tomorrow. Swallowing it instead would have popped the user
back to the chat as though the invite had gone out, which is the bug the
existing `runCatching` in `AddMemberToChatRoomConfirmationViewModel` was added
to stop.
**No schema change.** `messageType` is a free-form string column with a default,
so new values need no migration -- unlike the chronicle rename, which had to
rewrite the ones already stored. Nothing reindexes these either: they carry no
`marmotGroupEventId`, so `getResolvedMarmotGroupEventIds` cannot see them and
`UNRESOLVED_MARMOT_TYPES` does not name them.
Six new tests. Four on the DAO: the immediate path leaves a line naming the
invitee where the old code left none, the deferred path leaves one *and* claims
no Welcome sent before any ack, everything an invite writes is a membership type
rather than something the transcript would render as a bubble, and a refused
invite leaves no claim that one was made. Two new ones on
`DatabaseChatRepository`, which had no test file: a refused invite is written
into the room, and the caller still gets the throw.
Still open, and now said plainly in the doc rather than implied: a Participant
row carries no state saying where its invite got to. The transcript narrates it;
the `TODO: Update status of participant Invitation.PENDING -> Invitation.SENT`
is untouched. Nor does an invitee with no published key package reach the room
at all -- that fails in the view model, before there is an invite to write a
line about.
806 tests pass -- 509 jvm, 297 android.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Kgothatso Ngako
|
19d57ef3a5 |
Merge branch 'mantra' into claude/happy-gauss-dbe258
Brings in the Chronicle rename and the deprecation of the row rebuild, and carries the supersession fix across into the new vocabulary. Git followed every rename on its own -- `ArchiveManager` -> `ChronicleManager`, the tests, the docs -- and auto-merged all three files my fix had touched. What it could not do is rename identifiers inside the hunks it merged, so the fix arrived speaking the old language: `ChronicleAssemblyJvmTest` still called `ArchiveManager.assemble` and `ArchiveEvent.decodePage`, which does not compile, and six doc comments in `ChronicleManager` and `GroupSignedEvent` still said "archive" -- the exact ambiguity with archiving a chat that the rename exists to remove. One real conflict, in the design note, and it is the same sentence twice: my correction of "a retranslated passage archives once" against the rename of the uncorrected claim. Resolved to the correction, in the new vocabulary -- the property still holds, it just stopped being free the moment the chronicle was read from `GroupSignedEvent` rather than rebuilt from rows, and `ChronicleManager.currentTranslationsOnly` is what holds it up. `compileKotlinJvm` passes over a test file that does not compile, so it was no evidence here; `compileTestKotlinJvm` is. And the filter was re-checked the way it was written: removing it fails the same three tests, so the merge did not quietly neuter them. 503 jvm tests and 297 android unit tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Kgothatso Ngako
|
4890906b24 |
Merge branch 'mantra' into claude/rename-archive-chronicle-a4a8e0
The rebuild deprecation landed on mantra while the rename was in flight, and it touched the same files by their old names. Git matched the renames itself, so the only conflict was `ChronicleRoundTripTest`'s header, where both sides had rewritten the same paragraph: mantra's says this file is now the gate on a deprecated fallback rather than on the only path, which is the newer and truer claim, so it wins and the rename is applied on top of it. Everything the merge brought in went through the same substitution as the rest: the nine `@Deprecated` messages and the "Retiring the rebuild" checklist all name `ChronicleManager`, `ChronicleRoundTripTest` and docs/member-chronicle.md, which are the files that now exist. 797 tests pass -- 500 jvm, 297 android. The five new ones are the migration's. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |