Repository navigation
feat(devtools): inspector refinement — correctness, token system, dirty-first defaults #52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
4d11ff8
0c91a8b
415e566
72c668a
a6fa3ab
6a80e51
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| import { cleanup, fireEvent, render, screen } from "@testing-library/react"; | ||
| import { afterEach, describe, expect, it, vi } from "vitest"; | ||
|
|
||
| import { CommandFeedback } from "./command-feedback"; | ||
|
|
||
| /** | ||
| * One element used to flip between role="alert" and role="status" (with aria-live="polite" on | ||
| * both, which contradicts alert's implicit assertive live region). Assistive tech does not | ||
| * reliably announce a role change, so notices and errors now live in two stable regions. | ||
| */ | ||
| describe("CommandFeedback", () => { | ||
| afterEach(cleanup); | ||
|
|
||
| it("announces pending work and notices in a polite status region", () => { | ||
| render(<CommandFeedback pending="Rewinding" error={null} notice={null} onDismiss={() => {}} />); | ||
| const status = screen.getByRole("status"); | ||
| expect(status.getAttribute("aria-live")).toBe("polite"); | ||
| expect(status.textContent).toContain("Rewinding"); | ||
| expect(screen.getByRole("alert").textContent).toBe(""); | ||
| }); | ||
|
|
||
| it("announces errors in an alert that is always present in the tree", () => { | ||
| const { rerender } = render(<CommandFeedback pending={null} error={null} notice={null} onDismiss={() => {}} />); | ||
| expect(screen.getByRole("alert").textContent).toBe(""); | ||
| rerender(<CommandFeedback pending={null} error="Restore refused: entity changed" notice={null} onDismiss={() => {}} />); | ||
| expect(screen.getByRole("alert").textContent).toContain("Restore refused"); | ||
| expect(screen.getByRole("status").textContent).toBe(""); | ||
| }); | ||
|
|
||
| it("offers a dismiss control only when there is something to dismiss", () => { | ||
| const onDismiss = vi.fn(); | ||
| const { rerender } = render(<CommandFeedback pending={null} error={null} notice={null} onDismiss={onDismiss} />); | ||
| expect(screen.queryByRole("button", { name: /dismiss/i })).toBeNull(); | ||
| rerender(<CommandFeedback pending={null} error={null} notice="Snapshot restored" onDismiss={onDismiss} />); | ||
| fireEvent.click(screen.getByRole("button", { name: /dismiss/i })); | ||
| expect(onDismiss).toHaveBeenCalledTimes(1); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| export interface CommandFeedbackProps { | ||
| readonly pending: string | null; | ||
| readonly error: string | null; | ||
| readonly notice: string | null; | ||
| readonly onDismiss: () => void; | ||
| } | ||
|
|
||
| /** | ||
| * Two stable live regions: a polite status region for progress and notices, and an alert for | ||
| * errors. Both stay mounted so assistive technology announces content changes; a single element | ||
| * that swaps its role between "status" and "alert" is not announced reliably. | ||
| */ | ||
| export function CommandFeedback({ pending, error, notice, onDismiss }: CommandFeedbackProps) { | ||
| const state = error ? "error" : notice ? "success" : pending ? "pending" : "idle"; | ||
| return ( | ||
| <div className="pem-command-feedback" data-state={state}> | ||
| <div role="status" aria-live="polite" className="pem-command-status"> | ||
| {pending && !error && <span>Working: {pending}…</span>} | ||
| {notice && !error && <span>{notice}</span>} | ||
| </div> | ||
| <div role="alert" className="pem-command-alert"> | ||
| {error && <span>{error}</span>} | ||
| </div> | ||
| {(error || notice) && ( | ||
| <button type="button" aria-label="Dismiss command message" onClick={onDismiss}>×</button> | ||
| )} | ||
| </div> | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| import { cleanup, render, screen } from "@testing-library/react"; | ||
| import { afterEach, describe, expect, it } from "vitest"; | ||
|
|
||
| import { GraphPulse } from "./graph-pulse"; | ||
|
|
||
| describe("GraphPulse", () => { | ||
| afterEach(cleanup); | ||
|
|
||
| it("does not reference the segment list while collapsed, and does once expanded", () => { | ||
| const { rerender } = render( | ||
| <GraphPulse events={[]} selected={null} collapsed onToggleCollapsed={() => {}} onSelect={() => {}} />, | ||
| ); | ||
| const toggle = screen.getByRole("button", { name: /graph pulse/i }); | ||
| expect(toggle.getAttribute("aria-expanded")).toBe("false"); | ||
| expect(toggle.hasAttribute("aria-controls")).toBe(false); | ||
|
|
||
| rerender(<GraphPulse events={[]} selected={null} collapsed={false} onToggleCollapsed={() => {}} onSelect={() => {}} />); | ||
| const controls = toggle.getAttribute("aria-controls"); | ||
| expect(controls).toBeTruthy(); | ||
| expect(document.getElementById(controls!)).not.toBeNull(); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| import { render } from "@testing-library/react"; | ||
| import { describe, expect, it } from "vitest"; | ||
|
|
||
| import { ENTITY_GRAPH_DEVTOOLS_STYLES } from "../styles"; | ||
| import { InspectorDiff } from "./value-inspector"; | ||
|
|
||
| /** | ||
| * The devtools stylesheet is a string injected into a Shadow DOM, so jsdom never applies it. | ||
| * This test instead proves the contract between markup and stylesheet: every rule that styles a | ||
| * diff-row cell must actually match the cells InspectorDiff renders. Before the fix the rules | ||
| * targeted `.pem-diff-row > span` while body cells were `<code>`, so body rows rendered with no | ||
| * padding, no mono font, no wrapping and no change colour. | ||
| */ | ||
| function cellSelectors(): string[] { | ||
| const selectors = new Set<string>(); | ||
| for (const match of ENTITY_GRAPH_DEVTOOLS_STYLES.matchAll(/^([^{}]*\.pem-diff-row[^{}]*)\{/gm)) { | ||
| for (const selector of match[1].split(",")) { | ||
| const trimmed = selector.trim(); | ||
| if (trimmed.includes(">")) selectors.add(trimmed); | ||
| } | ||
| } | ||
| return [...selectors]; | ||
| } | ||
|
|
||
| describe("InspectorDiff markup matches the stylesheet", () => { | ||
| const rows = [ | ||
| { path: "status", kind: "changed", original: "pending", live: "approved" }, | ||
| { path: "owner", kind: "added", original: undefined, live: "ada" }, | ||
| ] as const; | ||
|
|
||
| it("styles every body cell, not only the header cells", () => { | ||
| const { container } = render(<InspectorDiff rows={rows} />); | ||
| const bodyRows = [...container.querySelectorAll('.pem-diff-row:not(.pem-diff-head)')]; | ||
| expect(bodyRows).toHaveLength(2); | ||
| const cellRules = cellSelectors().filter((selector) => !selector.includes("[data-kind")); | ||
| expect(cellRules.length).toBeGreaterThan(0); | ||
| for (const row of bodyRows) { | ||
| for (const cell of row.children) { | ||
| expect(cellRules.some((selector) => cell.matches(selector)), `${cell.tagName} matches ${cellRules.join(" | ")}`).toBe(true); | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| it("applies the change colour rule to the field cell of changed rows", () => { | ||
| const { container } = render(<InspectorDiff rows={rows} />); | ||
| const changedField = container.querySelector('.pem-diff-row[data-kind="changed"] > :first-child'); | ||
| const changeRules = cellSelectors().filter((selector) => selector.includes('[data-kind="changed"]')); | ||
| expect(changeRules.length).toBeGreaterThan(0); | ||
| expect(changeRules.some((selector) => changedField?.matches(selector))).toBe(true); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| import { cleanup, render, screen } from "@testing-library/react"; | ||
| import { afterEach, describe, expect, it } from "vitest"; | ||
|
|
||
| import { InspectorVirtualList } from "./virtual-list"; | ||
| import { ENTITY_GRAPH_DEVTOOLS_STYLES } from "../styles"; | ||
|
|
||
| describe("InspectorVirtualList", () => { | ||
| afterEach(cleanup); | ||
|
|
||
| it("passes the item index to renderItem so rows never search the array for their position", () => { | ||
| const items = ["a", "b", "c"]; | ||
| render( | ||
| <InspectorVirtualList | ||
| items={items} | ||
| getKey={(item) => item} | ||
| ariaLabel="letters" | ||
| renderItem={(item, index) => <span>{`${index + 1}:${item}`}</span>} | ||
| />, | ||
| ); | ||
| expect(screen.getByRole("list", { name: "letters" }).textContent).toBe("1:a2:b3:c"); | ||
| }); | ||
|
|
||
| it("bounds the membership scroll list so the virtualizer has a viewport", () => { | ||
| // jsdom applies no Shadow DOM stylesheet, so assert the rule exists in the sheet itself. | ||
| const rule = ENTITY_GRAPH_DEVTOOLS_STYLES.match(/\.pem-membership\s+\.pem-scroll-list\s*\{([^}]*)\}/); | ||
| expect(rule, "a .pem-membership .pem-scroll-list rule").not.toBeNull(); | ||
| expect(rule?.[1]).toMatch(/max-height\s*:/); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| import { describe, expect, it } from "vitest"; | ||
| import type { GraphDevtoolsChange, GraphDevtoolsEvent } from "@prometheus-ags/entity-graph-core/devtools"; | ||
|
|
||
| import { eventCorrelationLabel, eventTitle } from "./event-format"; | ||
|
|
||
| type MutationEvent = Extract<GraphDevtoolsEvent, { type: "mutation" }>; | ||
|
|
||
| const counts = { | ||
| entityTypes: 1, entities: 1, patchedEntities: 0, entityStates: 0, syncMetadata: 0, | ||
| lists: 0, listMemberships: 0, fetching: 0, stale: 0, errors: 0, | ||
| }; | ||
|
|
||
| function mutation(changes: GraphDevtoolsChange[], affected?: MutationEvent["payload"]["affectedEntities"]): MutationEvent { | ||
| return { | ||
| protocol: "prometheus.entity-graph.devtools", | ||
| version: 1, | ||
| storeId: "graph-1", | ||
| eventId: "graph-1:7", | ||
| sequence: 7, | ||
| correlationId: "graph-1:7", | ||
| observedAt: "2026-10-08T12:00:00.000Z", | ||
| type: "mutation", | ||
| payload: { | ||
| snapshot: { status: "retained", cursor: 3, eventSequence: 7, bytes: 10, capturedAt: "2026-10-08T12:00:00.000Z" }, | ||
| changes, | ||
| ...(affected ? { affectedEntities: affected } : {}), | ||
| before: counts, | ||
| after: counts, | ||
| projectionDurationMs: 0.5, | ||
| valuesTruncated: false, | ||
| changesOmitted: 0, | ||
| }, | ||
| } as MutationEvent; | ||
| } | ||
|
|
||
| describe("eventTitle for mutations", () => { | ||
| it("names the first affected identity and its patch operation, listing fields when values are included", () => { | ||
| const event = mutation([ | ||
| { category: "patch", action: "added", key: "Order", id: "o-1042", valueState: "included", after: { status: "paid" } }, | ||
| ]); | ||
| expect(eventTitle(event)).toBe("Order/o-1042 · patch status"); | ||
| }); | ||
|
|
||
| it("falls back to a bare patch operation under the metadata-only policy", () => { | ||
| const event = mutation([ | ||
| { category: "patch", action: "updated", key: "Order", id: "o-1042", valueState: "hidden-by-policy" }, | ||
| ]); | ||
| expect(eventTitle(event)).toBe("Order/o-1042 · patch"); | ||
| }); | ||
|
|
||
| it("describes entity upserts and removals", () => { | ||
| expect(eventTitle(mutation([ | ||
| { category: "entity", action: "updated", key: "Order", id: "o-1042", valueState: "hidden-by-policy" }, | ||
| ]))).toBe("Order/o-1042 · upsert"); | ||
| expect(eventTitle(mutation([ | ||
| { category: "entity", action: "removed", key: "Order", id: "o-1042", valueState: "hidden-by-policy" }, | ||
| ]))).toBe("Order/o-1042 · remove"); | ||
| }); | ||
|
|
||
| it("counts the other affected identities", () => { | ||
| const event = mutation([ | ||
| { category: "entity", action: "added", key: "Order", id: "o-1", valueState: "hidden-by-policy" }, | ||
| { category: "entity", action: "added", key: "Order", id: "o-2", valueState: "hidden-by-policy" }, | ||
| { category: "entity", action: "added", key: "Order", id: "o-3", valueState: "hidden-by-policy" }, | ||
| ]); | ||
| expect(eventTitle(event)).toBe("Order/o-1 · upsert · +2 more"); | ||
| }); | ||
|
|
||
| it("keeps the change-count form only when no identity is known", () => { | ||
| expect(eventTitle(mutation([ | ||
| { category: "list", action: "updated", key: "orders:all", valueState: "hidden-by-policy", beforeCount: 1, afterCount: 2 }, | ||
| ]))).toBe("1 graph change"); | ||
| expect(eventTitle(mutation([ | ||
| { category: "list", action: "updated", key: "a", valueState: "hidden-by-policy" }, | ||
| { category: "sync", action: "updated", key: "b", valueState: "hidden-by-policy" }, | ||
| ]))).toBe("2 graph changes"); | ||
| }); | ||
| }); | ||
|
|
||
| describe("eventCorrelationLabel", () => { | ||
| it("shows the last colon-separated segment rather than the store prefix", () => { | ||
| expect(eventCorrelationLabel(mutation([]))).toBe("7"); | ||
| expect(eventCorrelationLabel({ ...mutation([]), correlationId: "store:req:abc123" })).toBe("abc123"); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,6 +24,7 @@ import { | |
| DEFAULT_ENTITY_GRAPH_DEVTOOLS_SHORTCUT, | ||
| ENTITY_GRAPH_DEVTOOLS_PREFERENCE_KEY, | ||
| entityGraphDevtoolsAriaShortcut, | ||
| isEditableShortcutTarget, | ||
| matchesEntityGraphDevtoolsShortcut, | ||
| readEntityGraphDevtoolsPreferences, | ||
| writeEntityGraphDevtoolsPreferences, | ||
|
|
@@ -190,13 +191,14 @@ function EntityGraphDevtoolsSurface({ | |
| useEffect(() => { | ||
| if (!resolvedShortcut) return; | ||
| const onKeyDown = (event: KeyboardEvent) => { | ||
| if (isEditableShortcutTarget(event.target)) return; | ||
| if (!matchesEntityGraphDevtoolsShortcut(event, resolvedShortcut)) return; | ||
|
Comment on lines
193
to
195
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When the shortcut is pressed in an inspector search input or preview textarea, this window-level listener sees the event retargeted to the DevTools shadow host because the inspector is portaled into a Useful? React with 👍 / 👎. |
||
| event.preventDefault(); | ||
| if (panelOpen && visible) closePanel(); | ||
| else openPanel(); | ||
| }; | ||
| document.addEventListener("keydown", onKeyDown); | ||
| return () => document.removeEventListener("keydown", onKeyDown); | ||
| window.addEventListener("keydown", onKeyDown); | ||
| return () => window.removeEventListener("keydown", onKeyDown); | ||
| }, [closePanel, openPanel, panelOpen, resolvedShortcut, visible]); | ||
|
|
||
| useEffect(() => { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This commit adds nine Vitest unit/component suites and cites them as development-gate evidence, but the repository explicitly forbids creating or expanding unit, component, isolated, snapshot, or mock-backed tests and requires the assembled integration/acceptance suite instead. Remove these suites and retain coverage in the packed browser/integration flow.
AGENTS.md reference: AGENTS.md:L92-L102
Useful? React with 👍 / 👎.