UX review P1+P2: cycle-ordered nav, one save model, feed delivery reasons

[richardmhope]

Replaces #266, which GitHub auto-closed when its base branch (#265) was deleted on merge and then refused to reopen. Same branch, rebased onto main; the P0 commit it was stacked on is now upstream.

The second half of the Claude Design UX review, following #265. Same routes, same forms, same Alpine bindings — reorganisation only.

Nav — ordered by the intelligence cycle

Analysts get Workspace · Collect (notebooks, feed reader, tasking) · Produce (reports) · Discover. Tasking moves out of Administration, where it never belonged.

The admin rail's eleven links collapse to four, with the six integration/governance consoles behind the /admin hub added in #265. Nothing becomes unreachable: every collapsed console is still a ⌘K jump target, and a test walks each role's palette and asserts every destination resolves — the safety net for the collapse, since a dead entry there would strand a whole console.

"Matrix" becomes "ATT&CK coverage". The ⌘K trigger gets its own glyph — it and Search were both magnifiers, which read as one feature.

Editor — one save model, one place to publish

"Save draft" is gone: the editor already autosaved, so two controls reported one thing. A <noscript> submit keeps the form usable if Alpine never loads.

Dock tabs are verb-labelled and kept to four — Cite / Classify / Link / Publish (plus Assist, conditional on AI being enabled). Link merges the requirement and audience forms; Publish holds the lifecycle stepper and rendered PDFs, with the one available transition pinned in the dock footer, replacing the separate subhead stepper + submit row. The repeated per-tab ARIA wiring becomes a single Jinja macro.

Notebook — consistent add-forms

Every add-form sits behind a + Add … <details>, opened by default only when its section is empty. Native, so it costs no JS and no CSP surface.

Feed — why it reached you, and how to close the loop

Every row now says why it was delivered: own requirement ("Answers your RFI") → tag subscription → audience group → level preference. The reason is re-derived from the same rules dissemination.matched_stakeholders routes by, rather than stored on DisseminationEvent — no schema change, and no stored copy that can drift from the rule that actually applied.

Rows bucket into Today / This week / Earlier server-side; an All ⇄ Unread pill filters them client-side (All is the default, so the server render is complete without Alpine).

A row answering your own requirement offers "Mark satisfied →", deep-linking to the product's #feedback card with that requirement preselected.

One deliberate deviation from the handoff

The design asked for an inline POST. POST /reports/{id}/feedback requires a usefulness rating, so a one-click version would have to fabricate a rating the reader never gave. The deep-link uses the same endpoint and costs one extra click, but the recorded feedback is theirs. The ?requirement= parameter only ever preselects a requirement the reader owns and the report answers — a test pins that it cannot surface anyone else's.

Verification

621 passed, 11 skipped, plus ruff / bandit / vulture / mypy / djlint / biome. New tests cover the rail's cycle ordering, the four-link admin rail, every ⌘K destination per role, the editor's single save path and four-tab dock, and each delivery-reason branch. Smoke-run against the real app per role.

🤖 Generated with Claude Code

[icebergai-review-bot]

IcebergAutoReview

Verdict: request_changes

Two correctness issues undermine the feed’s stated behavior: the no-JavaScript view hides read items, and delivery reasons are not reliably derived from the routing decision.

Findings

• [src/iceberg/templates/feed.html:42] Read rows and all-read buckets receive x-cloak. The global CSS defines [x-cloak] { display: none !important; }, so if Alpine fails or JavaScript is disabled these products remain hidden. This contradicts the intended complete server-rendered default and can make a non-empty feed appear empty. Keep the x-show behavior but do not cloak content that must be visible before hydration.
• [src/iceberg/services/feed.py:27] delivery_reason() does not actually reconstruct why the historical DisseminationEvent was created. Requirements are not a predicate in matched_stakeholders, while subscriptions, audience memberships, and preferences are read from their current mutable state rather than their publish-time state. A later subscription or requirement link can therefore be presented as the delivery reason, and removal can replace the real reason with an unrelated fallback. The UI should distinguish “answers your RFI” from the routing reason, or persist sufficient publish-time routing metadata to report the historical cause accurately.

Validation

• git diff --check main...HEAD passed.
• Confirmed statically that x-cloak is globally styled as display: none !important.
• Targeted pytest execution was attempted, but the checked-out environment lacks project dependencies (ModuleNotFoundError: fastapi); uv run could not create .venv because the review workspace is read-only.

Residual risks / optional notes

• Current-head GitHub CI is green and remains the broad validation baseline; local runtime tests could not be executed in this workspace.

Automated review by Codex 21f443263a8a using IcebergAutoReview.

[richardmhope]

Fixed in 8252706. Both findings were correct.

Read rows were cloaked. You're right that this contradicts the PR's own claim of a complete server render — [x-cloak] is display:none !important, so a fully-read feed rendered empty whenever Alpine failed to load. x-cloak belongs on content that should be hidden until hydration; these rows must be visible before it. Removed from the rows and the all-read bucket wrappers. A test now asserts no feed row carries it, scoped so that base.html's ⌘K overlay — which should be cloaked — doesn't mask a regression.

The routing-cause claim was wrong, and I over-claimed it in the description. Requirements genuinely are not a predicate in matched_stakeholders, so "Answers your RFI" was never why a product was routed; and reading subscriptions/audience/preference from current state meant a subscription added after delivery could be shown as the historical cause.

Taking your first option: delivery_reason becomes delivery_context, reporting two independent, present-tense facts — whether the product answers a requirement the reader raised (a fact about the product), and how it matches their preferences as they stand ("Matches your Volt Typhoon interest"). Neither is presented as the publish-time decision.

