refactor: drop the header-order comment and the rest of the narration

The comment on AppHeaderActions claimed the component pins itself to the right.
It does not — the sibling content block takes `flex-1` and pushes it there — and
the rule it stated was an instruction to call sites, not a fact about the twelve
characters below it. The convention now lives in docs/list_detail_pages.md, next
to the detail-page recipe that needs it.

Two more went the same way, by making the code carry the point instead: the
knowledge header's two ids are now `documentId` and `routeKnowledgeId`, so
there is nothing to warn about, and the separator that doubles up is gated on a
named `hasViewRow`. In the pager spec the injected delay is held by an assertion
that counts the samples taken before the sibling appears — removing the delay
now fails the test instead of quietly emptying it.

Across the e2e specs, six comments that only explained why a test exists are
gone and five more are down to one line.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

frontend/docs/list_detail_pages.md

@@ -568,6 +568,22 @@ const entityNav = useEntityDetailNavigation(isNew ? null : entityId); // null wh
 // both reading `controller={entityNav}`.
 ```
 
+#### Ordering the header actions
+
+`<AppHeaderContent>` takes the remaining width (`flex-1`), so `<AppHeaderActions>` ends up
+against the right edge and grows leftward. Two rules follow:
+
+1. **Controls that appear on a data condition go first** in the children. Everything after
+   them keeps its position when they arrive late — the flow header's Report button loads with
+   the task list, and the pager beside it must not move under a cursor that is clicking Next.
+2. **Controls that are always meaningful for the route render always**, taking `disabled` from
+   an explicit loading flag rather than being unmounted while the entity is null. Unmounting
+   collapses the cluster for the length of every fetch, which costs a step per pager click.
+
+The flow header is the reference: `Report · favourite · pager · actions menu`. A page whose
+entity can be genuinely absent (a not-found card) is the exception — it renders no actions at
+all, because none of them are meaningful there.
+
 ## Design rationale
 
 ### Why URL > storage

frontend/e2e/mocks/cassettes/settings-prompts.ts

@@ -19,12 +19,8 @@ const agentPrompt = (system: PromptType) => entity('AgentPrompt', { system: prom
 const agentPromptPair = (system: PromptType, human: PromptType) =>
     entity('AgentPrompts', { human: prompt(human), system: prompt(system) });
 
-/**
- * A realistic Go `text/template` for the prompt detail route: the backend parses these with
- * text/template, so the `{{.Var}}` atoms and the fenced command must survive the editor's
- * parse/serialize round-trip. Deliberately has no list-nested fence — that is a separate,
- * already-tracked editor defect and would make this spec assert a known bug.
- */
+/** No list-nested fence on purpose: that editor defect is tracked separately, and including it
+ * here would make the spec assert a known bug. */
 export const RICH_PROMPT_TEMPLATE = [
     '# Pentester',
     '',

frontend/e2e/mocks/cassettes/templates.ts

@@ -23,12 +23,7 @@ export const TEMPLATE_SEED = makeTemplate('11', 'E2E Seed Template', 'Scan the t
 
 const flowTemplates: ResultOf<typeof FlowTemplatesDocument> = { flowTemplates: [TEMPLATE_SEED] };
 
-/**
- * Non-trivial body for the template detail route, whose editor loads it from the server.
- * Carries the `{{PLACEHOLDER}}` atoms a flow template is built from, so a parse/serialize
- * round-trip that drops them fails the spec. No list-nested fence — that is a separate,
- * already-tracked editor defect.
- */
+/** No list-nested fence on purpose: that editor defect is tracked separately. */
 export const RICH_TEMPLATE_TEXT = [
     '# Recon',
     '',

frontend/e2e/route-manifest.unit.test.ts

@@ -20,11 +20,7 @@ const EXCLUDED: Record<string, string> = {
     '/templates/new': 'create-mode variant of the template detail page',
 };
 
-/**
- * Dynamic route builders are functions, so the static walk below cannot see them — a new detail
- * route could otherwise stay outside every sweep with nothing failing. Each builder must say where
- * it is covered, or why it is not; the test asserts this list matches the builders that exist.
- */
+/** Route builders are functions, so the static walk below cannot see them. */
 const DYNAMIC_ROUTES: Record<string, string> = {
     flow: 'manifest entry (routes.flow("5"))',
     flowReport: 'not swept: needs a finished-flow report cassette',

frontend/e2e/specs/cross/a11y.spec.ts

@@ -30,8 +30,7 @@ for (const theme of THEMES) {
         test.describe('settings providers (populated)', () => {
             test.use({ cassette: populatedSettingsProvidersCassette() });
 
-            // Not a duplicate of the manifest scan: that one sweeps the empty state, which
-            // renders no provider cards at all.
+            // The manifest scan sweeps this route empty, so it renders no provider cards.
             test('provider cards have no axe violations', async ({ page }) => {
                 await page.goto('/settings/providers');
                 await expect(page.getByText('My Custom Endpoint')).toBeVisible();

frontend/e2e/specs/cross/palette.spec.ts

@@ -6,7 +6,6 @@ import { buttonVariants } from '@/components/ui/button';
 import { expect, test } from '../../fixtures/test.ts';
 import { ROUTE_MANIFEST } from '../../routes.ts';
 
-// Keyed off the unions so a new variant joins the sanctioned set without a manual edit.
 const BADGE_VARIANTS = Object.keys({
     blue: true,
     default: true,
@@ -43,11 +42,6 @@ const ACCEPTED: Record<string, string[]> = {
     '/resources': ['button: hover:text-blue-400'],
 };
 
-/**
- * The contrast gate probes cva output, so a colour written straight into a page — the shape of the
- * one badge defect that shipped — is outside it by construction. This walks the rendered DOM
- * instead: every badge and button must draw its colour from the variant set or from a token.
- */
 test.describe('palette compliance', { tag: '@cross' }, () => {
     for (const entry of ROUTE_MANIFEST) {
         test.describe(entry.path, () => {

frontend/e2e/specs/crud/template-detail.spec.ts

@@ -4,8 +4,6 @@ import { expect, test } from '../../fixtures/test.ts';
 import { expectCleanPage } from '../../helpers/errors.ts';
 import { RICH_TEMPLATE_TEXT, TEMPLATE_DETAIL, templateDetailCassette } from '../../mocks/cassettes/templates.ts';
 
-// The other route whose editor loads server content: the templates list spec only exercises
-// create mode, so nothing covered the load path in the production bundle this tier runs.
 test.describe('template detail', { tag: '@coverage' }, () => {
     test.use({ cassette: templateDetailCassette() });
 

frontend/e2e/specs/flows/pager.spec.ts

@@ -29,8 +29,6 @@ test.describe('flow pager', { tag: ['@flows', '@smoke'] }, () => {
         await page.goto('/flows/5');
         await expect(header.getByText('E2E Alpha')).toBeVisible();
 
-        // Without the delay the cassette answers before the first sample and the loop below
-        // passes against an unmounting pager — measured.
         await page.route('**/graphql', async (route) => {
             await new Promise((resolve) => setTimeout(resolve, 300));
             await route.fallback();
@@ -44,6 +42,7 @@ test.describe('flow pager', { tag: ['@flows', '@smoke'] }, () => {
             for (let index = 0; index < 10; index += 1) {
                 taken.push({
                     hasPager: !!document.querySelector('header button[aria-label="Next"]'),
+                    isSiblingShown: document.querySelector('header')?.textContent?.includes('E2E Beta') ?? false,
                     path: window.location.pathname,
                 });
                 await new Promise((resolve) => setTimeout(resolve, 30));
@@ -55,6 +54,9 @@ test.describe('flow pager', { tag: ['@flows', '@smoke'] }, () => {
         await expect(page).toHaveURL(/\/flows\/6$/);
         await expect(header.getByText('E2E Beta')).toBeVisible();
 
+        // Guards the delay above as much as the pager: without it the sibling lands inside the
+        // first sample and the loop measures nothing.
+        expect(samples.filter((sample) => !sample.isSiblingShown).length).toBeGreaterThan(2);
         expect(samples.every((sample) => sample.hasPager)).toBe(true);
         expect(samples.map((sample) => sample.path)).not.toContain('/flows');
 

frontend/e2e/specs/real/stand-smoke.spec.ts

@@ -28,9 +28,7 @@ test.describe('stand smoke', { tag: '@stand' }, () => {
             // span tag matters: NavLink puts aria-current="page" on the active
             // settings-sidebar <a> at runtime, the breadcrumb title is a span.
             await expect(page.locator('span[aria-current="page"]')).toHaveText(title);
-            // The only assertion here a backend erroring on every request fails: each page
-            // returns early into its ErrorState, rendering neither branch. Both branches are
-            // accepted because a stand may legitimately hold no rows.
+            // Either branch: a query error renders neither, and a stand may hold no rows.
             await expect(page.locator('[data-slot="table"]').first().or(page.getByText(emptyTitle))).toBeVisible();
             expect(pageErrors, `uncaught errors on ${path}`).toEqual([]);
         });

frontend/e2e/specs/settings/prompt-detail.spec.ts

@@ -8,9 +8,6 @@ import {
     RICH_PROMPT_TEMPLATE,
 } from '../../mocks/cassettes/settings-prompts.ts';
 
-// The route the editor loads a real Go text/template into. Covered by nothing else: the prompts
-// list spec expands the row in place, which renders a <pre>, never the editor — and the editor's
-// one shipped crash reproduced only in a production build, which is what this tier runs.
 test.describe('settings prompt detail', { tag: '@coverage' }, () => {
     test.use({ cassette: promptDetailCassette() });
 
@@ -29,7 +26,6 @@ test.describe('settings prompt detail', { tag: '@coverage' }, () => {
         const editor = page.getByRole('textbox', { name: EDITOR });
 
         await expect(editor).toBeVisible();
-        // The rich editor parsed the markdown rather than showing source.
         await expect(editor.getByRole('heading', { name: 'Pentester' })).toBeVisible();
         await expect(editor.getByText('nmap -sV {{.Target}}')).toBeVisible();
 
@@ -46,8 +42,6 @@ test.describe('settings prompt detail', { tag: '@coverage' }, () => {
 
         await expect(editor.getByRole('heading', { name: 'Pentester' })).toBeVisible();
 
-        // Typing makes the rich editor emit its own serialization — the half a load-only
-        // assertion cannot see, and where a parse/serialize defect would drop content.
         await editor.click();
         await page.keyboard.press('ControlOrMeta+End');
         await editor.pressSequentially(' E2E-MARK');

frontend/src/components/layouts/app/app-header.tsx

@@ -55,8 +55,6 @@ export function AppHeaderAction({
     );
 }
 
-// Right-pinned: it grows leftward, so a control that comes and goes only leaves the ones
-// before it in place. Put those first.
 export function AppHeaderActions({ children, className }: { children?: ReactNode; className?: string }) {
     return <div className={cn('flex shrink-0 items-center gap-2 px-4', className)}>{children}</div>;
 }

frontend/src/features/knowledges/knowledge-header.tsx

@@ -87,8 +87,7 @@ export function KnowledgeHeader({
     const [isDeleting, setIsDeleting] = useState(false);
     const [isDeleteDialogOpen, setIsDeleteDialogOpen] = useState(false);
 
-    const knowledgeId = knowledge?.id ?? null;
-    // The route's id, not `knowledgeId` above: the pager must work while the document loads.
+    const documentId = knowledge?.id ?? null;
     const knowledgeNav = useKnowledgeDetailNavigation(isNew ? null : (routeKnowledgeId ?? null));
 
     // Title source-of-truth is the server-side `question`. We intentionally do
@@ -99,6 +98,7 @@ export function KnowledgeHeader({
     const hasKnowledge = !!knowledge;
     const isEntityPending = !hasKnowledge;
     const hasAnonymizeRow = isMobile && canAnonymize;
+    const hasViewRow = !!onModeChange;
     const hasEntityRows = !isNew;
     const hasNavRow = isMobile && !isNew;
 
@@ -108,7 +108,7 @@ export function KnowledgeHeader({
         isEditing: isEditingTitle,
         startEdit: handleRenameStart,
         stopEdit: handleRenameCancel,
-    } = useInlineEdit({ resetKey: knowledgeId });
+    } = useInlineEdit({ resetKey: documentId });
 
     const handleRenameSave = useCallback(async () => {
         const newQuestion = editingInputRef.current?.value.trim();
@@ -139,14 +139,14 @@ export function KnowledgeHeader({
     }, [editingInputRef, handleRenameCancel, knowledge, renameKnowledge]);
 
     const handleDelete = useCallback(async () => {
-        if (!knowledgeId) {
+        if (!documentId) {
             return;
         }
 
         setIsDeleting(true);
 
         try {
-            await deleteKnowledge(knowledgeId);
+            await deleteKnowledge(documentId);
             onBeforeNavigateAway?.();
             navigate(routes.knowledges, { replace: true });
         } catch {
@@ -154,7 +154,7 @@ export function KnowledgeHeader({
         } finally {
             setIsDeleting(false);
         }
-    }, [knowledgeId, deleteKnowledge, navigate, onBeforeNavigateAway]);
+    }, [documentId, deleteKnowledge, navigate, onBeforeNavigateAway]);
 
     return (
         <>
@@ -307,9 +307,7 @@ export function KnowledgeHeader({
                             ) : null}
                             {hasEntityRows && (
                                 <>
-                                    {/* Unconditional it doubles up: callers without a mode toggle
-                                        render nothing between the two groups. */}
-                                    {onModeChange ? <DropdownMenuSeparator /> : null}
+                                    {hasViewRow && <DropdownMenuSeparator />}
                                     <DropdownMenuItem
                                         disabled={isDeleting || isEntityPending}
                                         onClick={() => setIsDeleteDialogOpen(true)}