Skip to content

Commit 44388f2

Browse files
bloveclaude
andauthored
feat(cockpit): make Run discoverable from Code (#934)
* docs(cockpit): design for making Run discoverable from Code Records the brainstormed design: mode stops being sticky, the mode rail becomes legible as a switch, Run carries a runtime phase dot, and the Activity dot is re-scoped to unseen problems so the two dots make distinct claims. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(cockpit): implementation plan for Run discoverability Six TDD tasks: drop mode stickiness, make the rail legible as a switch, add a status-dot slot to the ui-react rail primitive, wire the runtime phase onto Run, re-scope the Activity indicator to unseen problems, and verify in a browser. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(cockpit): stop persisting the active mode so every capability opens in Run Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(cockpit): restore docs-surface coverage, key shell on route, tighten mode tests Address code-review findings on 940e21d8: - Restore the only coverage of the docs-surface setExpanded branch, lost when the mode-persistence tests were rewritten (mutation-verified by flipping `surface === 'docs'` to `false`). - Key CockpitShell on canonicalPath in the [...slug] route so client-side navigation between capabilities always remounts fresh state, instead of relying on React Router reconciliation behavior that may or may not preserve activeMode across a params-only navigation. Add a component test proving a capability switch lands on Run even after switching to Code, mutation-verified by reverting to a shared key. - Give the invalid-mode-query test discriminating power by asserting the query param is stripped, not just that the default mode is shown. - Fix a prettier violation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(cockpit): correct the plan's verification commands apps/cockpit has no lint target (only ui-react does), and nx test swallows the vitest reporter output in this worktree, so real counts need vitest run --root directly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(cockpit): add real regression coverage for the per-route CockpitShell key The component-level test added in 58f55680 hard-coded key={...} in its own JSX and never imported the route module, so it only proved React's remount-on-key-change semantics rather than guarding the actual production wiring. Deleting key={canonicalPath} from page.tsx left the full cockpit suite green. Add page.spec.tsx, which imports the [...slug] route module directly and asserts the returned element's key equals the canonical path for a slug, and that two different capabilities get two different keys. next/navigation and the content-bundle loader are mocked so the route can be exercised in isolation. page.tsx also needed an explicit `import React from 'react'` (matching the sibling apps/cockpit/src/app/page.tsx) — without it, evaluating the module under Vitest's jsx transform threw ReferenceError: React is not defined before the route logic ever ran. Mutation-verified: with key={canonicalPath} removed, both new tests fail (element.key is null); restored via `git checkout --` and confirmed `git diff --exit-code` was clean before reapplying the fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(cockpit): make the mode rail read as a switch Group the four mode buttons under a "View" caption, separate them from the Activity/Settings utilities with a rule line, and lift inactive rail items off --ds-text-muted (the disabled-text token) onto --ds-text-secondary so the rail reads as a switch, not chrome. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(cockpit): make rail separator visible and group label accessible Two review fixes to 3d43dd54: - The utilities-group separator used --ds-border, which is a one-value difference from --ds-surface-tinted (the rail background) in dark mode (rgb(45,45,45) on rgb(44,44,44)) -- effectively invisible in the cockpit's default theme. Switch to --ds-border-strong, the token the pane divider already uses for this same collision. - The "View" caption on the mode group was aria-hidden, on the theory that the nav's own aria-label covered it. It doesn't: Activity and Settings render inside the same <nav>, so a screen-reader user heard one flat "Cockpit modes" sequence with no group boundary -- sighted users got the new two-group structure, non-sighted users didn't. Replace aria-hidden with role="group" + aria-labelledby on the primary group div, which announces "View" as a group label without adding a second landmark. Also bumped the caption to --ds-text-secondary/10px to match [data-control-plane-rail-label], and made the spec's cockpit.css path resolution cwd-independent via import.meta.url instead of a process.cwd() suffix check. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(ui-react): add a status dot slot to control plane rail items Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(ui-react): fold rail item status into one object prop status and statusLabel were independent optional props, so either could be set without the other: a status with no accessible label (colour-only, WCAG 1.4.1) or a label with no dot. Combine them into a single ControlPlaneRailItemStatus object so both invalid states are unrepresentable. No consumers of either prop existed yet, so this is a clean API tightening, not a migration. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(cockpit): align Task 4 with the folded rail status prop Task 3 tightened status/statusLabel into one { kind, label } object; update Task 4's mapping, wiring and mutation check to match, and record the flex-gap and tooltip-placement hazards it must handle. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(cockpit): show runtime phase on the Run rail item Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(cockpit): separate the rail tooltip from the status accessible name A labelled rail item with a status no longer renders the icon-rail tooltip, whose positioning is authored for the 56px rail and lands over the context pane. The status stays in the accessible name via aria-label and in a visually hidden span. The status dot's ring now tracks the rail item background so hover does not show a halo, and the dot keeps an explicit forced-colors treatment. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(cockpit): re-scope the Activity indicator to unseen problems Run's rail dot now reports what the runtime is doing right now, so the Activity dot no longer duplicates it. Activity means "there are problems in the log you have not read": errors only, counted from a seen marker that the panel clears on open. An error that self-recovers keeps its trace until someone looks. Also makes runtimeNeedsAttention an exhaustive switch so a new RuntimePhase fails to compile there as it already does in runtimeRailStatus, and decouples the Run rail assertions from the now phase-dependent accessible name. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(cockpit): correct the activity log ordering claim activityReducer prepends, so the log is newest-first: the unseen window runs from index 0, not from seenCount. The plan asserted the opposite and its three specified tests passed under both readings, so the inverted selector would have shipped green. Record the direction and the count-marker saturation limit at the 50-event cap. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(cockpit): runtimeNeedsAttention is dead, not live Both the spec and the plan claimed the helper still had a caller -- the spec said it drives the Run dot's error bucket, the plan said use-runtime-controller.ts uses it. Neither is true: runtimeRailStatus covers the Run dot, and removing the Activity dot's call left zero production callers. Record that it is deleted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(cockpit): drop dead runtimeNeedsAttention and cover the clear path Re-scoping the Activity dot removed the last caller of runtimeNeedsAttention; the Run dot goes through runtimeRailStatus. An exported, tested, uncalled predicate is what gets miswired back later, so delete it and its test table rather than hardening it. Adds the missing clear-path coverage: resetting the seen marker on clear is load-bearing, and dropping it leaves the marker stranded over an empty log, swallowing the next N problems for the rest of the page visit. Also drops a provably dead inner clamp in countUnseenProblems (slice already clamps a too-large end index) while keeping the outer one, which is not defensive -- a negative end index counts back from the tail and reports already-seen errors as unseen. The test for it now proves that. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(cockpit): record Task 6 browser verification results All six tasks verified live in both themes against a fresh build. Records two false alarms caused by testing under a live dev server after git-level file swaps, and the procedure that avoids them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 5142688 commit 44388f2

21 files changed

Lines changed: 1976 additions & 124 deletions
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
/** @vitest-environment jsdom */
2+
import { describe, expect, it, vi } from 'vitest';
3+
4+
vi.mock('next/navigation', () => ({
5+
redirect: vi.fn(() => {
6+
throw new Error('redirect() should not be called for a canonical slug');
7+
}),
8+
}));
9+
10+
vi.mock('../../lib/content-bundle', () => ({
11+
getContentBundle: vi.fn().mockResolvedValue({
12+
codeFiles: {},
13+
promptFiles: {},
14+
runtimeUrl: null,
15+
docSections: [],
16+
narrativeDocs: [],
17+
}),
18+
}));
19+
20+
import CockpitRoutePage from './page';
21+
import { getCockpitPageModel } from '../../lib/cockpit-page';
22+
23+
describe('CockpitRoutePage', () => {
24+
it('keys the rendered CockpitShell on the canonical path', async () => {
25+
const slug = [
26+
'langgraph',
27+
'core-capabilities',
28+
'streaming',
29+
'overview',
30+
'python',
31+
];
32+
const { canonicalPath } = getCockpitPageModel(slug);
33+
34+
const element = await CockpitRoutePage({
35+
params: Promise.resolve({ slug }),
36+
});
37+
38+
expect(element.key).toBe(canonicalPath);
39+
});
40+
41+
it('gives two different capabilities two different keys', async () => {
42+
const streamingSlug = [
43+
'langgraph',
44+
'core-capabilities',
45+
'streaming',
46+
'overview',
47+
'python',
48+
];
49+
const persistenceSlug = [
50+
'langgraph',
51+
'core-capabilities',
52+
'persistence',
53+
'overview',
54+
'python',
55+
];
56+
57+
const streamingElement = await CockpitRoutePage({
58+
params: Promise.resolve({ slug: streamingSlug }),
59+
});
60+
const persistenceElement = await CockpitRoutePage({
61+
params: Promise.resolve({ slug: persistenceSlug }),
62+
});
63+
64+
expect(streamingElement.key).not.toBe(persistenceElement.key);
65+
expect(streamingElement.key).toBe(
66+
getCockpitPageModel(streamingSlug).canonicalPath
67+
);
68+
expect(persistenceElement.key).toBe(
69+
getCockpitPageModel(persistenceSlug).canonicalPath
70+
);
71+
});
72+
});

apps/cockpit/src/app/[...slug]/page.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import React from 'react';
12
import { redirect } from 'next/navigation';
23
import { CockpitShell } from '../../components/cockpit-shell';
34
import { getContentBundle } from '../../lib/content-bundle';
@@ -27,6 +28,7 @@ export default async function CockpitRoutePage({
2728

2829
return (
2930
<CockpitShell
31+
key={canonicalPath}
3032
navigationTree={navigationTree}
3133
presentation={presentation}
3234
entryTitle={entry.title}

apps/cockpit/src/app/cockpit.css

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,7 @@ pre.shiki {
363363
.cockpit-control-plane {
364364
--cockpit-state-error: #b42318;
365365
--cockpit-state-success: #1a7a40;
366+
--cockpit-state-working: #9a6700;
366367
display: grid;
367368
grid-template-columns: 56px minmax(0, 272px);
368369
height: 100%;
@@ -373,6 +374,7 @@ pre.shiki {
373374
[data-theme="dark"] .cockpit-control-plane {
374375
--cockpit-state-error: #ff6369;
375376
--cockpit-state-success: #4cc38a;
377+
--cockpit-state-working: #e0a02f;
376378
}
377379
.cockpit-control-plane [data-control-plane-rail] {
378380
min-width: 0;
@@ -387,9 +389,28 @@ pre.shiki {
387389
flex-direction: column;
388390
gap: 4px;
389391
}
390-
.cockpit-control-plane [data-control-plane-rail-group="utilities"] { margin-top: auto; }
392+
.cockpit-control-plane [data-control-plane-rail-group="utilities"] {
393+
margin-top: auto;
394+
padding-top: 8px;
395+
/* --ds-border is a 1-value difference from --ds-surface-tinted (the rail
396+
background) in dark mode -- rgb(45,45,45) on rgb(44,44,44), effectively
397+
invisible. --ds-border-strong is the token the pane divider already
398+
uses for the same reason (cockpit.css ~L460). */
399+
border-top: 1px solid var(--ds-border-strong);
400+
}
401+
.cockpit-control-plane [data-control-plane-rail-group-label] {
402+
display: block;
403+
padding-bottom: 4px;
404+
color: var(--ds-text-secondary);
405+
font-size: 10px;
406+
font-weight: 600;
407+
letter-spacing: 0.09em;
408+
text-transform: uppercase;
409+
text-align: center;
410+
}
391411
.cockpit-control-plane-utility-anchor { display: contents; }
392412
.cockpit-control-plane [data-control-plane-rail-item] {
413+
--cockpit-rail-status-ring: var(--ds-surface-tinted);
393414
position: relative;
394415
min-height: 48px;
395416
padding: 6px 2px;
@@ -400,13 +421,14 @@ pre.shiki {
400421
align-items: center;
401422
justify-content: center;
402423
gap: 4px;
403-
color: var(--ds-text-muted);
424+
color: var(--ds-text-secondary);
404425
background: transparent;
405426
text-decoration: none;
406427
cursor: pointer;
407428
transition: background 120ms ease, color 120ms ease;
408429
}
409430
.cockpit-control-plane [data-control-plane-rail-item]:hover {
431+
--cockpit-rail-status-ring: var(--ds-surface);
410432
color: var(--ds-text-primary);
411433
background: var(--ds-surface);
412434
}
@@ -419,6 +441,24 @@ pre.shiki {
419441
line-height: 1;
420442
font-weight: 600;
421443
}
444+
.cockpit-control-plane [data-control-plane-rail-status] {
445+
position: absolute;
446+
top: 7px;
447+
right: 11px;
448+
width: 7px;
449+
height: 7px;
450+
border: 2px solid var(--cockpit-rail-status-ring);
451+
border-radius: 999px;
452+
}
453+
.cockpit-control-plane [data-control-plane-rail-status="success"] {
454+
background: var(--cockpit-state-success);
455+
}
456+
.cockpit-control-plane [data-control-plane-rail-status="working"] {
457+
background: var(--cockpit-state-working);
458+
}
459+
.cockpit-control-plane [data-control-plane-rail-status="error"] {
460+
background: var(--cockpit-state-error);
461+
}
422462
.cockpit-control-plane [data-control-plane-rail-icon],
423463
[data-cockpit-activity-icon] {
424464
display: inline-flex;
@@ -950,6 +990,10 @@ pre.shiki {
950990
color: CanvasText;
951991
background: Canvas;
952992
}
993+
.cockpit-control-plane [data-control-plane-rail-status] {
994+
border: 1px solid CanvasText;
995+
background: Canvas;
996+
}
953997
.cockpit-control-plane [data-control-plane-overflow-menu] {
954998
border: 1px solid CanvasText;
955999
color: CanvasText;

0 commit comments

Comments
 (0)