fix(frontend): land #294 + #296 with review fixes — AI Workspace retention copy, Settings landmark, revision labels - #300
Merged
Conversation
…rite AI Workspace copy Reorders the authenticated sidebar into flagship (Dashboard, Repositories, Upload), a labelled Analysis group (Architecture, Dependency Graph, Engineering Review, Insights, Documentation), and a separate Assist group for AI Workspace, replacing the previously unlabeled "More" section. Settings moves out of that grouping and is pinned to the bottom of the sidebar, visually separated above the account identity row. Also rewrites AI Workspace's page subtitle and zero-message empty state so it states plainly that it answers from sealed structural facts already computed by analysis, sends no source-file contents, and keeps no memory between sessions, rather than reading as a general chat tool.
The new "Analysis" and "Assist" nav section headers inherited the prior "More" label's text-muted-foreground/70 class, which measures 2.80:1 against the sidebar background -- below the 4.5:1 AA minimum. Dropping the /70 opacity modifier uses the token at full strength (4.98:1 light, 8.12:1 dark), which is what the Prototype Browser Acceptance CI gate caught on PR #294.
…board
Adds a single summary line above the metric cards showing which
repository's analysis is most current and at what revision, derived
only from fields the repository list already fetches for this page
(status, analysedAt, revision) -- no new endpoint, no fabricated data.
A "review findings by state" fact was considered but not added: that
data lives behind GET /analysis/{id}/review (engineering-review.v2),
which this page does not fetch, and inventing it here would violate
the authentic-data-only rule rather than extend it.
…s landmark Review follow-ups to the sidebar regrouping (#289) and the dashboard latest-analysis line, applied on top of both changes. AI Workspace copy stated the opposite of what the surface does. The subtitle claimed "nothing is remembered once you leave this page" and the empty state claimed "no history is kept between sessions", but conversation turns are persisted per owner and repository and restored on return (#231, `ai_conversation_messages`), and the last eight turns are replayed to the provider as context. Read as a privacy assurance those statements were false, so they now describe the real behaviour instead. The accurate parts -- sealed-snapshot grounding and no source-file contents -- are kept, and the active-repository header regains the repository name it had lost. Settings was pinned by moving it into a bare footer div, which removed it from every navigation landmark and left it undiscoverable to landmark-based screen-reader navigation. It keeps the same pinned position and separator but now sits in its own labelled nav. The utility lookup also changes from `find` to `filter` so a second utility surface cannot silently vanish. Dashboard revision labels now render as `kind value` for both kinds, matching Insights and Engineering Review, rather than showing a bare seven-character hex string for git. The abbreviation is display-only, so the full immutable value is carried in `title`; a prefix is not an identity. The ordering test now places the winner neither first nor last, so picking either end of the list can no longer pass by accident. Retires the #238 accessibility baseline entry: the `More` label it was keyed to no longer exists, so the automated allowance matched nothing and would only have masked a regression on a removed element.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Lands the work from #294 (sidebar Analysis/Assist grouping, pinned Settings, AI Workspace copy) and #296 (dashboard most-recently-analysed line) rebased onto current
dev, plus review fixes for three defects found in them. Both original branches live on a fork this session cannot push to, so their commits are carried here unchanged and with original authorship; the review fixes are a separate commit on top.Linked issue
Related to #289, #295. Retires the automated baseline entry for #238.
Not using
Closes: #289's mobile-navigation and engineer-feedback criteria remain observational, exactly as #294 flagged.Roadmap alignment
What changed
Carried unchanged from #294 and #296 (three commits, original authorship):
Analysis/Assistsections, Settings pinned to the bottom,NavGroupwidened toflagship | analysis | assist | utility.GET /repositoriesdata it already fetches.Review fixes (
11baa0f):AIWorkspacePage.tsx— corrected false retention claims. The new subtitle said "nothing is remembered once you leave this page" and the empty state said "no history is kept between sessions". Both are the opposite of the shipped behaviour: turns are persisted per owner and repository inai_conversation_messagesand restored on return (AI Workspace loses conversation history on navigation #231 — the model docstring states history "survives navigation away and back"), anduseAIWorkspace.ts:100replays the last eight turns as provider context. Read as a privacy assurance these were false. The copy now describes the real behaviour; the accurate claims (sealed-snapshot grounding, no source-file contents) are kept. The active-repository header also regains the repository name it had lost.Sidebar.tsx— Settings returned to a navigation landmark. Pinning it moved it into a bare footerdiv, removing it from every landmark and leaving it undiscoverable to landmark-based screen-reader navigation. Same pinned position and separator, now inside its ownnavelement labelledSettings. The utility lookup also moves fromfindtofilterso a second utility surface cannot silently disappear.DashboardPage.tsx— symmetric, non-lossy revision labels.gitrendered as a bareabcdef1with no kind whileuploadrendered asupload deadbee. Both now render askind value, matchingInsightsPageandEngineeringReviewPage. The abbreviation is display-only, so the full immutable value is carried intitle— a 7-character prefix is not an identity (RFC-0001 §3).accessibility.spec.ts+WCAG_2_2_AA_BASELINE.md— retired the bug: sidebar section label fails WCAG AA contrast #238 allowance. It was keyed tosecondary-navigation-label, the element feat(frontend): group sidebar into Analysis/Assist, pin Settings, rewrite AI Workspace copy #294 removes, so it matched nothing and would only have masked a regression on a removed element. The underlying contrast issue is genuinely fixed bya4e30fe.DashboardPage.test.tsx— the ordering test now places the winner neither first nor last, so an implementation that simply took either end of the list can no longer pass by accident.Compatibility with RFC-0002 and #286
Both PRs were opened before RFC-0002 (#297) and #286 merged. Verified compatible:
repository_lineagestable … or alter any API or frontend surface", and that implementation is blocked pending independent ratification (Q5).Repository.revisionfield this PR renders. The dashboard line stays valid.RepositoriesPage, which neither PR modifies.devwith no conflicts.Forward note, no action now: once lineage lands, the dashboard line will likely want a lineage
display_namerather than the per-rowname. That is gated behind the RFC's own implementation block.Acceptance criteria completed
productSurfacesremains the single source of truth for navigation exposure, order, and group.analysedAtand real revision identity (feat(frontend): surface most recently analysed repository on the dashboard #295).Testing performed
Playwright acceptance suite, run in full locally against a real backend (Python 3.13 venv, seeded disposable fixtures):
All six
accessibility.spec.tsWCAG 2.2 AA checks pass, including authenticated application shell and sidebar — this is the direct evidence that the newAnalysis/Assistlabels meet AA contrast, that removing the #238 allowance does not expose a violation, and that the addedSettingsnavigation landmark introduces no landmark conflict.The single local failure was
prototype-surfaces.spec.ts:74, waiting on.monaco-editor. It was environmental, not a regression:@monaco-editor/reactis used with no localmonaco-editordependency, so it fetches from the jsdelivr CDN, which is unreachable from that sandbox (curlexits 56). Nothing in this PR touches the evidence or code-preview path. Confirmed on CI: the Prototype Browser Acceptance job passed, along with all other checks (Frontend, Backend, API Contract Drift, Repository Hygiene, and both CodeQL analyses).Screenshots
None attached. The changes are copy, one landmark wrapper, and a revision label; the assertions above cover them, and no new visual surface was introduced.
Security and data considerations
One directly relevant item: the AI Workspace previously told users nothing was retained, while conversation turns are in fact persisted server-side per owner and repository and replayed to the provider as context. That was a false privacy assurance, corrected here. No auth, owner-scoping, secrets, logging, or migration changes; owner-scoping on
/ai/conversationsis unchanged.Dependencies and blocked work
Scope changes or remaining work
Contributor checklist
devupstream/devupstream/devCloses) is used only because the issue is fully resolved (not used here)