I did not take the persistence option here: it needs a column on DisseminationEvent plus a migration, which is a poor fit inside a UX PR, and it cannot backfill existing events either. The module docstring and ARCHITECTURE.md now say plainly that historical fidelity requires persisting the match at publish time rather than inferring harder — so the constraint is documented where the next person will meet it, not left implicit. Happy to do that as a follow-up if you'd rather it land now.

Full suite green (622 passed, 11 skipped) plus ruff/bandit/vulture/mypy/djlint/biome.

[icebergai-review-bot]

IcebergAutoReview

Verdict: request_changes

The no-JavaScript visibility blocker is fixed, but the revised present-tense delivery context can still make a false claim after preferences change.

Findings

• [src/iceberg/services/feed.py:58] _match() treats any non-null preferred_intel_level as a match without comparing it to report.intel_level. A previously delivered TACTICAL report therefore displays “Matches your TACTICAL preference” after the reader changes their current preference to STRATEGIC. This directly contradicts the new present-tense contract and the template tooltip claiming the explanation is based on current preferences. Compare the levels and represent a non-current match honestly; add regression coverage for a changed, nonmatching preference.

Validation

• git diff --check 21f443263a8a4fd3aa89ddb060fc90750c3d5b14..HEAD passed.
• Inspected the changed feed service/template against dissemination.matched_stakeholders; confirmed dissemination requires preferred_intel_level to be unset or equal to the report level.
• timeout 120s uv run pytest -n0 tests/test_feed_context.py -q was attempted but could not create .venv because the review workspace is read-only.

Residual risks / optional notes

• Current-head GitHub CI is green and remains the authoritative broad validation baseline; targeted runtime validation was unavailable locally.

Automated review by Codex 8252706c0e68 using IcebergAutoReview.

[richardmhope]

Fixed in 0f24cbd. Correct, and it was a bug in the fix I'd just written.

_match treated any non-null preferred_intel_level as a level match without ever comparing it to report.intel_level — and then built the label from the report's level while calling it the reader's preference. So a TACTICAL product delivered under a TACTICAL preference read "Matches your TACTICAL preference" after they switched to STRATEGIC: false about the match, and false about what they prefer.

Delivery is never retracted, so the product legitimately stays in the feed. The chip now names both sides — TACTICAL · outside your current STRATEGIC preference — under a distinct level_changed kind, so the template can style it differently later without re-deriving the state. Regression test asserts both levels appear and that "Matches your" does not.

That leaves the three preference inputs consistent: tag and audience were already true intersections; level was the one taken on trust.

Full suite green (623 passed, 11 skipped) plus ruff/bandit/vulture/mypy/djlint/biome.

[icebergai-review-bot]

IcebergAutoReview

Verdict: request_changes

The level label now correctly compares both levels, but the new mismatch state still produces a contradictory causal tooltip in the feed.

Findings

• [src/iceberg/services/feed.py:69] level_changed truthfully says the report is outside the reader’s current preference, but feed.html renders every match with the tooltip “Why this is in your feed, based on your current preferences.” For this state, current preferences explicitly would not deliver the report, so the tooltip restores the false causal claim this follow-up is intended to eliminate. Use non-causal wording such as “How this product relates to your current preferences,” and cover the rendered mismatch state.

Validation

• git diff --check 8252706c0e68ebc627160b5dd55ee5eb1f9767b0..HEAD passed.
• Inspected the follow-up delta, feed template consumer, and dissemination.matched_stakeholders routing rules.
• timeout 120s uv run pytest -n0 tests/test_feed_context.py -q was attempted but could not create .venv because the review workspace is read-only.

Residual risks / optional notes

• Current-head GitHub CI is green and remains the authoritative broad validation baseline; targeted runtime validation was unavailable locally.

Automated review by Codex 0f24cbde9609 using IcebergAutoReview.

[richardmhope]

Fixed in d87f730.

Fair catch: the label stopped claiming a level_changed product matches the reader's preferences, but the tooltip wrapping it still read "Why this is in your feed, based on your current preferences" — and for that state those preferences would not deliver the product at all. The chip and its own tooltip contradicted each other, which put the causal claim back one layer up.

Now "How this product relates to your current preferences" — a description of a relationship, not an explanation of a delivery, and true for all five match kinds.

Covered by a test that renders the changed-preference state end to end: it asserts the mismatch label appears, the non-causal tooltip is present, and the causal phrasing is absent from the page — so this can't regress via the template alone, which is how it survived the last two rounds.

Full suite green (624 passed, 11 skipped) plus ruff/bandit/vulture/mypy/djlint/biome.

[icebergai-review-bot]

IcebergAutoReview

Verdict: approve

The prior blocker is resolved. The changed-preference feed state now uses accurate, non-causal tooltip wording, with an end-to-end regression test covering both the mismatch label and absence of the former causal claim. No regressions found in the follow-up delta.

Findings

• No blocking findings.

Validation

• git diff --check 0f24cbd..HEAD — passed
• Inspected the complete two-file follow-up delta and affected feed service/template context
• Verified HEAD is a single follow-up commit based on the prior reviewed head, and main is the merge base
• timeout 120s uv run pytest -n0 tests/test_feed_context.py -q — attempted, but the read-only workspace prevented uv from creating .venv

Residual risks / optional notes

• Targeted runtime validation was unavailable locally; current-head GitHub CI is green and remains the authoritative broad validation baseline.

Automated review by Codex d87f7302baee using IcebergAutoReview.