diff --git a/android/COVERAGE_PLAN.md b/android/COVERAGE_PLAN.md index 0b4ab51..ce4afd2 100644 --- a/android/COVERAGE_PLAN.md +++ b/android/COVERAGE_PLAN.md @@ -202,3 +202,28 @@ class LoginViewModelTest { - `MainDispatcherRule` + a documented ViewModel test pattern exist and are reused. - Coverage exclusions list only genuinely non-unit-testable files (UI composables, Android-framework glue) — no ViewModel, repository, DTO, or pure core-logic file is excluded. + +## 6. Amendment (2026-08-08, M12 phase 8): `ui/theme/**` narrowed to one file + +**A directory glob in the exclusion list went stale as soon as another milestone put testable code +in that directory.** §2 phase 0 excluded `app/src/main/java/**/ui/theme/**` because at the time the +directory held `Color.kt`, `Type.kt` and the composables — constants and composable bodies, nothing +a JVM test could execute. M12 then added three *pure* resolvers to it: `ShardPalette`, +`ShardStructure` and `ShardTypeface`, which exist precisely so the theming milestone's no-op +invariants could be plain JVM assertions. JaCoCo measures them at **98%, 100% and 100%**, and the +glob was discarding every line. + +The exclusion is now the single file it was really about, `ui/theme/Theme.kt` (the composable, 52%). +Everything else in `ui/theme/` is measured and all of it covers at 93% or better. + +Two things worth carrying forward: + +- **This did not rescue the gate and was not meant to.** M12's already-measured code + (`data/appearance/` at 100%, `ui/navigation/` at 93–100%) clears `new_coverage ≥ 50` on its own. + The point is that a future change deleting those resolvers' tests would now move the number, where + before it would not have — the exclusion was hiding well-tested code, which is the opposite of what + §1 built the list for and what §5's third bullet asks for. +- **Prefer file globs to directory globs when a directory is mixed.** `ui/components/**` stays a + directory glob and correctly so: `BrandAssets.kt` sits at 11% because only `brandAssetUrl` is pure, + and the rest is composable bodies. The distinction is whether the directory is *uniformly* + untestable, not whether it is under `ui/`. diff --git a/android/PLAN.md b/android/PLAN.md index 42079bd..e59689e 100644 --- a/android/PLAN.md +++ b/android/PLAN.md @@ -1030,14 +1030,60 @@ push, and Play (M6–M8) follow the designed app. `nav_admin` two (`/player`, `/account`, `/admin`, `/admin/moderation`); the sidebar's other ~18 rows are admin *configuration* the app excludes, and two of the app's four staff entries are aggregates with no single web row. That is why they honor `label` and `hidden` only — and - why that phase is scheduled **last and marked optional**, so it can be dropped on its merits - once the rest is working. + why that phase was scheduled **last and marked optional**, so it could be dropped on its + merits. **It was: phase 7 is cancelled (2026-08-08).** The app makes no authenticated settings + call, `nav_admin` / `nav_player` are not consumed, and the player and staff drawer rows keep + their coded — and therefore localized — labels. Nothing in phases 0–6 was scaffolding for it. - **Excluded**, in the same class as M10's and M11's exclusions: the admin *configuration* panels themselves. The app does not gain Appearance or Navigation editors; it is a consumer. - Nine phases into a fresh `edge` in both repos, reaching `main` as one `edge` → `main` merge — - the same shape the website side used. Phase 0 (the contract and the appearance store) carries a - hard rule: it must change nothing on screen. + Eight phases (0–6 and 8; **7 cancelled**) into a fresh `edge` in both repos, reaching `main` as + one `edge` → `main` merge — the same shape the website side used. Phase 0 (the contract and the + appearance store) carries a hard rule: it must change nothing on screen. **Phase 0 landed + 2026-08-08** (Android-app#33 / docs#112): `SiteAppearance` now holds the brand, the resolved + theme tokens and the parsed `nav_public`, refreshed on resume, with nothing yet reading the last + two — see THEMING_AND_NAV.md "Phase 0 as landed". **Phase 1 landed 2026-08-08** + (Android-app#34 / docs#113): the fifteen color tokens now resolve into a `ShardPalette` that + feeds both the Material scheme and a `LocalShardPalette`, and the no-op invariant is a test — + the shipped palette reproduces the pre-M12 `ColorScheme` role for role. **Phase 2 landed + 2026-08-08** (Android-app#35 / docs#114): the four radii scale the app's own dp by their ratio + to the `runic-gateway` baseline, and `--shadow-card` maps onto card elevation. The shadow is + a **deliberate change to an untouched instance** — the app has been flat since M5 while the + preset it was drawn from selects a shadow — and is why every `Card(` became a `ShardCard(`: + Material takes elevation as a default argument, not from the theme. **Phase 3 landed + 2026-08-08** (Android-app#36 / docs#115): seven bundled families beside Cinzel, resolved from + the stacks the theme publishes. It **nearly tripled the APK** — 4.80 → 12.94 MiB against a + drafted 1.5–2.5 MB estimate, with Merriweather alone 6.08 MiB of the growth because upstream + ships it as a barely-compressible three-axis variable font; verbatim bundling was chosen with + the cheaper options costed and declined. **Phase 4 landed 2026-08-08** (Android-app#37 / + docs#116): logo in the drawer header and top bar, hero on Home, no new dependency and no APK + cost. **Phase 5 landed 2026-08-08** (Android-app#38 / docs#117): the public nav's label, order + and hiding, keyed by the path→route table — whose sort keys must be indices into the + *website's* sixteen-row nav, not the app's nine. **Phase 6 landed 2026-08-08** + (Android-app#39 / docs#118): the drawer gains admin-authored sections and added links, a link + the app can open natively doing so and one it cannot handing off to a Custom Tab. + + **Phase 8 landed 2026-08-08** (Android-app#40 / docs#119) and did more than its name. Scoped + as docs, coverage and the cutover, its **AC-5 on-device walk** — two AVD passes against a local + website, the themed one at every role rung — confirmed everything phases 1–6 had deferred to it + (including that a Custom Tab really opens, checked in `dumpsys`, and that an anonymous caller + sees the reordered, sectioned nav with no player or staff row leaking) **and found two theming + defects that 476 passing tests could not see**: + + - every `ShardCard` — 26 sites in 20 files — drew in Material's grey rather than the shard's + panel colour, because `CardDefaults.cardColors()` takes its container from + `surfaceContainerHighest` and the mapping had `surfaceContainer`, `…High` and `…Low` but not + `…Highest`; and + - the drawer's selected row ignored `--radius-pill`, because `NavigationDrawerItem` takes + `shape` as a default argument. + + Both are **phase 2's trap repeating**: Material takes these as default arguments, not from the + theme. Phase 2 found it for depth and swept the call sites; nobody asked whether colour and + shape had the same problem. The card fix also **changes the untouched app** — those grey cards + predate M12 and go back to M5 — so the milestone ships two deliberate changes to a shard that + has set nothing, the shadow and the card container, with AC-1 rewritten to record the second + rather than absorb it. Phase 8 also narrowed a stale `ui/theme/**` Sonar coverage glob that was + discarding three 98–100%-covered resolvers. **477 tests green.** ### Deferred (not a milestone) diff --git a/android/THEMING_AND_NAV.md b/android/THEMING_AND_NAV.md index 5eeb66e..756f786 100644 --- a/android/THEMING_AND_NAV.md +++ b/android/THEMING_AND_NAV.md @@ -45,6 +45,13 @@ shipped value. A bad `--accent` must not discard a good `--bg` beside it, and a settings call that fails is the same state as "no overrides" — never an error screen, never a half-painted theme. +**The one sanctioned exception is card depth (§5.4).** The app has been flat +since M5 while the preset it was drawn from selects a shadow, so applying the +shadow map as specified gives an untouched instance a depth it did not have. +Approved by the org lead in phase 2 rather than rebased onto the flat baseline, +because the alternative left three of the admin's four choices doing nothing on +the phone. Radii and colors are unaffected: both are still provable no-ops. + ## 3. What the server already publishes No backend work. Everything below is live on `website/main` today. @@ -54,7 +61,7 @@ No backend work. Everything below is live on `website/main` today. | `GET /public/settings` | `theme` | `Record` — 15 colors, 4 radii, `--shadow-card`, 3 font stacks. **Absent** when no row exists | | `GET /public/settings` | `brand.accent` / `.logo` / `.hero` / `.favicon` | Already **effective** values (override → env). The app reads `accent` today | | `GET /public/settings` | `nav_public` | Raw JSON **string**: a bare items map, or `{items, sections, links}` | -| `GET /api/v1/settings/nav` | `nav_admin`, `nav_player` | Raw JSON strings. Gate is `requireAuth`, **no role check** — a player may read it | +| ~~`GET /api/v1/settings/nav`~~ | ~~`nav_admin`, `nav_player`~~ | Raw JSON strings. Gate is `requireAuth`, **no role check** — a player may read it. **The app does not call this**: phase 7 cancelled (§6.4, §8) | Two shapes to get right on the wire: @@ -87,7 +94,7 @@ is no resolved counterpart — the merge is the *client's* job on the web too. | Light mode | Still **out of scope**. Every v1 preset is dark; the website's Parchment preset was cancelled (website §8 phase 9). The app stays dark-only, and `Theme.kt` keeps its single `darkColorScheme` | | Favicon | **No app surface.** Ignored, and not modeled | | Added links | A path matching a known app route opens the **native screen**; anything else hands off to a **Custom Tab** — see §6.3 | -| `nav_admin` / `nav_player` | **`label` and `hidden` only.** No order, no group — see §6.4 | +| `nav_admin` / `nav_player` | **Not consumed at all** — phase 7 cancelled 2026-08-08 (§6.4, §8). Was: `label` and `hidden` only | | Refresh | On connect, on **process start**, and on **resume** alongside the existing role re-validation — see §5.5 | | Failure posture | Forgiving on read, field by field. A failed settings call renders the shipped app, never an error | @@ -193,6 +200,13 @@ it. Consequences, all of them wanted: Round to whole dp and clamp at 0. +**The pill is the one field resolved as a literal**, settled in phase 2: +`CircleShape` is a *percentage*, so it has no shipped dp for a ratio to scale +against and the rule above has nothing to compute. Below the 500px floor the +resolved px is taken as dp directly — Fantasy's 4px → 4dp, Modern's 8px → 8dp — +which reads the same on a ~22dp chip as it does on the web. It is a literal +because there is no app scale to preserve here, not as an exception to the rule. + ### 5.3 Fonts — bundled, mapped by first family The shortlist is 12 options across three roles, spanning **eight** families: @@ -225,21 +239,57 @@ is constructed server-side and the only part that carries the choice: → the role's shipped family ``` +**The lookup is one global map, never scoped to the role's option list** — +settled in phase 3, and the same trap §5.4 hit with `--shadow-card`. The server +validates an *admin-entered* font against `FONT_OPTIONS[role]`, but a preset's +tokens are copied verbatim by `resolveThemeTokens` and never pass through that +list: `modern` publishes `--display: 'Work Sans', Arial, sans-serif`, which the +display dropdown does not offer, and `fantasy` publishes +`--sans: 'EB Garamond', Georgia, serif`, which the sans dropdown does not either. +A per-role lookup would have missed the display face of one preset and the label +face of the other. + The three roles map onto `Type.kt`'s existing three groups verbatim: `--display` → the Cinzel display/headline/title block, `--serif` → the `AppSerif` body block, `--sans` → the `AppSans` label block. Sizes, weights and tracking do not move — only the family. -**IM Fell English has no bold weight** (the website doc records the same). A -`FontWeight.Bold` request against it must resolve to its single weight rather -than synthesize; check what Compose does here on device and pin the behavior in -the phase's notes. +Because a family is not confined to the role its dropdown lives in, **every +bundled family supplies all four weights the scale asks for** — 400 (body), 500 +and 700 (labels), 600 (display) — pinned through `FontVariation` on the variable +faces. Two exceptions, both upstream facts rather than choices: -APK cost: roughly **1.5–2.5 MB** across seven families, variable-axis where Google -Fonts publishes one (Cinzel, EB Garamond, Merriweather, Playfair Display, Inter, -Work Sans, Source Sans 3) and single-weight for IM Fell English. Measure the -release APK before and after, and record both numbers in the PR — R8 does not -shrink `res/font/`. +- **IM Fell English has one weight per style.** Its 400 face answers all four + requests and Android synthesises the bold; the website's dropdown labels it + "(no bold weight)" for the same reason. What that synthesis actually looks like + is an AC-5 observation, not something a unit test can pin. +- **Cinzel keeps the 500/600/700 it shipped with in M5.** It is the only family + the server offers in the display role alone, so nothing can ask it for 400 and + adding an instance would have edited M5's type for no reachable case. + +**Italics mirror the website's set**, decided in phase 3: EB Garamond, IM Fell +English, Merriweather and Playfair Display carry a true italic, exactly the four +`client/index.html` requests one for. Inter, Work Sans, Source Sans 3 and Cinzel +are upright-only and Compose skews them — which is what the app already did for +every family before this milestone, and what the web does for its own +upright-only faces. The app draws italic in two places. + +APK cost, **measured, not estimated** — the drafted "roughly 1.5–2.5 MB" was +wrong by more than 3×. Unsigned release APK, R8 full-mode + resource shrink, on +`edge` at the phase 2 merge: + +| | added (compressed) | release APK | +|---|---|---| +| before phase 3 | — | 5,031,411 B — 4.80 MiB | +| after phase 3 | 8,543,292 B — 8.15 MiB | 13,574,703 B — **12.94 MiB** | + +**Merriweather is 6.08 MiB of the 8.15** — upstream ships it as a three-axis +`[opsz,wdth,wght]` variable font with a full charset, 4.6 MB per style, and it +deflates only 31% where the others manage 50–60%. Bundling it verbatim anyway was +the org lead's call, taken with the cheaper options costed: Google's own static +400/700 builds would have held the whole app near 6.8 MiB, and not bundling it at +all near 6.2 MiB, at the price of a serif option that silently does nothing on +Android. R8 does not shrink `res/font/`; the APK's deflate is the only saving. ### 5.4 Shadow depth @@ -253,8 +303,19 @@ it maps to card elevation: | `0 14px 34px rgba(0,0,0,0.3)` (Default) | 4dp | | `0 18px 44px rgba(0,0,0,0.45)` (Deep) | 8dp | -Matched by exact string against the server's `SHADOW_OPTIONS`; anything else is -the shipped default. Applied to `FeatureCard` and the Material `Card` defaults. +~~Matched by exact string against the server's `SHADOW_OPTIONS`~~ — **matched by +nearest blur**, corrected in phase 2. The `fantasy` preset publishes +`0 16px 38px rgba(0, 0, 0, 0.45)`, which `SHADOW_OPTIONS` does **not** contain, +because a preset's own tokens are copied verbatim by `resolveThemeTokens` and +never pass through the admin form's dropdown. An exact match would therefore have +missed the one preset whose point is a heavier shadow. Blur-matching puts any +future preset on the nearest step instead of silently on the default; Fantasy's +38px lands on Default's 34 rather than Deep's 44, and `none` is still matched as +a literal. Applied to `FeatureCard` and the Material `Card` defaults. + +**This map is not a no-op, and that is the decision, not an oversight** — see §2. +An untouched instance has no `--shadow-card`, which resolves to the shipped +default of 4dp, while the app draws its cards flat today. ### 5.5 When the appearance is (re-)read @@ -268,10 +329,11 @@ refreshed: changing the theme on a laptop and picking the phone up should see it, and the app already pays for a resume round-trip. -The authenticated `GET /api/v1/settings/nav` is fetched only when the session is -signed in, and re-fetched when the session changes — the same lifecycle -`ShardFeaturesRepository` already uses (M11). Signing out drops the cached admin -and player overrides. +~~The authenticated `GET /api/v1/settings/nav` is fetched only when the session is +signed in, and re-fetched when the session changes.~~ **Phase 7 is cancelled +(§6.4, §8): the app makes no authenticated settings call, and there is nothing +session-keyed to tear down on sign-out.** `GET /public/settings` is the whole +lifecycle. Every one of these is best-effort. A failed refresh keeps the last good appearance; there is no loading state and no error surface. @@ -286,12 +348,16 @@ a logo. Nothing new is needed to fetch them — they already arrive resolved, an Two surfaces, chosen to mirror the website's without inventing new layout: -- **the drawer header**, above the instance name that sits there today; -- **the top bar**, replacing the uppercased name when a logo exists. +- **the drawer header**, above the instance name that sits there today, at 32dp; +- **the top bar**, replacing the uppercased name when a logo exists, at 24dp. And the hero on **Home**, above the title block, which is the one screen with a hero-shaped space. +Both logo slots cap their width at **six times their height**, mirroring the +website's `maxWidth: height * 6`, so a long wordmark scales down rather than +pushing the drawer header or the top bar's title out of shape. + The M5 `BrandLogo` rule carries over: **render nothing when the slot is empty.** Not a placeholder, not a reserved gap — an instance with no uploaded logo must lay out exactly as it does today, which is §2 applied to assets. On the centered @@ -301,6 +367,35 @@ reason: a row would change the block's height on instances that have no logo. An asset that fails to load is the same as no asset. No broken-image icon, no retry. +**The top bar is the one place where "empty" is not "nothing".** Everywhere else +the empty slot draws literally nothing, because something else on the surface +already says the instance's name. In the top bar the logo *is* the title, so a +404 — or a logo that can't be fetched because the shard is down — would leave +the app in an unnamed shell until the next resume refresh. There "the same as no +asset" resolves to the text, since the text is what an instance with no logo +shows. There is deliberately **no fallback while the load is still in flight**: +drawing the text first would flash text → logo on every navigation for the sake +of one frame, as Coil serves the second and later reads from its memory cache. + +**The hero is a fixed 180dp band, cropped**, rather than the intrinsic aspect +ratio the app's other images (`PostScreen`, `BlockRenderer`) draw at. The +website's hero is a CSS background driven by `hero_layout`, which the app does +not port, so the app needs a rule of its own — and the website's *default* hero +is a square emblem (`/assets/img/runic-emblem.png`), so an uploaded square is a +case to expect rather than an edge one. At the intrinsic aspect that square +would be a ~360dp block that pushes the status card off the first screenful; +cropped to a band, a wide banner and a square give the same frame above the +title. It clips to `shapes.medium`, so the hero follows `--radius-card` like +every other surface the admin can round off (§5.2). Note that the app takes **no +fallback image**: where the website substitutes its own emblem for an unset +hero, the app draws nothing, because §2 outranks the mirror. + +Accessibility follows the website's split: the logo is **decorative wherever the +name is also on screen in text** (the drawer header, where the name is the very +next line) and **named only where it stands alone** (the top bar). Describing +the drawer's would have a screen reader say the instance's name twice — the same +call the website's `alt=''` makes. The hero is always decorative. + ## 6. Navigation ### 6.1 The hard constraint carries over @@ -315,43 +410,73 @@ are unchanged by it. An override cannot introduce an app route, cannot touch role or the shard's visibility config would otherwise withhold. Hiding is subtractive, exactly as `applyNavOverrides` has it. +> **Decision (2026-08-08): an admin may hide the Home row.** The website lets `/` +> be hidden from the public header — the brand link still goes home — and the app +> mirrors it rather than inventing a policy the site does not have. Home remains +> the `NavHost`'s start destination and remains reachable by back-press; unlike +> the website's `/admin/navigation`, which has three guards because hiding it +> would strip the only way to *undo* an override, nothing about a hidden Home row +> is unrecoverable. Hiding therefore stays one uniform rule with no special cases. + ### 6.2 Path → app route The public nav is keyed by **website** paths. The app needs a mapping table, and it is the one new piece of cross-repo coupling this milestone introduces — so it lives in one file with the website's `NAV` array quoted beside it. -| website `to` | app route | note | -|---|---|---| -| `/` | `Routes.HOME` | | -| `/site/news` | `Routes.NEWS` | | -| `/site/five-on-friday` | `Routes.news(FIVE_ON_FRIDAY)` | the app's News screen already has all four categories as tabs — these three select one | -| `/site/newsletter` | `Routes.news(NEWSLETTER)` | | -| `/site/screenshots` | `Routes.news(SCREENSHOTS)` | | -| `/wiki` | `Routes.WIKI` | | -| `/site/shard` | `Routes.SHARD` | `feature: status` | -| `/site/champs` | `Routes.SHARD_CHAMPS` | **not in `APP_MENU` today** — reached via the Shard hub | -| `/site/guilds` | `Routes.SHARD_GUILDS` | as above | -| `/site/governors` | `Routes.SHARD_GOVERNORS` | as above | -| `/site/houses` | `Routes.SHARD_HOUSES` | as above | -| `/site/rules` | `Routes.SHARD_RULES` | | -| `/site/atlas` | `Routes.ATLAS` | | -| `/site/leaderboards` | `Routes.SHARD_LEADERBOARDS` | | -| `/site/market` | `Routes.SHARD_MARKET` | | -| `/site/about` | `Routes.page("about")` | | +**The table is in the website's order, and that order is load-bearing.** A stored +`order` is an index into the site's nav, so a row the admin never moved has to +take its sort key from the same list or explicit and implicit keys sit on two +incomparable number lines (see "Phase 5 as landed"). The rows below are verbatim +from `website/client/src/components/SiteHeader.jsx` — note that the three news +categories are **Screenshots, Five on Friday, Newsletter** in that sequence, +between News and Wiki. + +| # | website `to` | app route | note | +|---|---|---|---| +| 0 | `/` | `Routes.HOME` | | +| 1 | `/site/news` | `Routes.NEWS` | | +| 2 | `/site/screenshots` | `Routes.news(SCREENSHOTS)` | the app's News screen already has all four categories as tabs — these three select one, and none has an `APP_MENU` row | +| 3 | `/site/five-on-friday` | `Routes.news(FIVE_ON_FRIDAY)` | as above | +| 4 | `/site/newsletter` | `Routes.news(NEWSLETTER)` | as above | +| 5 | `/wiki` | `Routes.WIKI` | | +| 6 | `/site/shard` | `Routes.SHARD` | `feature: status` | +| 7 | `/site/champs` | `Routes.SHARD_CHAMPS` | **not in `APP_MENU` today** — reached via the Shard hub | +| 8 | `/site/guilds` | `Routes.SHARD_GUILDS` | as above | +| 9 | `/site/governors` | `Routes.SHARD_GOVERNORS` | as above | +| 10 | `/site/houses` | `Routes.SHARD_HOUSES` | as above | +| 11 | `/site/rules` | `Routes.SHARD_RULES` | | +| 12 | `/site/atlas` | `Routes.ATLAS` | | +| 13 | `/site/leaderboards` | `Routes.SHARD_LEADERBOARDS` | | +| 14 | `/site/market` | `Routes.SHARD_MARKET` | | +| 15 | `/site/about` | `Routes.page("about")` | | + +**The `feature` values are not mirrored into this table**, though the website's +array carries one on nine of these rows. `APP_MENU` stays the app's own source of +truth for gating: a second copy of a security-relevant value that drifts silently +is worth more than it costs. The table carries the mapping and nothing else. Three asymmetries to resolve rather than paper over: - **`Routes.NEWS` takes no category argument today.** It gains an optional one so - the three category entries can land on the right tab. This is a small route - change with its own test, not a nav concern. -- **Four web entries have no `APP_MENU` row** (champs / guilds / governors / - houses — the app puts them behind the Shard hub, which is the better phone - shape and stays). An override for one of them therefore has a mapped route but - no menu entry. **Rule: an override for a path the app does not surface in its - menu is ignored**, exactly as the web drops an override for an unknown `to`. - It is *not* an invitation to add the entry — the hub is a deliberate design - choice, and a nav override may not introduce navigation. + a link to one of the three category pages can land on the right tab. This is a + small route change with its own test, not a nav concern. +- **Seven web entries have no `APP_MENU` row** — champs / guilds / governors / + houses (the app puts them behind the Shard hub, which is the better phone shape + and stays) and the three news categories (tabs on one screen). An override for + one of them therefore has a mapped route but no menu entry. **Rule: an override + for a path the app does not surface in its menu is ignored**, exactly as the web + drops an override for an unknown `to`. It is *not* an invitation to add the + entry — the hub and the tab strip are deliberate design choices, and a nav + override may not introduce navigation. + + > **Decision (2026-08-08):** this rule covers the **news categories too**, not + > only the hub four. The mapping in the table above is what phase 6's added + > links resolve against — and *there* a category tab is a perfectly good + > destination, because the admin named it by path — but neither a relabel nor a + > reorder of `/site/screenshots` puts a new row in the drawer. The alternative + > considered and rejected was surfacing such a row only when overridden, which + > keeps AC-1 but lets an override introduce navigation after all. - **Ten app entries have no `nav_public` counterpart** — Contact, Account and Notifications, the three player groups, and the four staff rows. They are unaffected by `nav_public` and keep their coded order, appended after the @@ -381,23 +506,70 @@ An added link **opens natively when its path maps to an app route**, and hands off to a Custom Tab otherwise. The patterns the app can resolve: ``` -/ → HOME -/site/news|five-on-friday|newsletter|screenshots → NEWS (category) -/site/news/ → POST -/wiki → WIKI -/wiki/ → WIKI_PAGE -/site/ → the mapped shard route (per §6.2) -/site/about, /page/ → PAGE -/contact → CONTACT -anything else → WebHandoff (Custom Tab), M3's existing hand-off +/ → HOME +/site/news → NEWS +/site/{screenshots,five-on-friday,newsletter} + → NEWS, that category's tab +/site/newsletter/ → POST +/wiki → WIKI +/wiki/ → WIKI_PAGE +/site/ → the mapped shard route (per §6.2) +/site/atlas/ → ATLAS_CREATURE +/site/market/vendors/ → SHARD_MARKET_VENDOR +/site/about → PAGE("about") +/ → PAGE(slug), unless is reserved +anything else → WebHandoff (Custom Tab), M3's existing hand-off ``` +> **Correction (2026-08-08).** The table this replaces named three paths the +> website does not serve, and was written from the app's routes rather than the +> site's. Checked against `website/client/src/App.jsx`: there is **no +> `/site/news/`** — news items render on their category page and the +> site's one post-detail route is the newsletter's `/site/newsletter/:id`; there +> is **no `/page/`** — CMS pages are served from a top-level `/`; and +> there is **no `/contact`** at all, the app's contact form being app-only (§6.2 +> says as much). The site's real detail routes for the atlas and the market were +> missing. A link resolver that does not read the site's own route table cannot be +> right by accident, so it now quotes it, next to §6.2's table and for the same +> reason. +> +> **Decision (2026-08-08): the `/` catch-all is in**, with the site's +> non-CMS top-level segments (`admin`, `account`, `player`, `site`, `wiki`, +> `invite`, `preview`, `api`, `uploads`) excluded from it. React Router ranks its +> static routes above `/:slug` and the app has to do the same, or a link to the +> admin panel would open an in-app 404 instead of the real thing in a browser. +> The reserved list is a second piece of cross-repo coupling and it lives beside +> the first. It buys the case that matters most: a page the admin wrote — the +> likeliest added link there is — opens natively. +> +> **A path carrying a query or a fragment hands off**, whatever its route part +> says. No app route takes either, so a native match would quietly drop what the +> admin typed; the browser honors it exactly. + A native match still passes through the app's own gates: an added link to `/site/market` on a shard that does not publish the market lands on the Market screen's honest "not published here" state (M11's `FEATURE_UNAVAILABLE`), which is what typing the URL on the web does too. The link itself is not gated — that is the website's decision and the app does not second-guess it. +**In the drawer** (all four settled by the org lead before code, 2026-08-08): + +- **A section is a static header with its rows indented beneath it**, always + open. A collapsible group was considered and rejected: it costs remembered + state per section and can hide the row the admin meant to surface. +- **A link that hands off carries a trailing icon** + (`Icons.AutoMirrored.Filled.ExitToApp`, the only "leaves the app" glyph in + `material-icons-core` — the extended artifact is not a dependency and phase 3 + already spent the APK budget). A link the app resolves natively is deliberately + indistinguishable from a coded row; that is the point of resolving it. +- **The Custom Tab's absolute URL comes from `LocalAssetResolver`**, which + already resolves any site-relative path against the configured base URL and is + already provided at the app root. A nav path travelling through something named + "asset resolver" is the cost; zero new plumbing is the benefit. +- **A resolved link opens like any other drawer row** — `navigateTopLevel`, + detail screen or not — rather than pushing onto the current screen. One rule, + and back-press lands on Home exactly as it does from every other row. + ### 6.4 `nav_admin` and `nav_player` — label and hidden only Both are bare maps and neither carries sections or links. The app honors @@ -435,6 +607,14 @@ and its teardown. It is scheduled last precisely so that decision can be taken with the rest of the milestone already working — dropping it costs nothing that phases 0–6 depend on. Unmapped keys are ignored either way. +> **Decision (2026-08-08): phase 7 is cancelled.** The measurement above was the +> whole case for building it and it did not carry. `nav_admin` and `nav_player` +> are therefore **not read by the app at all** — the player and staff drawer rows +> keep their coded `@StringRes` labels and their coded visibility, and this +> section stands as the record of why rather than as a spec. See §8. + + + **A label override replaces a `@StringRes`.** `MenuEntry.labelRes` is an int; the resolved entry carries `label: String?` beside it and the drawer prefers it. That means an admin's label is **not localized** — it is one string for every locale, @@ -445,7 +625,10 @@ which is what an admin typing a label means, and matches the website. - **AC-1 — the no-op proof.** With `theme` absent, `nav_public` absent and no `brand_assets`, the resolved `ColorScheme`, `Shapes`, `Typography` and drawer entry list are **equal** to today's shipped values. A unit test asserts the - full `ColorScheme` equality, not a spot check. + full `ColorScheme` equality, not a spot check. **Card elevation is excluded** + by the phase 2 decision in §2/§5.4 — an untouched instance gains the 4dp the + `runic-gateway` shadow resolves to, and the test asserts that value rather than + the app's former flat one. - **AC-2 — per-field fallback.** A `theme` map carrying one valid token and four malformed ones applies the one and falls back on the four. - **AC-3 — the gates still hold.** An override marking a feature-gated or @@ -474,20 +657,478 @@ PR against `main`. | # | Phase | Ships | |---|---|---| -| **0** | **Contract & appearance store** | `SettingsDto` gains `theme: Map?` and `nav_public: String?`; `SiteAppearance` replaces the bare `BrandDto` in `AppViewModel`; second-stage JSON parse; resume refresh (§5.5). **No visual change** — this phase must be invisible | -| **1** | **Colors** | `ShardPalette` + `LocalShardPalette`; all direct `Color.kt` imports migrated; `RunicGatewayTheme(appearance)`; AC-1 + AC-2 tests | -| **2** | **Radii & shadow** | Ratio-scaled `Shapes` (§5.2), elevation map (§5.4) | -| **3** | **Fonts** | Seven bundled families + licenses; stack → `FontFamily` resolution; `Type.kt` takes its three families from the resolved theme. APK size recorded | -| **4** | **Brand assets** | Logo in the drawer header and top bar, hero on Home (§5.6). Coil + `LocalAssetResolver` already exist; renders nothing when unset | -| **5** | **Public nav: label / order / hidden** | The path→route table (§6.2), `Routes.news(category)`, the merge, drawer wiring. AC-3 | -| **6** | **Public nav: sections & added links** | Drawer groups, `pruneNav` port, link path validation, native-route resolution + Custom Tab fallback (§6.3) | -| **7** | **Authenticated navs** | `GET /api/v1/settings/nav` behind a session-keyed repository; label/hidden for the four mapped rows (§6.4). **Optional — reconsider before starting it** | -| **8** | **Docs, coverage & cutover** | This doc's "as landed" notes and any amendments the build forces, the `PLAN.md` §9 M12 entry refreshed, Sonar coverage for the new modules, AC-5 on-device walk, then `edge` → `main` | +| **0** ✅ | **Contract & appearance store** | `SettingsDto` gains `theme` and `nav_public`; `SiteAppearance` replaces the bare `BrandDto` in `AppViewModel`; second-stage JSON parse; resume refresh (§5.5). **No visual change** — this phase must be invisible | +| **1** ✅ | **Colors** | `ShardPalette` + `LocalShardPalette`; all direct `Color.kt` imports migrated; `RunicGatewayTheme(appearance)`; AC-1 + AC-2 tests | +| **2** ✅ | **Radii & shadow** | Ratio-scaled `Shapes` (§5.2), elevation map (§5.4) | +| **3** ✅ | **Fonts** | Seven bundled families + licenses; stack → `FontFamily` resolution; `Type.kt` takes its three families from the resolved theme. APK size recorded | +| **4** ✅ | **Brand assets** | Logo in the drawer header and top bar, hero on Home (§5.6). Coil + `LocalAssetResolver` already exist; renders nothing when unset | +| **5** ✅ | **Public nav: label / order / hidden** | The path→route table (§6.2), `Routes.news(category)`, the merge, drawer wiring. AC-3 | +| **6** ✅ | **Public nav: sections & added links** | Drawer groups, `pruneNav` port, link path validation, native-route resolution + Custom Tab fallback (§6.3) | +| **7** | **Authenticated navs** | ❌ **cancelled** — was: `GET /api/v1/settings/nav` behind a session-keyed repository; label/hidden for the four mapped rows (§6.4). See "Phase 7, cancelled" below | +| **8** ✅ | **Docs, coverage & cutover** | This doc's "as landed" notes and any amendments the build forces, the `PLAN.md` §9 M12 entry refreshed, Sonar coverage for the new modules, AC-5 on-device walk, then `edge` → `main`. **The walk found two theming defects and phase 8 fixed them** — see "Phase 8 as landed" | Phase 0 is the one with a hard rule attached: **it must change nothing on screen.** Everything after it is additive on top of a store that is already proven not to have moved anything. +### Phase 0 as landed + +- **`theme` is modeled as a raw `JsonElement`, not `Map?`.** The + table in §5.1 is a closed set of string-valued tokens and the server validates + every one on write, so a typed map is what the contract says. But + `kotlinx.serialization` fails the decode of the *whole* object on a value of an + unexpected kind, and `theme` shares its payload with `brand` and `push` — so + one odd token would have blanked the branding and dropped the push relay URL, + which is the opposite of §2's forgiving-on-read. It is coerced field-by-field + in `SiteAppearance.from` instead: a non-string or blank value costs exactly its + own token. +- **The second-stage parse stops at "is this a plain object".** + `data/appearance/SettingsJson.kt` is the Kotlin counterpart of the web's + `lib/settingsJson.js` and makes the same single judgement — absent, malformed, + or a stored `null`/number/string/array all read as **absent**. Reading `items`, + `sections` and `links` out of the parsed object belongs to phases 5 and 6, so + phase 0 ships no half-built nav model. +- **`SiteAppearance.NONE` is the shipped app**, and three different things + resolve to it: no settings rows, a backend that predates the feature, and a + settings call that failed outright. That is §2 expressed as a value rather than + as a rule to remember. +- **A failed *refresh* keeps the last good appearance** rather than falling back + to `NONE`. §5.5 said "best-effort"; the distinction it did not draw is that a + moment of no connectivity on resume must not repaint a themed shard back to the + defaults. Only the initial load can produce `NONE`. +- **The resume refresh lives in `MainActivity`, not `RunicApp`.** The appearance + feeds the theme, which wraps the whole tree including the connect screen, so it + sits beside the theme rather than inside the app shell. It is a second + `LifecycleResumeEffect` next to the session's, and it no-ops unless the state + is `Ready`. +- **`RunicApp` still takes `brand: BrandDto?`.** Threading `SiteAppearance` + further down is phase 1's and phase 5's business; leaving the shell's signature + alone is what makes this phase's diff provably invisible. `AppState.Ready` is + the only place the type changed. +- Tests: `SettingsJsonTest` (6), `SiteAppearanceTest` (8), plus three decode + cases in `PublicDtoTest`. 360 unit tests green, `lintDebug` and `assembleDebug` + clean. `AppViewModel` itself stays untested — its four collaborators are + concrete classes with `Context`/prefs dependencies, and all of the phase's + logic is in the two pure modules above. + +### Phase 1 as landed + +- **`ColorScheme` does not implement `equals`.** AC-1 asks for "the full + `ColorScheme` equality, not a spot check", and material3 1.3.0 simply has no + `equals`/`hashCode` on it (checked against the artifact, not assumed). The + proof is therefore a **field-by-field compare by reflection** over every + `Color`-valued getter — 36 roles in 1.3.0 — rather than a hand-written list of + the roles the mapping happens to set. A role added to Material, or one the + mapping forgets, cannot escape the assertion, and a guard on the role count + fails if the reflection ever stops seeing them. The expected value is a + **verbatim copy of the pre-M12 `ShardColorScheme`** held in the test, the same + device the website used to lock `htmlShell`'s output: the proof is against what + the app used to do, not against what the new code does today. +- **`ShardPillFg` is derived, not a sixteenth token.** §5.1 lists it among + `ThemeComponents.kt`'s themable imports but the token table has fifteen rows + and none of them is it — because its value *is* `ShardCta`'s, both + `--accent-bright`. It follows `cta` under the same rule §5.1 states for + `ShardOnCta`, so the neutral pill's text tracks the CTA fill rather than + freezing at today's literal. +- **An env-accent instance changes, and that is the intended fix.** §5.1 called + the old `RunicGatewayTheme(accent)` "wrong twice over"; correcting it has a + visible consequence worth stating plainly. A shard with a `BRAND_ACCENT_COLOR` + and **no** `theme_visual` row previously had that color on + `primary`/`secondary`/`tertiary` — so its filled CTA buttons carried the accent. + It now seeds `--accent` only, which is `secondary`/`tertiary`, and `primary` + returns to `--accent-bright`. Links and highlights keep the brand color; filled + buttons go back to the light CTA fill the design specifies. An instance that + wants its buttons accented sets the accent from Admin → Appearance, which is + what the token map is for. +- **The palette resolution is pure and the theme is the only composable.** + `ShardPalette.resolve(theme, brandAccent)` takes a token map, and + `shardColorScheme(palette)` takes a palette — neither knows about + `SiteAppearance`, so both AC-1 and AC-2 are plain JVM assertions with no + Compose test rule. `RunicGatewayTheme` is the one place the two meet. +- **The §5.1 grep came out exactly as predicted.** After the migration, + `ui.theme.Shard` imports outside `ui/theme/` are the seven semantic constants + in `ThemeComponents.kt` and the two in `ShardComponents.kt` — nothing themable + left behind, which was the phase's stated correctness risk. +- **`toneColors` became `@Composable`** to read the palette. It is private and + called only from `StatusPill`, so this costs nothing; the alternative — passing + a palette parameter through the pill's public signature — would have leaked the + theme into every call site. +- Tests: `ShardPaletteTest` (9 — the shipped no-op, all fifteen tokens landing in + the right field, AC-2's one-good-four-bad map, unknown tokens ignored, the + brand-accent fallback and the token beating it, a malformed brand accent, and + both derived colors) and `ShardColorSchemeTest` (4). **373 unit tests green**, + `lintDebug` and `assembleDebug` clean. Not exercised on device — that is AC-5, + in phase 8. + +### Phase 2 as landed + +- **The app is flat, so §5.4 is a visible default change.** (It was written here + as the milestone's *only* one. Phase 8 found a second — the card *container* + colour — and a third that is not a change at all but a correction to this + record: an instance with a `BRAND_HERO_IMAGE` and no `brand_assets` row now + draws a hero on Home, because `brand.hero` carries the env default. See + "Phase 8 as landed".) + Material3 1.3.0's filled `Card` is `ElevationTokens.Level0` — 0dp, checked in + the artifact's bytecode, not assumed — and `FeatureCard` was a `Box` with a + clip, a gradient and a border, drawing none of the "soft shadow" its own KDoc + claimed. The `runic-gateway` preset meanwhile selects the *Default* shadow, so + reading §5.4 literally gives an untouched instance 4dp on every card. The org + lead chose that over rebasing the table on the flat baseline (which would have + collapsed `none`/`Soft`/`Default` onto 0dp and left only `Deep` doing + anything). §2 records the exception; the radius half remains a provable no-op. +- **`ShardCard` exists because Material's theme cannot carry elevation.** The + color scheme and the shape scale both reach screens through `MaterialTheme`, + but `Card` takes its elevation as a **default argument** — there is no + composition local behind `CardDefaults.cardElevation()`. So the phase migrated + all **24 `Card(` call sites across 20 files** to a one-line wrapper in + `ThemeComponents.kt`. Every one of them passed nothing but a modifier, which is + why the wrapper's signature is `(Modifier, ColumnScope.() -> Unit)` and the + migration is mechanical. A `Card(` outside that file is now, by construction, a + card the shard cannot theme. +- **`--radius-pill` reaches exactly one composable.** The app has three + `CircleShape` uses and two of them are 8dp status dots (`OnlineDot`, + `LiveChip`); a dot stays a dot however square an admin makes the site. Only + `StatusPill` takes the resolved shape. §5.2 records the literal-px rule the + pill needs because a percentage shape has no dp to scale. +- **`Shapes` *does* implement `equals`** — the opposite of phase 1's + `ColorScheme` finding, and also checked in the bytecode. So the structural + no-op proof is one assertion against a verbatim copy of the pre-M12 scale + rather than a reflection walk. Both the empty theme and the full + `runic-gateway` token map are asserted `==` to `ShardStructure.Shipped`. +- **`FeatureCard`'s literal 12dp became `MaterialTheme.shapes.medium`** — the + same value, so no-op, but now carried by the ratio. `StatBar`'s three + `RoundedCornerShape(3.dp)` stay literal: that is half the height of a 6dp + meter, not a member of the card radius family. +- **The structure resolution is pure, like the palette's.** + `ShardStructure.resolve(theme)` takes a token map and returns shapes + pill + + elevation, so every assertion is a plain JVM test with no Compose rule. + `RunicGatewayTheme` remains the only composable where resolution happens. +- Tests: `ShardStructureTest` (13 — the shipped scale against a verbatim pre-M12 + copy, three no-op paths, Fantasy and Modern scaled onto the app's dp, per-field + fallback, a zero radius, the pill floor, all four shadow options, the + off-catalog blurs, and an unreadable shadow). **386 unit tests green**, + `lintDebug` and `assembleDebug` clean. Not exercised on device — that is AC-5, + in phase 8, where the new shadow should be looked at with the flat build beside + it. + +### Phase 3 as landed + +- **The APK nearly tripled, and that was a decision rather than a discovery.** + §5.3 now carries the measured before/after and the two cheaper options that + were costed and declined. The one number worth remembering: Merriweather is + 75% of the payload, because upstream publishes it as a three-axis variable font + that barely compresses. Anyone revisiting the app's size should start there and + nowhere else. +- **The per-role font list is not the set of values a role can hold.** Two of the + three presets publish a font their own role's dropdown does not offer (§5.3). + The resolution map is therefore global, keyed by the lowercased first family + name, and the tests assert both preset cases by name so a future per-role + "tidy-up" fails loudly. +- **`Typography` implements `equals`** — like phase 2's `Shapes` and unlike phase + 1's `ColorScheme`, checked the same way in the material3 1.3.0 bytecode. AC-1's + type half is one comparison against a verbatim copy of the pre-M12 scale held in + the test, so a stray edit to a size or a letter-spacing in `Type.kt` fails there + rather than quietly redefining what "shipped" means. +- **No `LocalShardTypeface`, deliberately.** The palette and the structure each + needed a composition local for the parts `MaterialTheme` cannot carry; the + families need none. Every text style in the app comes from + `MaterialTheme.typography`, and the two places that override anything + (`NotificationsScreen`'s hint, `BlockRenderer`'s italic title) override the + *style*, not the family. `FontFamily.Monospace` on the recovery-codes screen + stays fixed, as a semantic choice rather than a themed one. +- **`Type.kt`'s `val Typography` became `shardTypography(faces)`**, which is the + whole migration: the three families were referenced from that one file and + nowhere else, so unlike phase 1's colour imports and phase 2's 24 `Card(` sites + there was no call-site sweep at all. +- **Licences live in `app/licenses/`**, one `*-OFL.txt` per family, never under + `res/font/` — aapt rejects a `.txt` there, the M5 gotcha. Three of the seven + carry a Reserved Font Name (Merriweather, Playfair Display, Source Sans 3), + which is a further reason the binaries are taken verbatim rather than subsetted + or instanced locally. +- Tests: `ShardTypefaceTest` (15 — the shipped families and the pre-M12 scale, the + `runic-gateway` no-op, both preset bypasses by name, all thirteen shortlist + options, first-name parsing against quoting/casing/whitespace, per-field + fallback, a colours-only theme, blank and comma-only stacks, and a themed scale + proved to differ from the shipped one *only* in its families). **401 unit tests + green** (386 + 15), `lintDebug` and `assembleDebug` clean. Not exercised on + device — that is AC-5, in phase 8, where IM Fell English's synthesised bold is + the thing to look at. + +### Phase 4 as landed + +- **The empty slot is enforced by layout, not by a conditional at each site.** + Every size and spacing modifier hangs off the image itself, so when the image + is not composed neither is its padding. A caller that wants space below the + hero passes `Modifier.padding(bottom = 16.dp)` rather than a sibling `Spacer`, + and both cases — asset and no asset — come out right without the caller + knowing which it got. This is the detail that makes "renders nothing when + unset" hold without a `brand?.hero != null` check leaking into `HomeScreen`. +- **Three §5.6 questions the spec left open, settled by the org lead before + code**: the top bar falls back to the text rather than going blank; the hero + is a fixed cropped band rather than its intrinsic aspect; the hero sits inside + Home's existing 20dp padding with themed corners rather than going full-bleed. + All three are written into §5.6 above with their reasoning, so they are not + re-litigated from scratch. +- **A failed load is keyed on the URL.** `remember(url)` resets the failure flag + when a resume refresh (§5.5) swaps the logo, so an instance that fixes a broken + upload recovers on the next refresh instead of inheriting the old failure for + the life of the process. +- **The blank check runs on both sides of the resolver.** `BrandDto` defaults + every asset field to `""` rather than null — the server publishes the empty + string for "not set" — and a resolver with no base URL configured may hand a + path straight back. Null out of `brandAssetUrl` is the "draw nothing" signal, + so a blank slipping through either side would put a zero-size image request in + the layout instead of no image at all. +- **No new dependency, no new asset, no APK cost.** Unlike phase 3 this phase + adds nothing to the package: `LocalAssetResolver` (M1) and Coil (M5) were both + already there, and `BrandDto` has carried `logo` and `hero` since M1 without a + reader. The whole phase is one new file plus three call sites. +- Tests: `BrandAssetsTest` (9 — every shape "not set" arrives in, the default + `BrandDto`, `SiteAppearance.NONE`, site-relative and absolute paths, and a + resolver that returns null or blank). **410 unit tests green** (401 + 9), + `lintDebug` and `assembleDebug` clean. **The drawing itself is untested and + cannot be tested here** — the app carries no Robolectric and has no + `androidTest` source set, so a composable body cannot run in a JVM test. Only + the decision of *whether* to draw is pure, which is why `brandAssetUrl` is + pulled out of the composables at all. Everything else about this phase is + AC-5's to catch, and it is the phase with the most riding on that walk: a + cropped hero, a synthesised bold, and a logo's contrast against the top bar + are all things only a screen shows. + +### Phase 5 as landed + +- **An untouched instance gets `APP_MENU` back by identity, not by equality.** + `applyNavOverrides` returns the *same list instance* when there is no stored row, + an empty one, or one with nothing usable in it — so AC-1's claim for the drawer + is a one-line `assertSame` rather than a structural comparison, and there is no + path where an unedited nav is rebuilt and could come out different. "Nothing + usable" is a real case worth the check: a blank label, `hidden: false`, an + unknown path, and a path the app maps but does not surface all say nothing. +- **An untouched row's implicit sort key is its index in the WEBSITE's nav, not + the app's** — the one thing the spec did not settle and the whole sort turns on. + A stored `order` is a position in the site's sixteen-row list, so a key taken + from the app's nine-row block would put explicit and implicit keys on two + incomparable number lines: an unmoved About (app index 8) would sort ahead of an + unmoved Market (web order 14) the moment any row carried an explicit order. This + is why §6.2's table is now numbered and why its order is called load-bearing. + Both tie-breaks are the web's — an explicit order beats a coincidental index, + and two explicit orders keep code order because the sort is stable. + - In practice the editor writes an order for *every* visible row when the admin + drags anything, so a mix of explicit and implicit keys is the stale-row case + rather than the normal one. It still has to resolve predictably. +- **The app's own rows are partitioned off, not sorted.** Contact, Account, + Notifications, the three player groups and the four staff rows have no website + counterpart to be reordered against, so they keep their coded order after the + public block. They already sit there today, which is what makes the partition + the current layout rather than a new one — but it does mean an `APP_MENU` that + interleaved an app-only row *among* the public ones would see it moved to the + tail. Nothing does today; a future row should be added with that in mind. +- **`group` and `section` are read and dropped.** The app renders no sections in + this phase (phase 6) and never renders the admin sidebar's groups at all, and a + value that cannot be honored is better dropped than half-applied. Both stored + shapes are read, though: website phase 10's `{items, sections, links}` and the + bare map phases 6-8 stored, which is unambiguous because every key in it is a + path and so can never be the string `items`. +- **`Routes.NEWS_ROUTE` is declared beside `Routes.NEWS` rather than replacing + it**, because the two are used for different things: the pattern is what + `composable()` and `destination.route` speak, the bare route is what callers + navigate to. Navigating to plain `Routes.NEWS` matches the pattern with no + argument, so the drawer row and the push deep-link (`Routes.forStream`) are + untouched. The consequence to remember: **`destination.route` is now a pattern + carrying a query**, so `RunicApp` compares on `substringBefore('?')` for both + the top-level check and the selected-row check. A future route that takes an + optional argument inherits that for free; one that does not, and is compared + against by hand, will not. +- **`Routes.news()` takes the `PostCategory` enum, not a slug string** (unlike the + existing `Routes.post()`), so an unmapped category cannot reach the NavHost — + the tab strip *is* the enum's entries, and a slug it does not know would select + nothing. `NewsViewModel` falls back to the default feed for an unknown slug + anyway, since a hand-edited settings row can carry one. +- **Phase 5 ships `Routes.news(category)`; phase 6 is its first caller.** Nothing + in the drawer navigates to a category tab, by the §6.2 decision above. It ships + here because it is the table's route builder and the table is this phase's. +- Tests: `NavOverridesTest` (19) + `NavPathsTest` (11) + 2 new `NewsViewModel` + cases (and three existing ones rewritten onto a `SavedStateHandle`). **442 unit tests green** (410 + 32), `lintDebug` and + `assembleDebug` clean. AC-3 is covered three ways — a feature-gated row that an + override relabels, moves to the front and marks `hidden: false` is still not + shown to an admin whose shard does not publish it; role-gated rows stay hidden + from an anonymous caller whatever the row says; and hiding composes with the + gates rather than competing with them. What the JVM still cannot reach is the + same limit phase 4 hit: that the reordered drawer *draws* in the new order, and + that navigating to plain `news` really does match the optional-argument pattern + at runtime, are AC-5's. + +### Phase 6 as landed + +- **The spec's link table was wrong about the website, and that is the finding to + remember.** Three of its rows named paths the site does not serve and two of the + site's real detail routes were missing — see the correction in §6.3. Phase 5 hit + the same class of error in §6.2's row order. Both times the fix was to quote the + website's own source beside the table, so `NavPaths.kt` now carries `App.jsx`'s + route list next to `SiteHeader.jsx`'s `NAV`. +- **Phase 6 does not re-implement phase 5, and AC-1 is unchanged because of it.** + `buildNavTree` hands straight to `applyNavOverrides` when the stored row carries + no sections and no links, so an untouched instance still gets `APP_MENU` back by + identity — the tree build only runs when the admin actually created structure. + The regression test is that an items-only row through the tree equals the same + row through the flat merge. +- **`section` on an item override is read but not counted as "usable".** A + section-only override says nothing to a flat list, so `NavOverride.isEmpty` + deliberately excludes it and an instance that only ever grouped rows still gets + its coded list back by identity from `applyNavOverrides`. The tree adds its own + check. This is the one place the two consumers of a stored row disagree about + what "empty" means, and they have to. +- **A dangling reference degrades to "no grouping", never to "no row".** A member + of a section that does not exist — because the admin deleted it, or its label + was blank and it was dropped — is an ordinary top-level row, not a row that + vanished with its section. Same for a link naming an unknown section: its + destination is still good, only the grouping was wrong. +- **`visibleEntries` was split into `isEntryVisible`.** `pruneNav` applies the + predicate inside a section as well as at the top level, and both callers must + ask exactly one question, or a sectioned row could end up gated by a rule its + top-level twin is not. The list form now just filters with it. +- **Sort keys extend phase 5's number line rather than starting a new one.** A + coded row keys on its index in the website's sixteen-row nav; an admin-created + section or link with no stored order keys on `16, 17, …` in creation order, so + it appends after the coded rows instead of jumping to the front on a `0` + default. Both tie-breaks are still the web's. +- **What is not gated is as load-bearing as what is.** An added link carries no + role or feature check — the screen behind it enforces its own — so a section + holding one is never emptied by the caller's role, while a section holding only + gated rows is dropped rather than drawn as a header over nothing. Both are + tested; the second is the case `pruneNav` exists for. +- Tests: `NavTreeTest` (27) + 7 new `NavPathsTest` cases. **476 unit tests green** + (442 + 34), `lintDebug` and `assembleDebug` clean, no new lint findings. + Phase 4's limit still holds — no Robolectric and no `androidTest` source set — + so the section header's indent, the hand-off icon, and that a Custom Tab really + opens are AC-5's, not the JVM's. + +### Phase 7, cancelled + +Reading `nav_admin` / `nav_player` is **not scheduled**. The measurement in §6.4 +is the reason and it stands: the two authenticated navs reach four app rows +between them (`/player`, `/account`, `/admin`, `/admin/moderation`), because the +website sidebar's other ~18 rows are admin *configuration* the app deliberately +excludes and two of the app's four staff entries are aggregates with no single +web row to be renamed from. Four rows does not pay for a new authenticated fetch, +a session-keyed cache and its teardown on sign-out. + +Consequences, all of them wanted: + +- the app never calls `GET /api/v1/settings/nav`, and the "a player may read it" + note in §3 becomes moot for this client; +- the player and staff drawer rows keep their coded `@StringRes` labels, so those + labels stay **localized** — which is the one thing the app gives up by not + honoring an admin's override, and the trade reads better in this direction for + four rows; +- §6.4's label-and-hidden-only rule and the `label: String?`-beside-`labelRes` + mechanism still ship, because **phase 5 needs both** for the public nav. Nothing + in phases 0–6 was scaffolding for phase 7. + +If it is ever picked up, §6.4 is the spec and the merge would reuse phase 5's. + +### Phase 8 as landed + +Phase 8 was scoped as docs, coverage and the cutover. The AC-5 walk turned it +into a phase with code in it, which is the outcome the walk existed to produce. + +**AC-5 confirmed what the JVM could not reach.** Two passes on the AVD against a +local website, the themed one walked at every role rung. Everything phases 1–6 +had deferred to it held: the Fantasy palette on screen; radii visibly square +against the untouched build's rounded; **EB Garamond arriving in the *sans* role**, +which is phase 3's preset-bypass finding proved on a device rather than in a test; +the logo replacing the top bar's text with the 6× width cap holding; the hero +centre-cropped into its 180dp band with the top and bottom of a 4:3 image cut; +reorder, relabel and hide; the section header with its indented children; the +added link with a native route carrying **no** badge and the handing-off one +carrying it; and the Custom Tab genuinely opening — confirmed as +`CustomTabActivity` in `dumpsys activity`, not by eye. AC-3's role dimension held +at every rung: an anonymous caller saw the reordered, sectioned public nav with +**no player or staff row leaking**, and the player rung stopped at `My houses`. +AC-4 fell out for free when the second AVD started against an unreachable server: +shipped theme, coded menu, one retry, no error surface. + +**A correction the walk forced, with no code behind it.** An untouched instance +**gains a hero on Home**. `brand.hero` carries the `BRAND_HERO_IMAGE` env default +— `/assets/img/runic-emblem.png` on this one — so "admin has set nothing" does not +mean "the slot is empty", and phase 4's `BrandHero` draws it. Nothing is wrong +here: the website's own Home shows the same emblem, and §5.6's rule is that a +blank slot draws nothing, which still holds. What was wrong is the *record* — +phase 2 claimed the card shadow was the milestone's only visible change to an +untouched app, and this was the second before the card container made it three. +Worth stating plainly because a shard operator upgrading the app sees it without +having touched Appearance at all. + +**Two defects, one root cause, and it is phase 2's.** Material takes some themed +values as **default arguments** rather than from the theme. Phase 2 found this for +card elevation and answered it by sweeping every `Card(` into `ShardCard(`. Nobody +then asked whether *colour* and *shape* had the same problem. They did: + +- **Every `ShardCard` drew in Material's grey, not the shard's panel colour.** + `CardDefaults.cardColors()` takes its container from `surfaceContainerHighest` + — `FilledCardTokens.ContainerColor`, checked in the material3 1.3.0 artifact's + bytecode the same way phases 1 and 2 checked theirs — and `shardColorScheme` + mapped `surfaceContainer`, `…High` and `…Low` but not `…Highest`. All 26 call + sites in 20 files. `FeatureCard` was unaffected because it reads + `LocalShardPalette` directly, which is exactly why Home looked right and every + other screen did not — the single most misleading thing about the bug. +- **The drawer's selected row ignored `--radius-pill`.** `NavigationDrawerItem` + takes `shape` as a default argument (`CircleShape`); the three call sites set + `colors` and never `shape`. On Fantasy every other radius went square while the + selected row stayed a full pill. + +**The card fix changes the untouched app, and that is the honest framing.** The +grey cards are not an M12 regression — an untouched instance draws them too, and +has since M5. M12 only made it visible by theming everything around them. So +fixing it moves the shipped app's cards from Material's grey to `--panel-flat`: +**the milestone's second deliberate change to a shard that has set nothing**, +alongside phase 2's card shadow. §9's last bullet had already called this shot — +"if a screen looks wrong under a warm preset, that is a token the screen should +have been reading and did not" — and this is that, at 26 call sites. + +There is no version of the fix that spares untouched instances: the shipped +palette *is* the `runic-gateway` preset, so the value the cards should take is the +same value either way. AC-1 therefore **records** the change instead of absorbing +it — every other role is still asserted byte-for-byte against the verbatim pre-M12 +scheme, and the two that moved are named, given their new values, and checked to +have genuinely differed before, so the test cannot pass by coincidence. + +**The role sweep, so the next person does not redo it.** Of the roles +`shardColorScheme` leaves at `darkColorScheme()`'s defaults, exactly one had a +live reader in this app: `surfaceContainerHighest`. Checked against the token +class of every Material component the app actually draws — +`NavigationDrawerTokens`, `SheetBottomTokens`, `AssistChipTokens`, +`FilterChipTokens`, `TopAppBarSmallTokens`, `BadgeTokens` — all of which read only +roles already mapped. `scrim` is read by the modal drawer and **stays Material's +black on purpose**; a tinted scrim is a design change, not a gap. +`surfaceContainerLowest` is mapped for consistency with `…Low` and has no reader. +`primaryContainer`, `tertiaryContainer`, `inverse*`, `surfaceBright`, `surfaceDim` +and `surfaceTint` are latent — the app draws no Snackbar, tonal button, elevated +or outlined card. Adding one of those components means checking this list again. + +**Coverage: the exclusion list had gone stale under the milestone.** +`sonar.coverage.exclusions` carried a `ui/theme/**` directory glob from the M11 +coverage push, when that directory held only constants and composables. M12 put +three pure resolvers in it — `ShardPalette`, `ShardStructure`, `ShardTypeface`, +the reason phases 1–3 could prove their no-op invariants as JVM assertions — and +the glob was dropping all three out of the denominator at 98%, 100% and 100%. It +is now the one file it was really about, `ui/theme/Theme.kt`. This did not rescue +the gate: M12's already-measured code (`data/appearance/` and `ui/navigation/`, +93–100%) clears `new_coverage ≥ 50` on its own. It makes the number honest about +which code the tests hold, so deleting those tests would move it. + +**Tests: 477 green** (476 + the card-container assertion), `lintDebug` and +`assembleDebug` clean. The JVM limit from phase 4 is unchanged and is why this +phase needed a device at all: no Robolectric, no `androidTest` source set, so a +composable body cannot run in a unit test. Both defects were invisible to 476 +passing tests and obvious within two minutes of looking at a themed screen. + +**Rig notes, for the next walk.** The emulator reaches a local website through +`adb reverse tcp:3000 tcp:3000`, which lets the app use `http://127.0.0.1:3000` +and the debug `network_security_config`'s loopback exception **unchanged** — do +not add `10.0.2.2` to it for a test rig. The Custom Tab resolves against the same +base URL, so hand-off links work under the same tunnel. + ## 9. Out of scope - **Light mode / a light preset.** The website cancelled its Parchment phase; the