651 Commits

Author	SHA1	Message	Date
Kgothatso Ngako	5f0afdf34d	feat: render every preview under the five conditions a screen has to survive ... Phase 8, item three. The tree had 51 previews and every one of them rendered one thing: a light theme, at whatever width the preview pane happened to be, at 100% text. That is the only condition under which this app has never had a defect. `@ConformancePreviews` replaces the bare `@Preview` at all 53 sites -- the two outside `ui/` included -- and renders each under five: light, dark, 200% text, compact 400dp, expanded 1000dp. `@Preview` is `@Repeatable`, so this is one annotation rather than five copied onto every preview and drifting apart. Each of the four new conditions is where a defect in this app has actually been: a colour that only fails in dark, a fixed-height container that clips at 200%, a layout that stretches because nothing held it, a row that reflows badly at phone width. It also gives phase 6 the check it could not make. "Every screen renders correctly at 400dp, 700dp, 1000dp, 1400dp and 1800dp" was verified structurally -- the measure applied at every root and asserted at those widths -- but never looked at per screen. Two of those widths are now one click away on every screen in the app. **High contrast is deliberately not in the annotation**, and the argument is worth stating because the omission looks like a gap. Contrast is a property of the *scheme*, not of a screen: the app declares six, `ColorSchemeContrastTest` measures every pair in all six, and a screen right in the default scheme is right in the high-contrast one by construction. Per-screen high-contrast previews would be 51 more renders of something already proved -- and there is no `@Preview` parameter for it in any case, since it needs `TorchTheme(contrast = …)` in the body. `ThemeGallery` covers them instead, once, over components rather than screens: all six schemes side by side, with body copy on surface, a card holding a list item -- the arrangement that rendered a headline at 1.00:1 before phase 3 -- and the three button emphases. `dynamicColor = false` on purpose, or an android 12+ preview paints all six columns from the wallpaper and the gallery shows nothing. It is the only place the medium and high contrast schemes can be seen at all: in the app they are reachable only through a platform setting, and on android only with dynamic colour off. 99 lines changed across 50 files, all of them one annotation and its import. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-08 08:24:49 +02:00
Kgothatso Ngako	9f09f48133	build: make the conformance audit part of check, and gate the branch on it ... Phase 8, the first two items. The audit has existed since phase 0 and has been run by hand at the end of every phase since, which is exactly the arrangement it was written to end: a budget nobody checks at the moment the number moves is a number that drifts. **`:composeApp:m3Audit`, wired into `check`.** It shells out to `docs/scripts/m3-audit.sh --check` and fails the build when a budget is exceeded or a floor is undercut. Verified to bite: adding one `Color(0xFFAABBCC)` to `LoadingScreen.kt` reports `hardcoded Color outside theme/ 1 over budget 0` and takes the build down with it. The task declares the script and the ui source tree as inputs and a marker file as its output, so it is up-to-date-able rather than re-running on every `check`. On a machine with no bash it warns and skips instead of failing, because a build that dies for a reason unrelated to the change under it teaches people to pass `-x`. **A Gitea Actions workflow**, since the remote is a Gitea 1.25 instance. Two jobs, deliberately: - `budgets` is grep over the source tree -- no gradle, no android SDK, no submodules, no network. This job is the reason the audit is a shell script rather than a gradle plugin, and it should stay runnable on a bare container. - `tests` needs a compiler and therefore the whole composite chain: four levels of submodule and a cross-compile of secp256k1's C sources, so a cold run is minutes rather than seconds. Split out so a runner can be pointed at `budgets` alone where that is all the capacity there is. Its two non-obvious steps carry the reasons at the site -- `submodules: recursive` or configuration fails with `Project with path ':library' not found`, and the android SDK is needed even for a jvm-only test run because `:secp256k1-kmp:jni:android` is in the graph. **The workflow is unverified**, and that is worth saying plainly: this repository has had no CI of any kind, so there is no runner registered to try it against. The syntax is valid and the commands are the ones used by hand throughout this work. The gradle task is the half that is proven, and it is the half that runs on every developer machine regardless. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-08 08:19:57 +02:00
Kgothatso Ngako	410ece2df9	feat: fade between a screen's states instead of cutting between them ... Phase 7, the second half. Every screen in this app is a `when` over a UI state -- loading, error, empty, loaded -- and every one of those changes was an unannounced cut: the spinner is there in one frame and the content is there in the next, with nothing saying they are the same screen answering the same question. `ScreenStateTransition` is M3's fade-through, which is the transition for content that replaces other content without being spatially related to it: the outgoing state fades out, the incoming one fades in and grows the last 8% into place. `SizeTransform(clip = false)`, so a tall loaded state does not stretch a short spinner on its way in. Specs from the theme's `MotionScheme`, effects for the fade and spatial for the scale. **The content key is the state's class, not the state.** This is the half that is easy to get wrong and impossible to see: keyed on the value, a screen re-runs the whole fade every time its loaded data changes -- a message arriving, a list growing by one -- so the screen flickers whenever anything happens, and every screenshot of it looks perfect. Keyed on the class, the animation runs when the state does and the data flows through untouched. There is a test for exactly that, and it is the more useful of the two. **Applied to 20 screens, and not to 15 others.** `AnimatedContent` is a layout node, so it can only wrap a `when` that is a composable's whole body. Where the `when` sits inside a `Column` whose branches use `Modifier.weight` -- the sign-in and create-profile flows, the frost signing and proposal screens, the two feed detail widgets, the four render helpers still on view models -- wrapping it would take those branches out of `ColumnScope`. The rule is mechanical, the reason is recorded once in `ScreenState.kt` rather than at each site, and the screens it excludes are named here rather than silently skipped. Reduced motion keeps the crossfade and drops the scale, which is the same position the navigation transitions take: what WCAG 2.3.3 and M3 ask to remove is movement, not the signal that something changed. **Most of this diff is indentation** -- 3,699 lines of it against 157 lines of substance, which is 21 screens gaining a wrapper and one helper being written. `git diff -w` shows the second number. **Verified by holding the clock still and looking at one frame**, which is the only frame that can tell a crossfade from a cut: during a transition both states are composed, and during a cut only ever one is. 639 jvm tests green; android and desktop compile. The audit's motion count goes 11 -> 13. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-08 08:16:05 +02:00
Kgothatso Ngako	61793e2779	feat: give navigation its transitions from the motion scheme, and honour reduced motion ... Phase 7, the first half. All 43 routes took navigation-compose's default, which turns out not to be the hard cut the plan expected: on android and desktop it is `fadeIn(tween(700))` / `fadeOut(tween(700))`, written into the library's own internals. Both halves of that are worth changing. 700ms is roughly three times M3's duration for a full-screen change, and a literal inside a dependency is not a decision this app made -- phase 1 wired a `MotionScheme` into the theme precisely so that there would be one place to make it. **The plan named an API that an app cannot reach.** It says every spec should come from `MotionSchemeKeyTokens`; that enum is `internal` to material3, so the tokens are not addressable by name from outside. `MaterialTheme.motionScheme` is the public surface and offers the same six specs. Two private helpers name which of them this app uses for what -- `defaultSpatialSpec` for the slide, `defaultEffectsSpec` for the fade -- which is the distinction the scheme draws: spatial motion is springy because it moves something, effects motion is not because a fading colour that overshoots looks like a fault. **The shape is M3's shared axis.** The arriving screen slides in from the trailing edge while the leaving one slides out toward the leading edge, both fading; going back mirrors it, so the direction of travel is legible rather than a dissolve that looks the same either way. `slideIntoContainer` is layout-direction aware, so an RTL locale gets the mirror for free. **Reduced motion, on the three platforms, in the shape phase 1 established.** `platformReducedMotion()` is an expect/actual beside `platformThemeContrast()`, observed rather than read once, because somebody who turns it on because motion makes them ill should not have to restart the app. Android has no "reduce motion" switch -- it has **Remove animations**, which sets the animation duration scales to zero. The platform applies that scale to `ValueAnimator` and **not to Compose**, which runs on its own clock and ignores it entirely, so an app that draws its own transitions has to read the setting itself. A `ContentObserver` on `ANIMATOR_DURATION_SCALE` catches the change without a restart. iOS is the one platform where it is a single documented call, `UIAccessibilityIsReduceMotionEnabled`, with the same notification shape as the darker-system-colours one already observed there. Desktop answers `false`, and says at the site why that is honest rather than a stub: Windows, macos and the freedesktop desktops each have the setting and none of the three reaches AWT. That is the same wall `platformThemeContrast` hits on linux and macos, and the same eventual answer -- a preference with the platform as its default. Reduced motion does not mean *no* transition. The screen still fades; what goes is the movement, which is what M3 and WCAG 2.3.3 are both about. **Two tests, and the first one found a design flaw in the second.** The claim "this transition slides and that one does not" cannot be asserted on the values -- `EnterTransition` has no public shape to inspect -- so it is measured: hold the clock, navigate, advance a third of the way, and read where the arriving screen is. Sliding, it is 54dp from home; reduced, it is already there. The reduced case read 54dp at first, because the test provided `LocalReducedMotion` *around* `TorchTheme` and the theme overwrote it. The fix is not in the test: `reducedMotion` is now a `TorchTheme` parameter defaulted to the platform, exactly as `contrast` is, because a value nothing can override is a value nothing can test -- and because the desktop actual is a hardcoded `false` that a settings screen will eventually need to override anyway. 637 jvm tests green; android and desktop compile. The audit's motion count goes 2 -> 11 and navigation transitions 0 -> 3. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-08 08:11:54 +02:00
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>	2026-09-08 08:04:00 +02:00
Kgothatso Ngako	3f78eef1e8	feat: put the chat list beside the conversation, from the expanded breakpoint up ... Phase 6, step 4, chat first as the plan asks. On a window 840dp or wider the home screen is now the room list at a fixed width and the selected conversation filling the rest; on anything narrower it is exactly what it was. **Below expanded is not caution, it is the spec.** The breakpoints page says not to put two dense panes in a medium window, and `calculatePaneScaffoldDirective` in `material3-adaptive` says the same thing in code -- `maxHorizontalPartitions = 1` for compact and medium alike. A chat transcript is precisely the dense content that rule is about. It is also what this app can support. `ChatRoomMessagingRoute` is navigated to from **eleven** places -- a DKG ritual finishing, room-type selection, the npub dialog, a profile -- so the conversation has to remain a pushed destination whatever the window is doing. The list pane is a second way to reach it on a wide window, not a replacement for the first. **Why not `ListDetailPaneScaffold`.** The dependency is available and resolves for every target; the scaffold was not used, and the reason is the paragraph above. It earns its API surface -- a navigator, a destination history, an `AnimatedPane` per pane, three experimental opt-ins -- by owning the single-pane case as well: showing the detail *instead of* the list on a phone and animating between them. This app cannot hand it that, so it would sit permanently in its two-pane state and amount to a `Row` with more words and a history nothing reads. What it does have that is worth keeping is its numbers, and `Panes.kt` takes them: 360dp of list at expanded, 412dp from large upward, 24dp between. A hand-built pair measures the same as the scaffold would. **Three smaller decisions.** The floating action button moves into the list pane when there are two. The `Scaffold`'s slot is the bottom-right of the *window*, which with two panes is on top of the transcript's send button; M3 puts a list-detail layout's primary action in the list pane. It is one composable used from both branches so the two cannot drift. `readableContent()` comes off the pair. Capping two panes together to one column's measure is the opposite of what a second pane is for -- each pane holds its own content instead, and the conversation already did. The detail pane says "Pick a conversation to read it here" rather than being an unexplained empty half of a window, and the conversation is keyed on the room so switching rebuilds its view models rather than feeding a new id to ones already subscribed to another room's relays. **Measured in real windows of the widths the phase names.** 400 and 700 are one pane; 1000 splits with a 360dp list; 1400 splits with a 412dp list. The repositories are the no-op ones with the two reads this screen makes delegated to a fixed answer -- Kotlin's interface delegation makes that ten lines rather than a reimplementation of two large interfaces. Five more unit tests pin the widths against the directive's, including that the detail pane still clears a 40-character line in the narrowest window that allows two of them. 635 jvm tests green; android compiles. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-08 08:01:16 +02:00
Kgothatso Ngako	714354ae9a	feat: give the app a navigation component, and stop the app bar duplicating it ... Phase 6, step 3. The app had no navigation component of any kind: 43 screens reached by pushing a route, and one home screen whose top app bar carried the only two peer surfaces -- a profile avatar in the leading slot, a search icon in the trailing one. **This is an information-architecture change and was taken as one.** With a single top-level destination, a navigation bar would have held one item and been strictly worse than the app bar it replaced -- M3's caution is to swap only functionally equivalent components. Promoting search and profile to peer destinations is what makes a navigation component mean anything here, and it was put to the product owner rather than inferred. Answered: promote them. The consequence is in `HomeScreen`: the app bar now carries a title and nothing else. Two routes to one destination is the thing the caution is about, and the navigation component is now the one route, at every breakpoint. **Which component, at which breakpoint**, straight from the layout foundation: | compact | navigation bar | | medium, expanded | collapsed rail | | large, extra-large | expanded rail | `NavigationSuiteScaffoldDefaults.navigationSuiteType` is not used, and the difference is the last row -- it stops at the collapsed rail, because it classifies with the three-value window size class rather than the five breakpoints the May 2026 revision published. Deriving from `Breakpoint` reaches the row the library's default cannot, and keeps one source of truth for window width in the app. `NavigationSuiteType.None` on everything else. A navigation bar belongs on the destinations it switches between; on a chat room, a signing screen or an onboarding step -- pushed to and left by coming back -- it is a permanent invitation to lose your place. **Two things the wiring needed.** `ActiveProfileRoute` is addressed by metadata event id, not by public key, and only the home screen ever had one. The nav host now observes it for as long as a key is signed in, and the profile item is *disabled* until it arrives rather than absent -- an item that appears late moves the two beside it, and a bar whose items move under a thumb is worse than one briefly unavailable. The item click pops to `HomeRoute`, not to the graph's start destination. The android docs give the second shape and it would be wrong here: this graph starts at `LoadingRoute`, and onboarding clears the stack with `popUpTo(0)` on its way to home, so by the time these items exist the start destination is not on the stack at all -- popping to it would leave the loading screen underneath as the thing back returns to. **Tests, and one that could not be written.** The breakpoint-to-component table is a pure function so all five rows are asserted; the two rail rows differ only in whether labels are drawn, and nobody opens a 1200dp window on purpose. Four more compose the component around a real nav graph, because `TopLevelDestination.of` matches by `hasRoute` -- reflection over the serialized route -- and a renamed route would fail by never showing the component at all. Navigation in those is driven through the controller rather than by tapping an item. That is a harness limitation, established rather than assumed: a click handler that navigates trips navigation-compose's own main-thread assertion under `runDesktopComposeUiTest`, reproducible in twenty lines containing no app code -- a `NavHost`, two routes and a `TextButton`. What an item's `onClick` builds is asserted where it is a pure function instead. Also `material3-adaptive-navigation-suite`, versioned with material3 rather than with the adaptive library: it is published by the material3 group, and its 1.10.0-alpha05 is what names adaptive 1.2.0 in the first place. Most of the `MantraNavHost` diff is indentation -- the `NavHost` call gained an enclosing composable. `git diff -w` shows the 38 lines that are not. 626 jvm tests green; android compiles. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-08 07:53:27 +02:00
Kgothatso Ngako	a53a9c3e11	feat: open the desktop window at a width the layouts are now written for ... Phase 6, step 6. The window opened at 480x900 under a comment that said why: *"The layouts have only ever been exercised at phone widths. This is a starting size that does not immediately misrepresent them, not a considered desktop layout."* That was honest, and it has stopped being true. 1100dp is inside the expanded breakpoint (840-1199), which is the narrowest window M3 recommends two panes in and so the smallest opening size at which a desktop user sees a desktop layout rather than a phone one stretched sideways. The content does not stretch to fill it: screens are held to a readable measure and centred, so the extra width becomes margin. Also a minimum size, which the window never had. Compose Desktop's `WindowState` carries no minimum, so the window could be dragged narrower than anything in the app was written for; 400x600 is the narrowest of the five widths this phase is meant to be checked at, and the compact breakpoint's own floor is a phone rather than nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-08 07:36:38 +02:00
Kgothatso Ngako	e95ece9027	fix: give form helper text and the review list the leading edge of what they describe ... Phase 6, step 5, second half: the plan asked to revisit the 91 `TextAlign.Center` uses, on the grounds that start alignment is what gives the rulers something to align to. Revisited, and 84 of them are right. Centring is correct for a block that is the only thing on the screen, because there is nothing for it to align to: an empty state, a loading or error message, one of the six onboarding status screens, a "coming soon" placeholder, the landing screen's hero, a dialog's title. Converting those would have been a restyle wearing a conformance argument. Seven were wrong, and they share one shape -- text sitting in a column *beside a full-width element*, so there was a leading edge and it was being ignored: - the two helper lines under `CreateProfileScreen`'s name and bio fields, and the two under `ChatRoomCreationScreen`'s. Each `TextField` is `fillMaxWidth()`, and its label, placeholder, leading icon and supporting text all begin at the same edge; the sentence explaining the field floated centred at whatever width it happened to be; - `SelectChatRoomTypeScreen`'s "this decides who can change the group later", which sits directly above three full-width cards; - `CreateProfileScreen`'s confirmation list, where "Name" and the name below it were each centred at their own width, so the label and the value it labels started in different places. Five texts there now share one edge. The parent columns are still `Alignment.CenterHorizontally`, which is why each of these needed `fillMaxWidth()` and not merely the removal of `textAlign`: a `Text` without a width in a centred column is centred as a box, so dropping the text alignment alone would have changed nothing visible. Nothing else in the sweep moves. The remaining 84 are listed above by category rather than site because the category is the reason. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-08 07:34:55 +02:00
Kgothatso Ngako	05e80bf099	feat: hold every screen's content to a readable line, and centre it in the window ... Phase 6, step 5, first half. Every one of the 40 screens rendered a single column that filled whatever width it was given, so on a 1800dp desktop window a paragraph became a 1800dp line -- long enough that the eye loses the start of the next one -- and a six-character text field stretched to 1700dp. M3: *"across all breakpoints, adjust margins and type styles to keep text between 40–60 characters per line."* **The measure is derived, not written down.** `readableContentWidth()` is `bodyLarge`'s font size converted through the current density, times half an em per character, times sixty: 480dp at the default text size. Writing `480.dp` instead would be the same number today and wrong for anybody who has turned text size up -- at 200% the same column holds thirty characters, silently, because the text still fits. Deriving it means the column widens with the type and keeps its sixty. `AverageCharacterAdvance` is the one estimate in it, named and documented, because a proportional face has no character width and half an em is the standard figure for mixed-case Latin prose. Only the ceiling is enforced. The floor needs nothing: a 400dp compact window less its two 16dp margins holds about 46 characters, which is inside the range, and no cap can add characters to a window that has none. There is a test for exactly that, so the claim is checked rather than asserted in a comment. **The column is centred; the text is not.** Those are opposite things and it is worth being explicit, because "centre it" is how the second one gets done by accident. A centred column still has one straight leading edge for every row, avatar and icon to align to, which is what the grids-and-spacing page asks for. Centred text has none. The 91 `TextAlign.Center` uses are a separate question and a separate commit. **Applied at 49 sites in one pass**, at the point every screen consumes its `Scaffold`'s padding -- the one place in each file that is reliably the top of the content. Below 480dp it is not a cap, an inset or a centring; it is nothing, so no phone layout moves. **Verified by measuring a real composition, not by reading the code.** `readableContent()` is `fillMaxWidth` then `wrapContentWidth` then `widthIn`, and every permutation of those three compiles and renders something that looks right in a phone-width preview. This needed `compose.desktop.uiTestJUnit4` in `jvmTest` -- pinned to the same 1.11.1 as the rest of Compose Multiplatform, test-only -- and `runDesktopComposeUiTest(width = 1400)`, which gives a window that genuinely is 1400 pixels across at density 1. Four assertions, and they bite: swapping the last two modifiers makes the 1400dp case report `Actual width is 1400.0.dp, expected 480.0.dp`, which is the "centred but never capped" failure the doc comment names. The same test also pins `currentBreakpoint()` to the real window at all five widths -- 400, 700, 1000, 1400, 1800 -- with the screen margin following. A version of it that measured the parent's constraints rather than the window would answer `Compact` everywhere and pass every unit test in the suite. 37 theme tests green; android and desktop both compile. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-08 07:31:33 +02:00
Kgothatso Ngako	b5bb0042b1	refactor: take the chat transcript out of the view model it was living in ... Phase 6, step 7, and the prerequisite for the pane work rather than a tidy-up: the transcript has to render at 400dp as a whole screen and at 900dp as the detail half of a two-pane layout, and a layout that lives in a view model cannot be composed twice or previewed once. `ChatMessageListViewModel` was 1,113 lines, of which 380 were `RenderMessages` -- a `@Composable` member function holding a `LazyColumn`, a `DropdownMenu`, `Card`s and both of the app's only two `BoxWithConstraints` -- plus three private composables under the class. It is now 356 lines of state and coroutines, and `ui/composable/widgets/chat/ChatTranscript.kt` is 779 lines of layout. **The move is verbatim.** `ProposalsAwaitingYouNotice`, `PrivateMessageNotice` and `RitualNotice` are byte-identical -- `diff` says so. `RenderMessages` becomes `ChatTranscript` and differs by exactly the signature line and fourteen references that had been resolving against the enclosing class and now say `viewModel.`. Nothing was rewritten while it was in the air; the diff is small enough to read line by line, which is the only reason to move 760 lines in one commit. **Why a parameter and not a receiver.** Keeping it as `fun ChatMessageListViewModel.ChatTranscript(...)` would have made the diff a single word, and left every one of those fourteen reads bare. `openMessageActionsFor` read bare says nothing about where it is kept; `viewModel.openMessageActionsFor` says it survives the composition, which is the fact a reader of a transcript needs and the one a pane split will make load-bearing. **Two imports the extraction nearly lost.** `androidx.compose.runtime.getValue` and `setValue` are used implicitly, by `by mutableStateOf`, so a "drop imports whose name does not appear" pass drops both and the five delegated properties stop compiling. The compiler caught it; noting it because the same pass over the next file will do the same thing. The earlier version of that pass also required an import's name not to follow a dot, which silently dropped every `Modifier.fillMaxWidth()`-shaped extension. `:composeApp:compileDebugKotlinAndroid`, `:composeApp:compileKotlinJvm` and the jvm test suite all green. The three `Icons.Filled` deprecation warnings in the new file came with the code and are unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-08 07:21:07 +02:00
Kgothatso Ngako	03d1e8e3b1	feat: give the app the five breakpoints, and let the screen margin follow them ... Phase 6, steps 1 and 2. The app had no notion of window width at all -- two `BoxWithConstraints` in 30,000 lines of UI, both inside a view model -- so every layout decision in it was made once, for a phone, and then rendered unchanged into a 1800dp desktop window. **The dependency question the plan asked to settle first.** `material3-adaptive` publishes multiplatform under `org.jetbrains.compose.material3.adaptive`, with android, desktop and ios variants; the ios ones carry `ios_arm64` and `ios_simulator_arm64` attributes despite the `uikit*` artifact names, so the targets this project declares on a mac resolve. Version **1.2.0**, not the newer 1.3.0-beta02, because that is the version the pinned material3 itself resolves: `material3-adaptive-navigation-suite:1.10.0-alpha05` names `adaptive:1.2.0` in its pom, and 1.3.0 would pull window-core 1.5.0 in beside the 1.4.0 the pinned material3 compiled against. Nothing is lost by staying: 1.2.0 already computes the large and extra-large breakpoints through `supportLargeAndXLargeWidth`, and carries `ListDetailPaneScaffold` for the pane work. So steps 3-4 can use the library scaffolds rather than a hand-rolled equivalent. **`Breakpoint`** is the five-value enum -- compact / medium / expanded / large / extra-large at 0 / 600 / 840 / 1200 / 1600dp -- with `ofWidth` as a pure function so the thresholds are assertable without a Compose runtime. `TorchTheme` classifies once and provides `LocalBreakpoint`, so no two screens can disagree about the window they are both in. It reads `currentWindowDpSize()` rather than `currentWindowAdaptiveInfo()`. The latter also computes a `Posture` from the platform's fold state, which on android reaches for `WindowInfoTracker` and an activity; this call sits in `TorchTheme`, which wraps all 51 `@Preview` bodies in the tree, and a preview context is not an activity. The pane scaffolds ask for posture themselves, at the one place a fold changes the answer. **Spacing now adapts, and exactly one value moves.** M3 publishes a margin per breakpoint -- 16dp compact, 24dp everywhere wider -- and publishes nothing else that varies with window width. The scale itself is absolute: `space200` is 16dp on a phone and 16dp on a desktop, and what adapts is which token a job reaches for, not the token. So `screenMargin` goes 16 -> 24 at medium and holds there, and `containerPadding`, `itemGap` and the rest do not move -- a card does not become a different component because the window grew. Widening all of them is the "everything breathes on a big screen" instinct, and it reads as a zoomed phone rather than as a layout. A test asserts the non-movement, because that is the edit a later reviewer would wave through. Mechanically this made the eight semantic names constructor parameters instead of `get()`s over the scale, so a breakpoint can reassign one without moving the stop underneath it. Kotlin resolves a default expression against the parameters before it, so each still reads its stop by name and still follows it when the scale is overridden -- phase 2's `Spacing(space200 = 24.dp)` assertion holds unchanged. The two instances are singletons because `LocalSpacing` is a `staticCompositionLocalOf` and invalidates on identity, not equality. **A test found a real defect while being written.** `ofWidth` was `entries.last { width >= it.minWidth }`, which throws `NoSuchElementException` below 0dp. A desktop window reports a zero size for the frame before its first layout pass, and this is called from the theme on every composition, so the crash would have arrived on a resize rather than on anything a user did. Now total. `:composeApp:compileDebugKotlinAndroid` and `:composeApp:compileKotlinJvm` both green; 28 theme tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-08 07:15:43 +02:00
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>	2026-09-08 02:01:39 +02:00
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>	2026-09-08 01:57:18 +02:00
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>	2026-09-08 01:45:35 +02:00
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>	2026-09-08 01:38:31 +02:00
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>	2026-09-08 01:27:26 +02:00
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>	2026-09-08 01:18:23 +02:00
Kgothatso Ngako	9dc748f2d3	fix: lift the eight text-field screens above the software keyboard ... Phase 3, third step, of docs/material-design-conformance.md. Nine screens and a dialog carry text fields. One of them called `imePadding()`; the audit had said seven screens, and was wrong about that too. `Scaffold`'s `contentWindowInsets` defaults to `systemBars`, which does **not** include the ime, so a `Scaffold` on its own does nothing about a keyboard covering the field being typed into. `Modifier.imePadding()` on the Scaffold lifts the whole screen, which is the standard shape and the one that needs no per-field handling. Eight screens get it: AddArtifact, AddChapter, AddDialect, TranslateChunk, ChatRoomCreation, CreateProfile, SignIn, WriteNewNote. Each carries a one-line comment saying why, since a bare modifier in a Scaffold argument list is the kind of thing that gets deleted in a cleanup. **ChatRoomMessagingScreen is deliberately not one of them**, and the reason is now written where somebody would look for it. Its composer already reserves its own bottom inset with `navigationBarsPadding()`. Adding `imePadding()` to the Scaffold as well would pad twice while the keyboard is up, because the ime inset already covers the navigation bar area that row is separately reserving. Getting that combination right wants a device with a keyboard open, not a compiler, and it is the one screen where the existing code shows signs of having been tuned by hand. **Not device-verified, and worth saying so plainly.** The emulator's account boots to a populated home screen, and reaching any of the eight means several hops through onboarding; what was confirmed is only that the keyboard interaction works on the path that was reachable -- the npub dialog's field moved from a bottom edge of y=1250 to y=840 with `mInputShown=true`, so ime handling is live on this build. The eight Scaffolds themselves were not each opened with a keyboard up. `ChatRoomCreationScreen`, reached through New Chat -> Start a group chat, is the shortest path for whoever checks. **Tests.** 944 pass, 595 jvm over 72 classes and 349 android over 44, unchanged. Window insets are a property of a running composition against a real window; there is no Compose UI test infrastructure here to assert them, and a test that the modifier is present would only restate the diff. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-08 00:57:21 +02:00
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>	2026-09-08 00:52:15 +02:00
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>	2026-09-08 00:45:01 +02:00
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>	2026-09-08 00:36:59 +02:00
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>	2026-09-08 00:32:27 +02:00
Kgothatso Ngako	73d99f9a41	feat: put M3's spacing scale in the theme, with semantic names over it ... Phase 2, first step, of docs/material-design-conformance.md. 527 `.dp` literals in the UI tree and no record of what any of them is for. This is what they migrate onto; the sweep that moves them is the next commit. **The scale is M3's own**, transcribed from m3.material.io/m3/pages/spacing/tokens: an 8dp system where `space100 = 8dp`, including the sub-8 nested units (2, 4, 6) and the non-multiples (10, 14, 20, 36) that Material defines because its own components need them. Eighteen stops. Worth being precise about what the audit found, because it changes what this phase is for. The two dominant values in the tree are `10.dp` (132 uses) and `20.dp` (115), and **both are already on the scale** -- `space125` and `space250`. Only 89 of 527 are genuinely off-grid. So this is not mostly a sweep for wrong numbers. It is that nothing records whether a given `10.dp` is padding, a gap or a margin, which are three things the spec gives different rules to, and none of them can be adapted per breakpoint or per density while they are literals. **A `data class` behind a composition local, not a file of constants.** Nothing scales it today and `Spacing()` is provided unmodified. It is shaped this way because two things are coming that need it: spacing adapts across breakpoints, and M3 has a density setting for data-heavy views. Both become a matter of providing a different instance rather than touching a call site -- but only if the values arrive through the local. Top-level `val`s would read identically and adapt to nothing, which is the version of this that looks done and is not. **Eight semantic names, because `space125` is no more readable than `10.dp`.** It says the size and not the job. `screenMargin`, `containerPadding`, `compactPadding`, `relatedGap`, `itemGap`, `sectionGap`, `emphasisGap`, `targetGap` say the job, and they are what call sites should reach for; the raw stops are for the cases none of them fits. They are split along the distinction the spec draws -- padding is inside an element, a gap is between elements in a container, a margin is outside one -- and there is exactly **one** margin, for the screen edge. That is deliberate: "define padding and gaps on the parent container", "avoid defining margins on child elements as they usually aren't uniform, and require more tokens". A semantic layer with a margin per element would have re-created the problem in better-sounding names. **Six assertions, and three of them are about failure modes that are invisible in review.** - Every stop matches its published value. `space175 = 15.dp` would look entirely plausible in the source, compile, and put every call site one unit off the grid. - The token name predicts the value: the number after "space" is the value as a percentage of the 8dp base, so `space250` is 20dp. A stop that does not obey that is a stop nobody can predict from its name. - Every semantic name resolves to a stop that is actually on the scale. The layer stops being a scale the moment one of them is handed a literal, which is easy to do and invisible to review. - `targetGap` is at least 8dp, M3's minimum separation between adjacent touch targets -- the one semantic name with an external floor, and the one phase 3 will apply between icon buttons. - A scaled instance moves the semantic names with it. This is what the data class is *for*: if a semantic name were a hardcoded `Dp` rather than a reference to a stop it would stay behind at a wider breakpoint and the layout would half-adapt, which is worse than not adapting. **Tests.** 936 pass, 594 jvm over 72 classes and 348 android over 44, up from 930/588/342. `:composeApp:compileDebugKotlinAndroid` builds. No call site changed, so no pixels moved. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-08 00:26:05 +02:00
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>	2026-09-08 00:23:31 +02:00
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>	2026-09-08 00:17:41 +02:00
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>	2026-09-08 00:10:06 +02:00
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>	2026-09-07 23:59:34 +02:00
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>	2026-09-07 23:51:34 +02:00
Kgothatso Ngako	7c41992ea7	fix: hold the wallet state where a background thread can be heard, so a device with no seed boots ... Since 11ab889 removed the "Introducing... Torch" gate, a device with no account boots to "Decrypting..." and stays there. The gate was not the cause. It was the only thing standing between the user and a write that had already been getting thrown away, and taking it out is what made the app depend on that write. **What the gate was doing for a device with no account.** It navigated off SovereignWalletStartupScreen on every cold boot, and the handler it navigated through did two things: onNavigateToWalletIntroPage = { navController.navigate(route = LoadingRoute(text = "Introducing... Torch")) applicationIOScope.launch { navigationViewModel.loadNostrProfile(route) } } The second line is what routed a device with no account. `loadNostrProfile` found no local account and a non-null startupRoute, answered `NavigationUIState.Landing`, and the user got the create-a-profile screen. It never read `listWalletState`. With the gate gone the only remaining route to Landing is `availableWallets.isEmpty()` inside the startup screen, and every branch of that screen sits behind `ListWalletState.Success`. So a state that nothing used to depend on became the whole of first run. **The screen is only ever shown Init.** Instrumented on an emulator with no seed.dat, one process, one view model (`vm=52654198` throughout): 15:15:41.400 31861 31888 SeedFileNotFound branch done: state=Success stateObj=150621551 snap=GlobalSnapshot@151321212 15:15:43.527 31861 31861 composing: vm=52654198 stateObj=150621551 listWalletState=Init 15:15:44.554 31861 31861 poll #0: stateObj=150621551 dbgMarker=Success snap=GlobalSnapshot@151321212 before=Init afterSendApply=Init Thread 31888 is `Dispatchers.IO`; 31861 is main. The write happened two seconds before the read, on the same `MutableState` object, and the reader never saw it. Neither a later `Snapshot.sendApplyNotifications()` nor anything else recovered it -- the value stayed Init for the life of the process, which is the spinner the user was looking at. **Three probes, written on the same line, on the same thread, at the same instant.** They were needed because the two obvious explanations -- two view models, or a stale snapshot -- both predict the same log line, and both are wrong: - a plain `@Volatile` String field: the main thread read `Success`. So this is one object written once, not two objects that happen to share an identity hash. - a `mutableStateOf` held in a top-level `object`, i.e. created at class-load time and never inside a composition: the main thread read `Success`. So writing Compose state from `Dispatchers.IO` works fine in this app. - the view model's own `mutableStateOf`: `Init`. Same instant, same thread, same write site, opposite answer. And a `mutableStateOf` written from `Dispatchers.IO` *later*, from the poll, was seen immediately (`ioWrite/mainRead=io-0`). What separates the two is not the thread and not the state -- it is when the write happens relative to the composition that created the state. **Why.** `SovereignWalletViewModel` is built by `viewModel(factory = ...)` in MantraNavHost, so its constructor runs *inside* a composition, and a `mutableStateOf` created there is created inside that composition's snapshot. A write from another thread that lands before that composition is applied does not survive it. Delaying the write past the composition proves the boundary rather than describing it -- with `delay(1500)` inserted ahead of the decrypt and nothing else changed: 15:18:07.520 result=SeedFileNotFound -> state=Success 15:18:08.142 composing: listWalletState=Success 15:18:08.143 no wallets -> landing Same code, same threads, same write; two seconds later it is heard. **Why only a device without an account.** The window is a few milliseconds wide, and which side of it you land on is decided by how long reading the seed takes. With no seed file, `SeedManager.loadAndDecrypt` goes as far as `FileSystem.SYSTEM.exists(seedFile)`, says "seed file doesn't exist", and returns `SeedFileNotFound`: 15:09:59.060 SeedManager: loadAndDecrypt 15:09:59.063 SeedManager: seed file doesn't exist Three milliseconds after `init` fired it, which is inside the first composition. With a seed the same call decrypts through the keystore, runs `MnemonicCode.toSeed`, builds a `LocalKeyManager` and derives a node id, then `DecryptSeedResult.Success` waits on a DataStore read for the wallet metadata -- long enough, every time, to land after. So the account case booted normally and the empty case hung, and the difference had nothing to do with accounts. That also means this was never specific to the no-seed path: it is a race that path always loses. **The change.** `listWalletState` becomes a `MutableStateFlow` behind `asStateFlow()`, and the screen collects it with `collectAsState()`. A StateFlow has no snapshot to belong to, so the same write from the same thread at the same moment is seen. This is not a new pattern here -- it is what `_availableWallets`, `_desiredWalletId` and `_activeWalletInUI` already do on this same class, and `_availableWallets` is written on the line below the one that was being lost and was always read correctly. `listWalletState` was the odd one out. The comment on the declaration says why it has to stay a flow. Changing it back would restore this bug exactly, and would do so silently: nothing throws, nothing logs, the state simply keeps its initial value. **Not audited.** `press.mantra.compose.ui.view.model` has around a dozen more `by mutableStateOf` properties on view models built the same way -- HomeViewModel, ChatMessageListViewModel, InReplyToViewModel, SovereignWalletStartupViewModel and others. Every one of them is exposed to this, and every one of them is fine only for as long as nothing writes it off the main thread during the composition that creates it. Most load from Room, which is slow enough to be safe by accident, which is the same kind of safety this bug had until the gate came out. Converting them is a sweep, not this commit; it wants each call site read rather than a mechanical replace. **Not tested.** There is no Compose UI test infrastructure in this repo, and the defect lives in the interaction between a view model constructor, a composition snapshot and a background coroutine -- there is nothing to assert against without a running composition. A test that the property's type is a StateFlow would only restate the declaration the compiler already checks. What stands in for it is the comment and the fact that the three sibling properties on the class establish the pattern. **Verified on a device, twice over.** emulator-5554, no seed.dat: was "Decrypting..." for ever, now goes to the landing screen and on into Create Profile. emulator-5556, seed.dat present and a broadcast profile: still boots through `processLocalAccount` to its home screen, so the path that always worked still does. 906 tests pass, 576 jvm over 69 classes and 330 android over 41 classes, unchanged -- this commit adds none. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-07 15:57:02 +02:00
Kgothatso Ngako	f301a924fd	fix: build a release apk, by dropping the app's copies of what the library already ships ... `:composeApp:assembleRelease` dies in `mergeDexRelease`: Type com.machankura.compose.ui.composable.widgets.nfc.ComposableSingletons$HceMonitorKt is defined multiple times: composeApp/build/intermediates/project_dex_archive/release/dexBuilderRelease/out/... lightning-kmp-app/library/build/.transforms/.../bundleLibRuntimeToDirAndroidMain_dex/... Both paths are ours. composeApp compiles a class, lightning-kmp-app's `:library` compiles the same fully-qualified name, and D8 will not merge the two into one apk. Debug never objected, because it packages the per-project dex archives as they stand and only the release merge walks the whole set looking for collisions -- so this has been true for a while and surfaced the first time anyone asked for a release. There were two such copies, and fixing the first only uncovered the second. **The NFC widgets were renamed by directory, not by package.** 9abdf42 ("Refactor torch to mantra", 2026-07-15) moved three files under `composeApp/src/androidMain/kotlin/press/mantra/compose/ui/composable/widgets/nfc/` and left their `package com.machankura.compose.ui.composable.widgets.nfc` line alone. The library holds the same three at `fr/acinq/phoenix/compose/ui/widgets/nfc/`, also declaring `com.machankura...`. So three directories say three different things and the package -- the only one of them D8 reads -- says one. `NfcState.kt` is byte-identical across the two; `HceMonitor.kt` and `NfcReaderMonitor.kt` differ in a single import line, `press.mantra` against `fr.acinq.phoenix` for `ModalBottomSheet`. Deleted the app's three rather than renaming their package to `press.mantra`, which was the other way out and is the worse one. `NfcStateRepository` is an `object`. The library's own `HceService` and `NfcReaderCallback` import it under the `com.machankura` name, and `MainActivity:50-52` reaches for it fully qualified. Renaming the app's copy would have compiled, dexed and run -- with two singletons: one that `MainActivity` sets back to `Inactive` on a new intent, and a different one the HCE service is collecting. Nothing would warn, because by then the names genuinely differ; the tag emulation would just not stop. Deleting leaves one object, and `MainActivity`'s existing fully-qualified references land on it untouched. The two composables went along with it as dead weight -- nothing outside their own files names `HceMonitor` or `NfcReaderMonitor` anywhere in composeApp. **The sqldelight databases were a second copy of the same kind.** With the NFC clash gone, `mergeDexRelease` came back with `fr.acinq.phoenix.db.sqldelight.AppDatabase$Companion`, out of the same pair of directories. composeApp's build.gradle.kts declared `ChannelsDatabase`, `PaymentsDatabase` and `AppDatabase` at packageName `fr.acinq.phoenix.db.sqldelight` from `.sq` sources under `src/commonMain/sqldelight`; the library's build.gradle.kts declares the same three names, the same package and the same layout, over a tree `diff -rq` reports as identical file for file. Both plugins ran, and both generated the same classes. Removed the plugin alias and the whole `sqldelight { }` block from composeApp and deleted its 32 `.sq`/`.sqm` files, rather than moving the app's generated package to `press.mantra`. Nothing under `press.mantra` imports `fr.acinq.phoenix.db` -- grep finds no file -- and nothing there touches `app.cash.sqldelight` either, against 18 files in the library that do. The app was generating a database layer it has never opened. Renaming would have kept generating it, and kept a second identical schema in the tree for someone to edit and then wonder why nothing moved. A comment sits where the plugin alias was, since the absence is the load-bearing part and an alias is a one-line thing to add back by reflex. **The sqldelight driver dependencies stay.** `-android-driver`, `-sqlite-driver`, `-native-driver`, `-runtime` and `-coroutines-extensions` are still declared in composeApp. They are runtime drivers rather than code generation, and are very likely redundant -- the library declares the same five as `implementation`, which still carries them onto the runtime classpath. But they are not what D8 objected to, and a missing driver fails when the app opens a database on one platform, not when it builds, so retiring them wants more evidence than a green build and is its own change. **What this does not fix.** Anything else copied into both trees fails exactly this way, and only on release. A scan of the two source sets for colliding fully-qualified top-level types is clean now -- the three NFC files were the only ones -- but that scan reads `.kt`, and the sqldelight collision had no `.kt` to find. Generated code is where the next one hides: Room and compose-resources generate into composeApp, and the library generates compose-resources too, which is why settings.gradle.kts keeps the submodule's project named `:library` and renames only the coordinate. **Verified.** `:composeApp:assembleRelease` produces the unsigned 36MB apk; `:composeApp:assembleDebug` builds. 906 tests pass, 576 jvm over 69 classes and 330 android over 41 classes, unchanged from before the change since it adds none. The apk was not installed, so that NFC still works on a device is read from the code -- one `NfcStateRepository`, the one both sides were already naming -- rather than watched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-06 22:13:32 +02:00
Kgothatso Ngako	224d6009dc	Merge branch 'mantra' into claude/marmot-profile-metadata-loading-e2a067	2026-09-06 21:50:44 +02:00
Kgothatso Ngako	e6524e201d	Merge branch 'mantra' into claude/proposals-signature-card-090e46	2026-09-06 21:21:55 +02:00
Kgothatso Ngako	f146afd49e	fix: keep asking who a Marmot group's members are, instead of once and never again ... A member of a Marmot group shows as "LOADING..." and stays that way. The same member in a NIP-17 room starts as "LOADING..." and then turns into their name. The difference is not that Marmot forgets to ask. It asks exactly once, and NIP-17 is the one that gets asked again. **"LOADING..." is a row, not a spinner.** Every pubkey this device sees gets a Profile row immediately, because Participant.participantPublicKey and ChatRoom.userPublicKey are both foreign keys onto Profile and nothing can be filed until one exists. What gets written is a placeholder: Profile( displayName = "LOADING...", publicKey = ..., createdAt = GENESIS_AT, nostrEventId = nostrEvent.id, // Will get overwriting by sync, ) GENESIS_AT (1231006505000L, the Bitcoin genesis block) is the marker: a row carrying it has never been read off a kind:0. `NostrDao.indexNostrEvent` writes one, `MarmotInboundManager.processGroupMembershipChanges` writes one, the Welcome branch of `NostrDao.indexNostrEvent` writes one, `NostrDao.getOrCreateNip17ChatRoom` writes one. Each of them then queues the kind:0 request that is supposed to replace it. Each queues it once. Once is a whole lot of load-bearing. `Relays.DefaultDMRelayList` is `listOf(ephemeral)` -- one relay -- so "ask the relays" is one negentropy reconciliation against one host, at whatever moment the pubkey first appeared. If that host has not got the member's kind:0 yet, that is the end of the enquiry. **NIP-17 gets a second chance twice over.** Opening a NIP-17 room runs `ChatMessageListViewModel.scheduleSynchronization`, which fetches each participant's kind:10050. That request is queued at level 0, so the kind:10050 it brings back is *indexed* at level 0 -- and the top of `indexNostrEvent` says: } else if (profile.createdAt == GENESIS_AT && level == 0) { logger.i("This is a placeholder profile... that might need to get synced...: $profile") ... profilePublicKeysToSync[relayURL]?.add(nostrEvent.pubKey) } which queues the full `profileEventKinds` set, kind:0 included. So the name arrives on the bounce: we asked for a relay list, we got an event that member signed, indexing it noticed the placeholder was still there, and it asked again for the profile. Any other event of theirs we happen to index does the same thing. **A Marmot group has neither half.** The first half is gated off explicitly: if (localChatRoom.chatRoom.mlsGroupState == null) { which is the whole body of `scheduleSynchronization`. Opening a Marmot room asks for nothing, by construction -- and reasonably so on its own terms, since an MLS room does not need a member's kind:10050 to address a message to them. The second half cannot fire, because a Marmot member never authors anything this device indexes under their own key. A kind:445 is signed by a throwaway keypair minted for that one event (`MarmotOutboundDao`, two sites: `NostrSignerInternal(KeyPair())`), and the real sender is inside the MLS frame, recovered in `indexMarmotGroupEvent` as `mlsGroup.memberIdentityHex(it.senderLeafIndex)` -- long after the pubkey check at the top of `indexNostrEvent` has already run against `nostrEvent.pubKey`. That check does fire on every kind:445; it just fires on the throwaway key, mints a placeholder for a key that will never exist again, and queues a profile sync for it. The member it is standing next to is not looked at. So: one ask at the Welcome (or at the commit that added them), and then nothing, ever, for the life of the room. Lose that one ask and the room is full of "LOADING...". **Two smaller holes, same shape.** Both Marmot mint sites test `profile == null`: val profile = database.profileDao().getProfileByPublicKey(newParticipant.participantPublicKey) if (profile == null) { // create placeholder AND queue the sync } A placeholder is not null. A member we already hold one for -- seen in another room, or removed from this one and added back -- takes the `false` branch and is never queued at all. Not even the single ask. **The change.** - New `nostr/MemberProfileSync.kt`. Picks out, from a set of rooms, the members nobody has read a kind:0 for -- missing row and placeholder row treated the same, ourselves excluded because our own profile is not something a relay teaches us -- and builds the kind:0 requests for them. Authors are chunked 100 per filter: a relay may refuse a filter it thinks is too big, and one refusal should not take every member down with it. Requests go out at level 0, which is deliberate: it is what marks a request as one somebody is waiting on, and it is what lets the arriving kind:0 pull the rest of the member (DM relay list, key packages) in behind it via the placeholder branch quoted above. - `LiveSubscriptionManager.queueCatchUpSynchronization` now also asks about every member it cannot name, across every room on the account. This is the main repair. It is the right home for it: the foreground catch-up already holds the room list (it was fetching it for `groupIdsFrom` and throwing the rooms away -- `liveGroupIds` is gone, the rooms are kept), it already exists to answer "what did I miss", and running there covers the chat list, the member lists and the message feed at once rather than one screen at a time. It re-runs on every foreground, so an ask that comes back empty is retried rather than lost. - `ChatMessageListViewModel.scheduleSynchronization` asks too, for both kinds of room, before the NIP-17-only relay-list block it already had. This closes the gap between foregrounds: join a group while the app is open, and the names resolve without backgrounding it first. - `MarmotInboundManager.processGroupMembershipChanges` and the Welcome branch of `NostrDao.indexNostrEvent` now treat a placeholder as unresolved. The placeholder insert still only happens when there is no row (it is an @Insert and would throw on conflict); it is the *ask* that now happens either way. **Left alone, deliberately.** The `mlsGroupState == null` gate below the new code stays: kind:10050 genuinely is NIP-17-only, and an MLS room's messages go to the group's own relays. The placeholder minted for a kind:445's throwaway signer is untouched -- it is waste, not a bug, and removing it means deciding what `indexNostrEvent` should do with an event whose author is by design nobody, which is a bigger question than this. `DefaultDMRelayList` being a single host is left as it is; widening profile lookups to the directory relays (purplepag.es, user.kindpag.es, directory.yabu.me are all already in `Relays`) would find more kind:0s than asking one relay repeatedly, and is worth doing on its own. **Verified.** `:composeApp:compileDebugKotlinAndroid` builds. `:composeApp:jvmTest` is green: 576 tests over 69 classes, including 7 new ones in `MemberProfileSyncTest` covering placeholder-vs-null, self-exclusion, a member in several rooms counted once, the filter shape (kind:0, level 0, one request per relay) and the 100-author chunking. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-06 21:21:46 +02:00
Kgothatso Ngako	73cba2ae0e	feat: say at the foot of the transcript what the group is still waiting for you to sign ... The transcript carries a proposal past as it happens, and b977326 gave a member owed two decisions a queue to find the second one in. Neither says anything before it is tapped. A proposal announces itself as one line among everything else the room said, the conversation carries it upward, and from then on the only evidence that the group is waiting on this member is a line they would have to scroll back to -- or the Proposals button on a screen behind a kebab menu, which is a place to look rather than a thing that tells you to look. The decision does not expire with the scroll, and a member who has not answered is what the whole room is waiting on. **Where it sits.** First item of the transcript's LazyColumn, which has `reverseLayout = true`, so index 0 is at the bottom edge of the viewport -- under the newest message and directly above the composer. That is also where the list is scrolled to when a room is opened, so a member who has just been asked for something is told so without moving. `ChatMessageDao` orders `createdAt DESC` and the reversal turns it back the right way up, which is why "first item" and "under the newest message" are the same place. **Not pinned above the composer.** It scrolls with the transcript and leaves view when a reader goes back through history. Pinning it is not a move of the composable: `RenderMessages` is a Column of `Spacer(weight(1f))` then the messages column, and a Column measures its unweighted children first and in order, each against what the previous ones left. The messages column holds a LazyColumn with no height modifier, which takes the whole remaining height, so a sibling card placed after it would be measured against nothing and would not appear. Making that work wants `weight(1f)` moved onto the messages column, which changes how every part of this screen is measured rather than where one card goes. **Guarded outside `item { }` rather than inside it.** The relay-list notice beside it is written the other way round -- an item that always exists and sometimes composes nothing -- and `verticalArrangement = Arrangement.spacedBy` puts its gap between every pair of adjacent items whatever their height, so an item composing nothing still costs 10.dp. This one is not added at all when nothing is owed, so a room with no proposals carries no phantom gap at the foot of its transcript. **Read before the list builder.** `proposalsAwaitingYou` is pulled into a local above the `LazyColumn` call rather than inside its scope, so the state read is plainly a read of `RenderMessages` and the notice appearing or disappearing is a recomposition of this function. Reading it inside the builder would work -- the item provider is snapshot-aware -- but it puts the difference between "no proposals" and "one proposal" inside a lambda whose re-execution is the lazy list's business rather than this function's. **What the held state carries now.** `proposalsAwaitingYou` widens from `Set<String>` to `List<AwaitingProposal>`: the session id it already had, plus a `ProposedEvent.Summary` of what the batch is about and the batch's size. It is still filled from the same `observeSessionsForChatRoom` collector asking `FrostSigningManager.isAwaitingApproval` -- the point of b977326's version was that the transcript and the proposal list ask one question, and that is unchanged. The summary is built in the collector rather than at render because it is parsed out of stored JSON, once per item, and this is a scrolling list. `ProposalListUIState.Proposal` derives its own for the same reason and says so; doing it in the composable would re-parse every event in every open proposal on every recomposition of the room. **The first item that can be read, not the first item.** `firstNotNullOfOrNull` over `Event.fromJsonOrNull`, matching `ProposalListScreen`, whose `lead` is the first of the already-`mapNotNull`ed events. A batch is named after its first item because the rest hang off it, but a batch whose first item this build cannot parse is still about something, and falling back to "Proposal" when the second item says "New chapter" would be throwing away the name for a reason the reader cannot see. **Named when there is one, counted when there are several.** One proposal gets "Waiting for your signature" over what it signs -- "New chapter · Genesis 1 · 797 words · 31 chunks" -- because "a proposal is waiting" is not something anybody can decide about, and the whole value of the card over a dot on a menu is that it says what the group wants. Several get the count and nothing else. Naming the first of several would say the others were not there, which is exactly the fault `ProposalListScreen` was built to fix; listing them all would be building that screen a second time in the composer's space. **The two ways to have nothing to name.** A session can exist before its proposal has arrived, and a proposal can arrive holding events this build cannot read. They are different situations and the card says which, in the same words `ProposalCard` uses -- "Nothing has arrived to sign yet" against "None of its events could be read" -- so a member who taps through finds the row saying what the card said. **It opens the queue, in every case.** Not the proposal it names, even when it names exactly one. The transcript's own lines are the way to a single proposal and keep their existing routing; this is the standing count of what is owed, and the queue is the screen that answers the question it raises -- including for a proposal it could not name, where opening one session would be opening the one thing the card just admitted it could not describe. **Its own callback rather than `onOpenSigning(null)`.** That would have worked: `ChatRoomMessagingScreen` already sends a null session id to `ProposalListRoute`. But null there is a claim -- "this line predates `ChatMessage.frostSigningSessionId` and cannot say which proposal it meant" -- and the card knows precisely which proposals it is about. Reusing the branch would make the null case mean two unrelated things and leave the next reader unable to tell which callers actually do not know their session. `onOpenProposals: () -> Unit` says the one thing it does. **`hidesOtherDecisions` is untouched in meaning.** It follows the new shape -- `size > 1 && any { it.sessionId == sessionId }`, with the cheap check first -- and still answers the same two questions about a tapped transcript line. No line changes where it goes. **Not covered, deliberately.** The empty-transcript branch gets no card. A proposal writes its own lines into the room that signs, which is what b977326 established, so an awaiting proposal implies a transcript; the branch this skips is the "Break the ice" case, which cannot coexist with one. The card lives and dies with an open room. b977326 closed by noting that nothing counts outstanding decisions where a reader can see them before tapping, and that is still true one level up: the home chat list says nothing, and a badge there wants this count somewhere it outlives one open room, which is its own change. `ChatRoomMessagingScreen` still calls `initiate()` inside `key(true) { }` rather than a `LaunchedEffect`, so its collectors are re-launched on recomposition. This adds no observer -- it reads the one b977326 added -- so the exposure is unchanged rather than widened. No tests. What changed is a composable and a navigation callback, and there is no UI test harness here to press a card in. The seam that does have one, `isAwaitingApproval`, is untouched and is already what the proposal list is tested through; the summary this reuses, `ProposedEvent.summarize`, is likewise already covered where it is defined. Verified: :composeApp:compileDebugKotlinAndroid succeeds; 884 tests pass, 565 jvm and 319 android, unchanged from before the change since it adds none. That the card appears exactly when a proposal is owed, and that it lands on the queue, are read from the code rather than asserted -- both want the app on a device in a group that has a key. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-06 21:20:53 +02:00
Kgothatso Ngako	ff1ecc1194	Merge branch 'mantra' into claude/torch-intro-boot-hang-76b0a8	2026-09-06 20:44:42 +02:00
Kgothatso Ngako	11ab8892f0	fix: let startup finish, rather than leave it for an intro that was never built ... Sometimes the app boots to "Introducing... Torch" and stays there. That string is not a screen. It is the `text` of a LoadingRoute that SovereignWalletStartupScreen navigated to whenever a preference called showIntro was true, and the one thing that was supposed to move the user off it had been commented out since 0a5a219 (2026-07-02). **The gate is always open.** GlobalPrefs.kt, in the lightning-kmp-app submodule: /** True if the intro screen must be shown. True by default. */ val getShowIntro: Flow<Boolean> = safeData.map { it[SHOW_INTRO] ?: true } suspend fun saveShowIntro(showIntro: Boolean) = data.edit { it[SHOW_INTRO] = showIntro } `saveShowIntro` has no caller anywhere in this repository -- its definition is its only occurrence. SHOW_INTRO is therefore never written, `?: true` is the answer every time, and the gate fired on every cold boot rather than once ever. Phoenix clears it when its onboarding carousel finishes. Mantra has no onboarding carousel; the destination is a loading placeholder. Nothing clears it because there is nothing to have been shown. **Leaving the screen is what stops the wallet.** The gate was the first thing in the composable, ahead of the Box that does the actual work: val showIntro = sovereignWalletStartupViewModel.getShowIntroFlow().collectAsState(initial = null) if (showIntro.value == true) { LaunchedEffect(Unit) { onNavigateToWalletIntroPage.invoke() } } Startup is not something that happens behind this screen; it *is* this screen. Reaching setActiveWallet means descending ListWalletState.Success -> a non-empty availableWallets -> wallet metadata and default wallet -> StartupViewState.Init -> LoadWallet, whose produceState reads getLockBiometricsEnabled and getLockPinEnabled before its LaunchedEffect calls doLoadWallet -> startupNode. Navigating away disposes that composition and cancels the LaunchedEffect that was going to make the call. A screen that has been left behind does not start a wallet, so activeWalletInUI stayed null. **And the state machine had already gone quiet.** NavigationViewModel. observeProfile collects activeWalletStateFlow with collectLatest. Null wallet, so it had already put StartupPhoenix into _navigationUIState -- which is how the user arrived at the startup screen in the first place. With activeWalletInUI pinned at null the flow never emits again, so collectLatest never re-runs; and even a re-run would `getAndUpdate { NavigationUIState.StartupPhoenix }` onto a MutableStateFlow already holding that same `data object`, which conflates. No emission, so MantraNavHost's collector never fires. Nothing was watching anything any more. **The one remaining exit was commented out.** onNavigateToWalletIntroPage launched loadNostrProfile(route) alongside the navigate, and that was the whole plan: park on a placeholder, work out where the user actually belongs, go there. In 0a5a219, NostrRepository.observeProfile was renamed observeLocalAccount and the call stopped compiling, so the branch was commented out where it stood: } else { // if (startupRoute != null) { // val localAccount = nostrRepository.observeProfile( // publicKey = activeUserPublicKey // ).firstOrNull() // // processLocalAccount(localAccount) // ... } Note where the comment markers fall. The surviving `if` covers only `activeUserPublicKey == null`. For anyone who already had an account, loadNostrProfile read the database, decided nothing, and returned -- a suspend function whose entire contract is to leave the navigation state pointing somewhere, doing so for exactly one caller out of two. **Why "sometimes".** Two axes. The first is a race between DataStore reads. The gate is a single read; the startup path is a chain of them. Usually the gate wins and nothing starts. When doLoadWallet did fire first, startupNode runs on SovereignWalletStartupViewModel.viewModelScope, scoped to the back stack entry -- and the intro navigate used no popUpTo, so the entry survived the departure. The node came up anyway, setActiveWallet fired, activeWalletInUI went non-null, collectLatest re-ran and routed properly. Boot looked fine. The second is whether there is an account. With none, `getLocalAccounts().firstOrNull()?.profile?.publicKey` is null, the surviving branch returns Landing, and the user lands on the create-a-profile screen. Only a device that already had a profile could reach the dead end. Fresh installs looked healthy, which is a good way for a bug to stay hidden. **The other dead end, which this one was hiding.** availableWallets.isEmpty() means no seed on the device, and it called onNavigateToWalletLandingPage, which navigated to LoadingRoute("Loading... Torch") and launched nothing at all. Not a race, not a rename -- just a loading screen with nothing left to load. It was never noticed because on a device with no seed the intro gate got there first and reached Landing by the account check above. So the accidental path was the only working route a new user had to creating a wallet, and removing the gate without fixing this would have broken first run. **The change.** Three files. - SovereignWalletStartupScreen: the gate and the onNavigateToWalletIntroPage parameter are gone. Startup runs to completion here; everything downstream reads the node's key manager, so departing before the node is up cannot work regardless of where it departs to. - MantraNavHost: onNavigateToWalletLandingPage navigates to LandingRoute with popUpTo(0), which is where making or restoring a seed lives, and matches how every state-driven navigation in this file clears the stack. - NavigationViewModel: the branch restored against observeLocalAccount, as `else if (startupRoute != null)`, so every path out of loadNostrProfile leaves the navigation state somewhere. Neither "... Torch" loading string exists any more. **Left in place.** getShowIntroFlow -- the expect/actual across android, ios and jvm, and the accessor on SovereignWalletStartupViewModel -- now has no callers. It is kept because a real intro screen will want it, but it has to be a step *inside* this flow, arriving before the wallet is needed and continuing to startup, not a detour around it. Wiring it back where it was would restore this bug exactly. **Not covered, deliberately.** loadNostrProfile still keys off `getLocalAccounts().firstOrNull()?.profile?.publicKey`, so an account whose kind-0 is not yet indexed reads as no account and answers Landing, while observeProfile -- which uses the key manager's pubkey and does not need a Profile row -- answers with the real state a moment later. Last write wins, self-correcting, and unchanged by this commit; it predates it and wants the account lookup rethought rather than patched here. **Tests.** NavigationRoutingTest, four of them in commonTest, against loadNostrProfile directly. NostrRepository.NO_OP_NOSTR_REPOSITORY throws from all 41 of its members, so `by` delegation over it gives a device that answers getLocalAccounts and observeLocalAccount and fails loudly on anything the call was not supposed to touch. Each test starts the view model on NavigationUIState.Loading("Introducing... Torch") -- the screen people were stranded on -- and asks whether the answer moved. Three of the four fail with the branch commented back out, and the headline one fails saying "boot stopped on the screen it was asked to move off. Actual: Loading(text=Introducing... Torch)", which is the bug report. The fourth, a device with no account reaching Landing, passes either way: that branch was never broken, and it is here so that losing it would not be free. The unqueued-profile case earns its place because the headline assertion is weak alone -- a constant would satisfy it. Two accounts differing only in signedAt come back ProfileLoaded and UnqueuedProfile, so what is pinned is that the destination is read off the account rather than being one fixed answer for "has an account". Both entities default their timestamps to Clock.System.now() and compare them in equals, so the fixture pins them. Without that, building the expected Profile a second time builds a different Profile, which is how the first run of these failed. **Not tested.** The other two files. Removing the showIntro gate and pointing onNavigateToWalletLandingPage at LandingRoute are Compose and NavHost wiring, and there is no Compose UI test infrastructure here to hang them on. Note where that leaves the coverage: the race that decides whether a given boot hangs lives in the untested half. What these tests hold is that the boot has somewhere to land once it arrives -- the half that turned a lost race into a dead end rather than a delay. Verified: :composeApp:compileDebugKotlinAndroid succeeds; 515 jvm tests, 511 before these four, 0 failed. ChronicleApplyJvmTest "an answered catch-up leaves one line, whatever it took to deliver" is flaky independently of this change -- it failed with these files reverted to their committed state, and has both passed and failed on identical code since. Filed separately, not touched here. The two untested files are read, not run: I did not put the app on a device. What is asserted about them is the code -- that saveShowIntro has no caller, that the commented branch was the only exit from that route, and that both replacements compile. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-06 20:39:33 +02:00
Kgothatso Ngako	e8a07aadf7	Merge branch 'mantra' into claude/proposals-view-navigation-9f3a58 ... Brings in ten commits: `ChatRoom.joinedGroupAt` and the pre-join indexing gate with schema v15, the home list's last-message preview and its `chatRoomId` index at v16, the membership transcript lines, and the widening of the notary's third queue. No conflict. mantra touches two of this branch's four files and meets neither change. In `ChatMessageListViewModel` its edits are a `MEMBERSHIP_TYPES` branch in the items loop and three icons and a tint in `RitualNotice`; this branch's are the constructor, `hidesOtherDecisions` and `observeProposalsAwaitingYou`. In `ChatRoomDetailScreen` its edit is the reindex report's wording at the foot of the screen, a couple of hundred lines below the button that moved. **The membership branch sits in front of the signing one**, which is worth checking rather than assuming: both are early returns in the same loop, and order decides which of them claims a row. Neither can claim the other's. `MEMBERSHIP_TYPES` is the three invite types and `FROST_TYPES` the eight signing ones, disjoint sets, and a membership line's `onClick` is `{}` -- it leads nowhere at all, so it never reaches the routing this branch changed. **Nothing mantra deletes can take a signing line.** `MIGRATION_14_15` deletes transcript rows, which is exactly the sort of thing that could quietly empty the transcript this branch routes from. It cannot: the delete is scoped to `ChatMessage.UNRESOLVED_MARMOT_TYPES`, which is `TYPE_UNDECRYPTABLE_OUTER_LAYER` and `TYPE_PENDING_COMMIT` -- placeholders standing in for events that were never read -- and no FROST type is in that set. Every other line, the signing ones included, is the final word on its group event and is left alone. **The pre-join gate and the proposal count agree by construction.** A signing message is a kind:445 like every other event in a Marmot group, so `NostrDao.indexMarmotGroupEvent` holds back the ones a group published before this device joined, and a session proposed before then leaves no rows here at all. `observeProposalsAwaitingYou` counts sessions rather than lines, so there is nothing for it to over-count: a member added mid-signing is not told they owe a decision on something they cannot see, and the transcript is not asked for a line about a session that was withheld from it. The two changes needed no reconciling because they are answering about the same withheld events from opposite ends. Verified after the merge rather than assumed from before it: :composeApp:compileDebugKotlinAndroid succeeds; 884 tests pass -- 565 jvm and 319 android, up from 808 because 76 of them are mantra's. This branch adds none, for the reasons its own commit gives. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-06 20:32:54 +02:00
Kgothatso Ngako	b97732643b	fix: put proposals in the room that signs, and open the queue when one row is not all of it ... The Proposals button sat behind `mlsGroupState == null`, so it was offered in NIP-17 rooms and withheld from Marmot ones. That is the wrong way round rather than a gap: FrostSigningManager signs "in its #admins room", and an #admins room is a Marmot group. **Where a group actually proposes.** `FrostSigningManager.broadcast` writes a signing message as an unprocessed `MarmotInnerEvent`, which `NotaryViewModel` MLS-encrypts and broadcasts as a kind:445 -- "the same path every other event in a Marmot group takes, which is why this needs no transport of its own". A NIP-17 room holds no MLS state for that path to use, and since 9107b81 `encryptAndSendMarmotInnerEvent` throws `MarmotMissingChatGroupException` on exactly that rather than silently doing nothing. So the button was shown in the one kind of room where a proposal cannot leave the device, and hidden in the kind where every proposal this app has made actually lives. The Shared Key button beside it keeps the gate, and the comment above it keeps its wording, because for a ceremony the gate is right: ChillDKG runs over NIP-17 because it has to -- its participants are not yet a Marmot group, and its whole purpose is to produce the key one would be keyed on -- so a room that already has an MLS tree is not a room a ceremony can run in. The two buttons stand next to each other and answer different questions; only one of them is about the transport the group already has. **Not gated on whether the room can sign.** A plain DM now shows Proposals too, and opening it says "This group has not been asked to sign anything yet." Gating on `FrostSigningRepository.canSign` would mean threading that repository into `ChatRoomDetailScreen` for one button's visibility, and the Shared Key button it sits beside is not gated either -- a room with no ceremony behind it still offers to hold one. An empty list is a truthful answer to a tap; an absent button is no answer at all to a member wondering where the proposals went, which is the complaint this commit starts from. **One row is not the queue.** Tapping a signing line in the transcript went straight to that session, and the list was reached only when the line predated chat rows naming their session. That is right while the reader has one decision outstanding and wrong the moment they have two: the second proposal is not in the transcript beside the first -- it may be pages up, or have arrived while they were reading -- so answering the one they tapped and leaving looks exactly like being done. A group proposing a chapter and the translation that depends on it is two sessions at once, which is the case `ProposalListScreen` exists for; the transcript was still handing over one of them and calling it the answer. **Only for a row that is itself waiting.** `hidesOtherDecisions` asks two things rather than one: that the tapped session is waiting on this member, and that it is not the only one. A line about something the group has already signed still opens that session directly. A reader who taps history is asking to see what was signed, and meeting that with the queue would be substituting a general answer for a specific request -- the same fault as the one being fixed, pointing the other way. **Where the answer comes from.** `ChatMessageListViewModel` observes `observeSessionsForChatRoom` and keeps the ids of the sessions `FrostSigningManager.isAwaitingApproval` calls pending. That is the same question `ProposalListUIState.waitingForYou` asks, deliberately, so the transcript and the list cannot come to different views about which proposals still have a decision in them. Observed rather than read once, because a proposal arrives while a room is open as often as before it is opened, and one answered on another device stops being owed with nothing happening here at all. Held as state rather than queried at the tap: a navigation callback is not suspend, and there is nowhere inside one to put a query. The read happens in the click lambda rather than during composition, so a proposal arriving moves where the next tap goes without recomposing the transcript to do it. That took the repository through `MantraNavHost` -> `ChatRoomMessagingScreen` -> `ChatMessageListViewModel.factory`; the screen's preview takes `NO_OP_FROST_SIGNING_REPOSITORY`, which already answers with an empty list. **Not covered, deliberately.** `ChatRoomMessagingScreen` calls `chatMessageListViewModel.initiate()` inside `key(true) { }` rather than a `LaunchedEffect`, so it re-runs on recomposition and launches a fresh collector each time. The three observers already there carry that exposure; `observeProposalsAwaitingYou` joins them rather than diverging from them, and since each collector writes the same value from the same Room flow the cost is duplicate collection, not a wrong answer. Fixing it changes how this screen starts all of its work, which is every observer's business rather than this one's. Nothing counts the outstanding decisions anywhere a reader can see them before tapping. A signing line still says only whether the line it sits on has been answered, and the room list says nothing. A badge wants this count somewhere it outlives one open room, which is its own change. No tests. Both changes are navigation decisions taken in composables -- `ChatRoomDetailScreen`'s visibility gate and `ChatRoomMessagingScreen`'s route choice -- and there is no UI test harness here to press a button in; the one piece with a seam, `isAwaitingApproval`, is already what the proposal list is tested through. Verified: :composeApp:compileDebugKotlinAndroid succeeds; 808 tests pass, 511 jvm and 297 android, unchanged from before the change. That a Marmot room now offers the list and that a second waiting proposal redirects the tap are read from the code, not asserted -- both want the app on a device with a group that has a key. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-06 20:27:34 +02:00
Kgothatso Ngako	acaf13fcd6	Merge branch 'mantra' into claude/marmot-group-message-queue-1ecaed	2026-09-06 20:21:19 +02:00
Kgothatso Ngako	30c732af67	Merge branch 'mantra' into claude/home-chat-previews-e1913f ... Two conflicts, and both are the same collision: mantra took schema v15 while this branch was also calling its migration v15. **The version number.** mantra's v15 adds `ChatRoom.joinedGroupAt` with a manual MIGRATION_14_15, because half of what it does -- deleting the placeholder chat lines already written for messages sent before this device joined -- is not a shape Room generates. That is the older claim on the number and it keeps it. The index migration here becomes `AutoMigration(from = 15, to = 16)` and the database goes to v16, so a device that has already run v15 gets the index on top of it rather than the two fighting over one version. `15.json` is resolved to mantra's wholesale -- an add/add conflict between two unrelated schemas is not something to merge line by line -- and 16.json is regenerated from the build. Checked rather than assumed: 16.json differs from 15.json in exactly one place, `index_ChatMessage_chatRoomId`, and no table's fields, createSql or other indices move. **mantra's new membership lines needed handling here**, and nothing would have told me: `98f766f` added `TYPE_MEMBER_INVITED`, `TYPE_MEMBER_INVITE_SENT` and `TYPE_MEMBER_INVITE_FAILED`, which the transcript renders as system notices. The chat list preview dispatches on the same question the transcript does -- is this somebody's words -- and a type missing from that check falls through to the chat bubble branch. A room whose newest line was an invite would have previewed as "Alice: Invited Bob to the group", which reads as Alice having said it. Exactly the failure `ChatMessage.MEMBERSHIP_TYPES`' own comment warns about, one screen over from where it was written. So `MEMBERSHIP_TYPES` joins the ritual and chronicle sets in `lastChatMessagePreviewText`. They are not in the AUTHORED sets -- their content is a whole sentence with the invitee's name already in it -- so they stand alone, which is what the transcript does with them too. One new test, over all three types rather than a representative one, since the set is the thing being relied on. Nothing else needed reconciling. mantra's pre-join fix filters at indexing time and deletes the rows outright, so the last-message subquery sees fewer rows and needs no `memberSince` clause of its own to stay in step with the transcript. 550 jvm tests and 319 android unit tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-06 20:12:49 +02:00
Kgothatso Ngako	5a22592c48	Merge branch 'mantra' into claude/member-invite-transcript-7a2d63 ... Brings in `ChatRoom.joinedGroupAt` and the pre-join indexing gate, plus schema v15. No conflict: mantra's only edit to `MarmotOutboundDao` is in `createMlsDirectMessageChatRoom`, stamping the new column as it builds the ChatRoom, and every line of this branch's is further down -- `inviteMember`, `addMembersToChatRoom`, `deliveryWelcome` and the three new announce helpers. The two changes do meet in one place, and it is worth saying why nothing had to be done about it. `MIGRATION_14_15` deletes transcript lines, which is exactly the sort of thing that could quietly eat the lines this branch adds. It cannot: the delete is scoped to `ChatMessage.UNRESOLVED_MARMOT_TYPES` and to rows whose `marmotGroupEventId` names an event older than the room, and a membership line is neither -- it is not a placeholder for an event still to come, and it has no group event behind it at all. Nor could it ever be in reach, because these lines are written by the *inviter*, whose own room has no epoch predating them. 828 tests pass -- 526 jvm, 302 android. The six new ones are this branch's. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-06 20:04:56 +02:00
Kgothatso Ngako	950deb2288	fix: widen the last of the notary's queues, the one already known to stall ... 9107b81 left the unsigned Nostr event queue alone on the grounds that it carries account traffic rather than group messages. It has the same defect, and unlike the other two it is not a latent one: NostrDao carries a written account of it having already happened. **The queue.** UnsignedNostrEventDao.observeUnsignedNostrEvents selected every unsigned row for a key and returned `Flow<UnsignedNostrEvent?>`, so Room handed back the first and dropped the rest: SELECT * FROM UnsignedNostrEvent WHERE pubKey = :publicKey AND signedAt IS NULL ORDER BY kind ASC The only exit is a successful publish. NostrDao.commitPublishedNostrEvent stamps `signedAt`, stores the signed event and queues a BroadcastNostrEventRequest per relay, all in one @Transaction. Nothing else clears it -- no attempt count, no failure status, no sweep -- so a row that cannot be published is selected again at the head of every later emission. **It has already happened.** The comment on commitPublishedNostrEvent is the report: indexing used to share that transaction, so any throw in it rolled `signedAt` back, "leaving the notary to re-select the same unsigned row forever and never sign anything queued behind it, including the MLS key package that goes last". That was closed by giving indexing its own transaction, and NostrDaoJvmTest pins it. What it did not close is the queue: it fixed the one known way to produce a stuck row and left the queue as narrow as it was, so the next way in has the same consequence. **The frozen variant.** Both other queues sat behind distinctUntilChanged too, and 9107b81 recorded that they failed by different mechanics depending on whether the row's `equals` was honest. UnsignedNostrEvent.equals is a plain value comparison with no @Ignore'd Logger in it, so this is the worse one: the stuck row's re-emission compared equal to the last, was dropped as no change, and the collector saw nothing again for the life of the session. Not a retry loop that never advances -- a collector that has stopped, while rows keep piling up behind a head nobody is looking at. **What sits behind the head.** The kinds matter here in a way they did not for the other two, because `kind ASC` is not an arbitrary order. An account queues 0 metadata, 3 contacts, 10007 search relays, 10012 relay feeds, 10050 DM relays, 10051 key package relays, and later 30443, the MLS key package -- which is what "goes last" means, since 30443 is the highest of them. Sitting in the middle is 10012, the one row of that burst carrying `privateTags`, and therefore the only one whose publish runs a NIP-44 encryption before it signs. A throw there takes 10050, 10051 and 30443 with it: both relay lists a peer needs to find this user, and the key package they need to invite them into a marmot group. The device looks fine to its owner -- the profile is announced, the gate has opened -- and is unreachable to everyone else. That is the shape of the next stall rather than a hypothetical one, which is why it is written on the query. **The fix.** Same as the other two. The query returns the backlog, NotaryViewModel walks it serially and keeps guardNotarization per row, and distinctUntilChanged goes. A publish that throws rolls back its own transaction and nothing else, so the row stays queued for the next pass while the rest of the account's events go out. `kind ASC` is kept, and now says why: kind 0 sorts first and NavigationViewModel holds the user on "announcing your profile" until it lands, so the order is load-bearing rather than incidental. `id ASC` is added as the tiebreak -- it is the autogenerated row id, so two events of one kind publish in the order they were queued, which for a replaceable kind is the difference between the newest version standing and an older one being published last and winning. Nothing about the navigation gate changes. It reads the kind-0 LocalAccount's relations, not the queue's shape, and kind 0 is still the first row of the first pass. With this the notary has no single-row queue left. The one remaining `distinctUntilChanged` in it, on observeActiveMarmotKeyPackageBundle, is correct: that flow is a state observation -- null means "no active bundle, make one" -- not a backlog, and re-running the creation on every unrelated emission is exactly what it is there to prevent. **Tests.** UnsignedNostrEventQueueJvmTest, seven of them, Room-backed. The account's real kinds are used rather than an inert one, because their sort order is the whole reason a stall in the middle of that burst leaves a user nobody can reach. "an event that cannot be published no longer hides the ones behind it" fails the 10012 row and asserts the metadata ahead of it and both relay lists and the key package behind it are all signed, with the stopper still queued and still unsigned; "an event past the stopper is queued for every relay" then checks the key package got a pending BroadcastNostrEventRequest per relay, since per 0211764 a row with `signedAt` and no request is the same silence in a different place. The rest pin the backlog arriving whole and lowest-kind first, one kind's rows keeping their insertion order across two passes, and one key's queue not containing another's. **Not covered.** The drain in the test is shaped like NotaryViewModel's loop but is the test's own, so these pin the DAO's half -- the backlog arrives whole, publishing some rows neither disturbs nor depends on the others -- and not the collector. Standing that up wants an ActiveWallet StateFlow and the whole ViewModel with it. A row that can never be published is still never published; as with 9107b81 it is only no longer contagious, and nothing yet tells the user which of their events is stuck or why. Verified: :composeApp:compileDebugKotlinAndroid succeeds; 518 tests pass, 511 before these seven. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-06 19:04:27 +02:00
Kgothatso Ngako	9107b81c99	fix: hand the notary the whole queue, not the one row standing at its head ... A message sent into a marmot group sometimes sticks on the unsealed icon and never leaves. It is not that message that is broken. Something ahead of it in the queue cannot be sent, and because the notary was handed one row at a time, that row was the queue. **The queue.** MarmotInnerEventDao.observeUnprocessedMarmotInnerEvents selected every unsent row for a key and returned `Flow<MarmotInnerEvent?>`, so Room handed back the first one and dropped the rest: SELECT * FROM MarmotInnerEvent WHERE publicKey = :publicKey AND marmotGroupEventId IS NULL ORDER BY createdAt ASC The only exit from that queue is a successful send. MarmotOutboundDao.encryptAndSendMarmotInnerEvent stamps the row with the group event it became, inside the same @Transaction that writes the event, the NostrEvent and the BroadcastNostrEventRequests. Nothing else clears `marmotGroupEventId`, there is no attempt count, no failure status and no sweep. A row that cannot be sent therefore does not move, and it is selected again, and again, at the head of every later emission. Note what the filter is: the sender's key, not the room. One room whose MLS state is gone silences every group on the device. **The stopper.** DatabaseMarmotRepository.encryptAndSendMarmotInnerEvent was two nested `?.let`: database.chatRoomDao().findChatRoomById(...)?.let { localChatRoom -> localChatRoom.chatRoom.toMlsGroup()?.let { mlsGroup -> ... } } A row for a room this device holds no MLS state for -- the shape a room restored from an inbound gift wrap has -- fell out of both and returned. No send, no throw, no log, and no mark on the row. So the queue manufactured its own permanent head: a message that could never succeed, reported as though nothing had happened, sitting in front of everything else forever. MarmotOutboundDao.inviteMemberToChatRoom already throws MarmotMissingChatGroupException on exactly this condition, with a comment saying the point is to "say so instead of silently doing nothing and letting the caller report success". The send path disagreed with the invite path about the same missing group. **distinctUntilChanged.** Both notary collectors sat behind it, which is the wrong question to ask a work queue. A queue re-emits because its table changed; that is the signal to look again, not a duplicate to discard. Comparing an emission against the previous one asks "is this new work?" when the question is "is there work left?". The two queues then failed by different mechanics, which is worth writing down because it explains why the symptom looks like a retry loop in one place and a dead collector in the other: - MarmotInnerEvent.equals compares `logger`, an @Ignore'd `Logger.withTag(TAG)` initialised per instance. Kermit's `withTag` returns `Logger(this.config, tag)` -- a fresh object -- and neither Logger nor BaseLogger overrides equals, so two reads of one row are never equal. distinctUntilChanged suppressed nothing here, and the notary spent the session retrying the stopper and never looking past it. Correct behaviour by accident, resting on a field that is not part of the row. - GiftWrapPayload.equals is an honest value comparison with no logger in it. The refused payload's re-emission compared equal and was dropped, so after one refusal nothing on that queue was collected again for the life of the session, whatever was queued afterwards. **Why it reads as unsealed.** ChatMessageListViewModel picks its status icon off three relations, in order: a broadcast receipt, a broadcast request, a NostrEvent. A queued marmot message has none of them until the notary turns it into a kind:445, so it falls to the last branch -- KeyOff, "Unsealed message status". The icon is accurate. The message is exactly as unsealed as it looks, and will stay that way. **The gift wrap queue has it too, and a Welcome rides it.** MIP-02 addresses kind:444 to a joiner who holds no group state and cannot read a kind:445, so MarmotOutboundDao.deliveryWelcome queues one as a GiftWrapPayload deliberately -- marmot traffic on the NIP-17 path. That queue had the same single-row shape and no ORDER BY at all, so which row was "the head" was whatever SQLite returned first. Two known refusals leave a payload there with `giftWrapSealId` still null: sealGiftWrapPayload refuses outright to seal a non-Welcome payload belonging to an MLS room (65e4a3a, and the comment there already named the blockage this causes), and a Welcome whose joiner published no key package matches no participant and produces no wraps at all. **The fix.** Both queries return the backlog instead of its head, ordered `createdAt ASC, id ASC` -- the same ordering BroadcastNostrEventRequestDao settled on, and for the same reason: createdAt is persisted at second resolution, a burst of sends shares one, and an order that is only ever "some row with this timestamp" lets two passes disagree about what comes next. NotaryViewModel walks the list and keeps guardNotarization per row, so a failure costs only itself. It walks it serially and in order on purpose: each send ratchets its room's MLS state forward and writes it back, and encryptAndSendMarmotInnerEvent re-reads that state per row, so two sends for one room in parallel would encrypt from the same generation and the group could read only one of them. The two `?.let`s become two throws, which the per-row guard logs. A failed row writes nothing -- the DAO is one transaction -- so it stays queued and is tried again on the next pass. That is wanted: a room whose state has not caught up yet deserves the retry, and a room that never will is at least no longer standing in front of anybody. The retry is bounded by the fact that it is Room's invalidation driving it: a pass in which every remaining row fails writes nothing, invalidates nothing and emits nothing further. No schema change. The queue's shape was in the query and the collector, not in the table. **Tests.** MarmotOutboundQueueJvmTest, eight of them, Room-backed against a real MlsGroup -- the DAO seam MarmotOutboundDaoJvmTest opened, which 0211764 could not use and said so. Two rooms stand side by side, one holding real MLS state and one holding none, and the unsendable row is queued first on purpose because under the old queue it was the only row the notary ever saw. The one that matters is "a message that cannot be sent no longer holds up the ones behind it": the stopper fails, exactly once, and the message behind it in another room still comes out with a group event and one pending BroadcastNostrEventRequest per relay -- pending because, per 0211764, that is the only status the broadcaster looks at and the only thing that actually puts a kind:445 on a relay. The rest pin the supporting facts: the backlog arrives whole and oldest first, rows sharing a second come back in the same order twice, a room with no MLS state and a room that does not exist are each refused rather than ignored, a refused send writes neither a group event nor a broadcast request, and two messages for one room each ratchet the group forward and both leave the queue. **Not covered, deliberately.** The notary's third queue, unsigned Nostr events, still has this shape, and UnsignedNostrEvent.equals is a value comparison, so it is the frozen-collector variant rather than the retrying one. NostrDao.kt's comment on commitPublishedNostrEvent records that it has already bitten once -- an indexing throw rolled back `signedAt` and "every later event (including the MLS key package, which is enqueued last) would never be signed at all" -- fixed point-wise by moving indexing out of the transaction, leaving the queue shape untouched. It carries account traffic rather than group messages and NavigationViewModel gates the user on it, so it is its own change. Nothing here surfaces *why* a message is stuck. A row that can never be sent is still never sent; it is only no longer contagious. Telling the sender that would want a persisted attempt count and a place in the UI to put it, which is also its own change. Verified: :composeApp:compileDebugKotlinAndroid succeeds; 511 tests pass, 503 before these eight. That a stuck room no longer silences a healthy one is asserted against a real group in a real database, not inferred -- but that a second participant now receives the messages that were backing up is inference from the code, since it wants two devices. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-06 18:56:33 +02:00
Kgothatso Ngako	473228bfab	feat: show the last message and its time on the home chat list ... The row was a centred column with the room's name in it and nothing else. It is now a row: the name and a one-line preview of what was last said in a weighted column on the left, the clock on the right. Both text lines are clipped to one with an ellipsis, the name included, and that is what keeps the clock on the row. A long subject, or a five-member group whose title is five names joined with commas, would otherwise push the timestamp off the edge; a multi-line message would push the next room down the screen. The clipping is on the title's own composable rather than the row, because that is where the three ways of building a title live -- a subject, "Note to Self", or the participant list -- and only one of them being clipped is the shape this would rot into. The clock is absent rather than blank for a room with nothing in it. There is no message time to show, and the room's own creation -- which is what it sorts on in that case -- is not something the user has any reason to read here. The preview line carries "No messages yet" in its place, which is the honest state for a room that exists because it was just made, or because a member joined a working group and is still waiting on the chronicle to fill it in. Everything the row renders was decided in the model, so this commit is the card body and the title clipping and nothing else. Verified: `:composeApp:compileDebugKotlinAndroid` builds, and the full `:composeApp:jvmTest` suite passes at 526 tests, 0 failures -- the 503 that were there before this branch plus its 23. The layout itself has not been run on a device; it is checked by compilation and by the model tests behind it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-06 18:21:48 +02:00
Kgothatso Ngako	81965ba2d3	perf: index chat messages by their room ... The query in the previous commit reads the newest line of every room the user is in, once per room, and Room re-runs the whole thing every time a message lands anywhere. Without an index on `ChatMessage.chatRoomId` each of those lookups is a scan of every message on the device: work that grows with the entire history rather than with the room, on the hot path of every arriving message. A device with ten rooms and a few thousand messages does tens of thousands of row reads to redraw a list whose visible change is one line of text. Room has wanted this index since the foreign key was declared and has said so on every build -- `chatRoomId column references a foreign key but it is not part of an index. This may trigger full table scans whenever parent table is modified` -- which is the same warning it still emits for a dozen other `chatRoomId` columns. Those stay as they are; this one now has a reader that makes it matter. **Schema v15, and Room writes the migration itself.** Adding an index changes no columns and moves no rows, which is one of the shapes `AutoMigration` handles without a spec, so this is an entry in the list rather than another manual migration alongside MIGRATION_13_14. The generated 15.json differs from 14.json in exactly one place, checked rather than assumed: `index_ChatMessage_chatRoomId` appears on ChatMessage, and no table's fields, createSql or other indices move at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-06 18:21:31 +02:00
Kgothatso Ngako	50feb6fa0c	feat: carry each room's newest line with the room, and say it in one line ... The home screen listed rooms by name and nothing else, in whatever order SQLite handed them back -- which for a query with no ORDER BY is rowid, so the list was ordered by when each room was first written and never moved again. The room somebody messaged an hour ago sat wherever it was created, indistinguishable from one nobody has touched since March. **The query.** `ChatRoomDao`'s room reads now LEFT JOIN each room's newest `ChatMessage` and order on it. The join is a correlated subquery rather than a `GROUP BY chatRoomId` with `MAX(createdAt)`: ON lastMessage.id = (SELECT id FROM ChatMessage WHERE chatRoomId = ChatRoom.id AND deletedAt IS NULL ORDER BY createdAt DESC, id DESC LIMIT 1) `MantraConverters` stores an `Instant` as epoch *seconds*, so lines written in one second tie -- a ceremony puts a dozen into a room faster than that -- and the aggregate form resolves a tie arbitrarily, which would leave a room quoting whichever of its last three lines SQLite happened to reach first. `id DESC` breaks it on write order, which is the order the transcript shows them in, so the list and the room it opens agree about what was said last. A room with nothing said in it sorts on its own `createdAt`. The alternative is sorting it last, which buries a room the user just made under every conversation they have ever had. **The flow now re-emits on message traffic**, because the query reads ChatMessage and Room invalidates on the tables a query touches. That is the point -- a row's preview and its place in the order stay current without the list asking for either -- but it is a real change for the other collector of this flow. `LiveSubscriptionManager.followGroupMembership` maps to group ids through `distinctUntilChanged()` before its debounce, so the extra emissions collapse there and no relay subscription churns on an arriving message. **The carried line is a `ChatRoomLastMessage`, not a `ChatMessage`.** Embedding the entity would mean aliasing thirty-odd columns onto every room query, and colliding with the room's own `id` and all four of its timestamps on the way. Six columns are everything a one-line preview and a clock can be written from. It is nullable, and every other way of getting a `LocalChatRoom` leaves it null rather than paying for a join no screen reads. So a null there means "not asked for" as often as it means "nothing said", which is why nothing hangs a decision on it beyond what to draw. **What that line is rendered as** follows the transcript's own dispatch in `ChatMessageListViewModel`, because the two must not disagree about what a room's newest activity was: - a ritual or chronicle line is nobody's words. Its content is written as a predicate for an actor's name, so the authored ones get that name in front ("Alice published their share") and the rest stand alone ("The group now has a shared key"). A name in front of the latter reads as that member having announced it, which is exactly the misattribution the transcript renders these as system lines to avoid. - a direct message with blank content is one this device cannot open. An empty preview reads as the sender having said nothing, so the line says instead what the group can in fact see: that a private message was sent, and to whom. - anything else is somebody's words, prefixed with who said them -- except in a two-person room, where the only other name is already the row's title and repeating it says nothing. Names resolve through the existing `HexKey.memberName` rather than a second copy of that lookup, so they follow a rename and fall back to a shortened key instead of dropping the attribution to nobody. 17 new tests. Seven run against a real SQLite, for the parts only it can answer -- which row the subquery picks, the same-second tie, room scoping, a soft-deleted last line, and where a room with no messages lands. Ten exercise the preview text directly, one per shape above plus the unknown-sender fallback. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-06 18:21:19 +02:00
Kgothatso Ngako	fd9137ab5a	feat: a chat list clock that says only as much as it has to ... The one timestamp format this app had, `toFormattedTimeAndDateString`, writes "14:05 6 Sep 2026". That is right for a message bubble, where it is the only clock on a line the reader has already stopped at. A chat list row is not that. Its job is to place a message relative to now, in whatever width is left after the room's name and the preview of what was said in it -- and a full date on this morning's message spends all of that saying "today" the long way. So the new format gets coarser the further back it goes, and never coarser than the reader can still resolve: - today, the time of day. Anything less cannot order two of today's rooms. - yesterday, named. A date here is a small arithmetic problem to read. - the rest of the last week, an abbreviated weekday. It stops at six days because the seventh is this weekday again, and "Sun" on a message from last Sunday reads as today. - inside this year, day and month. Past a week the weekday has stopped saying anything. - beyond it, the year as well, for the same reason one rung up: day and month repeat. Reading a rung too far is the failure mode and it is silent -- nothing about "Sun" admits which Sunday it means -- so every boundary is a test. Each one is anchored to the local day rather than to a fixed instant, because the boundaries are local midnights and the test would otherwise pass or fail on the machine's zone. A timestamp ahead of `now` deliberately falls through to a date rather than a time. Relay clocks disagree and an event can arrive stamped in the future; rendering that as "14:05" files it under a today it does not belong to. `now` is a parameter defaulting to `Clock.System.now()`, which is the whole reason any of the above is testable without a clock abstraction. It is sampled once per composition, so a list left open across midnight goes on saying "14:05" until something recomposes it -- acceptable for a list that recomposes on every arriving message, and not worth a ticker to fix. Six new tests, one per rung plus the future case. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-06 18:20:57 +02:00
Kgothatso Ngako	49c012bf8b	fix: a member is not shown the messages sent before they were in the room ... A joiner gets the MLS key schedule from their own epoch forward and nothing before it. The relay does not know that and hands them the whole room: negentropy syncs down every kind:445 the group ever published, `indexMarmotGroupEvent` read each one against the group, the outer layer refused, and every refusal wrote an `undecryptableOuterLayer` line. So the room a member had just been invited to opened on a screenful of "Undecryptable Message" above the conversation -- one per message the group had sent before they arrived, none of them ever readable, and the count only grows with how long the group had been talking. **The epoch of a kind:445 is inside the layer that will not decrypt**, so an event this device cannot read cannot be asked what epoch it is from. "From before we joined", "from an epoch we have not caught up to" and "from an epoch that fell out of the retention window" are indistinguishable from the outside, and only the first is permanent. What separates it is not the ciphertext but the clock: it was published before the group made the epoch we joined at. **`ChatRoom.joinedGroupAt` is that moment, written down.** The Welcome's `created_at`, which the inviter stamps as it mints the Welcome out of the Add commit that made us a member -- so it is the group's own account of when our epoch began, not this device's account of when it heard about it. A group this device created sets it to the room's creation; it was a member from epoch 0 and there is nothing behind it to hold back. Stored rather than read off `createdAt`, which today holds the same value in both paths. `createdAt` is row bookkeeping and this decides which of a group's messages a member is allowed to see at all; the two being equal is a coincidence of the current code, and hanging the second off the first makes a future change to when a room row is written into a change in what gets discarded. `memberSince` is `joinedGroupAt ?: createdAt`, so a room joined before the column existed gets the fix too -- and gets it from the value every path that sets the column would have written anyway. **`predatesMembership` draws the line strictly before**, and that is a judgement rather than a fact. Nostr stamps `created_at` in whole seconds, so the second the Welcome was minted holds both the commit that added us -- the last act of the epoch before ours, unreadable by construction -- and any message another member sent the instant they applied it. Only one of the two can be had. An unreadable event kept costs one refused decrypt; a readable event discarded is a message the member never sees. So the second is kept, and a room may still show a single placeholder for the commit that added its newest member. **Two places gate on it.** `indexMarmotGroupEvent` returns before touching the MLS group, so nothing is decrypted, no `MarmotGroupEvent` row is filed for ciphertext whose key this device never had, and no line is written. `reindexMarmotGroupEvents` partitions them out of the sweep entirely: a replay can say in advance that no pass will ever read them, so replaying them only spends a refused decrypt per sweep and reports every one as a failure on a room where nothing is wrong. `MarmotReindexSweep` is untouched apart from carrying the new count -- it decides how many times to go round, not what is worth going round for. **Schema v15, and the migration is the half that fixes devices already showing the bug.** Nothing rewrites a chat line that is already in the transcript, so fixing the write path alone would leave every member who joined a busy room opening it on the same run of placeholders forever. `MIGRATION_14_15` adds the column and deletes the lines: only the two types in `UNRESOLVED_MARMOT_TYPES`, and only where the group event behind them predates the room. Those lines say nothing by design -- they stand in for an event that was never read -- so removing one loses nothing, while every other line is the final word on its group event. The group events themselves stay; this is about what the room shows. The column is left null rather than backfilled from `createdAt`. Null already means "ask `createdAt`", and copying the value would turn a fallback into a claim this migration is in no position to make. It is manual rather than an `AutoMigration` only because of the delete: `ALTER TABLE ... ADD COLUMN` appends, which is where Room's own generated migration for a nullable addition puts one, and Room compares a table's columns by name rather than by position. **The reindex report stopped being true**, so it carries the number now. With the backlog held back, `unresolved` falls to zero and the screen said "Nothing to reindex - 30 event(s) all read" about a room where 27 of them were never this device's to read. `MarmotReindexReport.predatingMembership` is reported alongside `stored`, and the detail screen names it: "3 event(s) all read - 27 from before you joined". A member invited into an old room is the ordinary case, not an anomaly to bury in a total. Seventeen tests. `ChatRoomMembershipWindowTest` holds the boundary, including the same-second case and both directions of the `createdAt` fallback. `JoinedGroupAtMigrationJvmTest` runs the migration's own SQL against v14's three tables and covers what it must not take as carefully as what it must: a placeholder for an event from *after* the join is left to be recovered, a message that was read is left alone however old it is, a line with no group event behind it is out of reach of the rule, and two rooms joined at different times are each measured against their own join. `MarmotPreJoinIndexingJvmTest` drives the DAO against a room with no MLS state, which is what separates "left alone because it predates the join" from "tried and failed". 520 jvm tests and 302 android unit tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-06 18:18:16 +02:00
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>	2026-09-06 18:16:26 +02:00