From 637c6dd1fc23582b090ffacf30a83e454669760a Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 15:24:08 +0000 Subject: [PATCH 1/8] Read dataset columns through ColumnPath, stepping through instance references MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Generalizes the grid's one-level CellPath into ColumnPath (specs/datasets.md §Q.1): a field or instance root and any number of keys, each step taken by what it stands on, including an instance reference's arguments. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01QMiMTviyLp3ciCvQf3mzkf --- src/client/components/DatasetView.tsx | 6 +- src/client/components/dataset-grid.test.ts | 33 --- src/client/components/dataset-grid.ts | 61 ++--- src/shared/column-paths.test.ts | 251 +++++++++++++++++++++ src/shared/column-paths.ts | 227 +++++++++++++++++++ 5 files changed, 500 insertions(+), 78 deletions(-) create mode 100644 src/shared/column-paths.test.ts create mode 100644 src/shared/column-paths.ts diff --git a/src/client/components/DatasetView.tsx b/src/client/components/DatasetView.tsx index c5a2b70..26b1f9e 100644 --- a/src/client/components/DatasetView.tsx +++ b/src/client/components/DatasetView.tsx @@ -38,6 +38,7 @@ import { } from "react"; import { createPortal } from "react-dom"; import { shortSyntax } from "ts-proppy/react"; +import { pathField } from "../../shared/column-paths"; import type { Dataset, DatasetField, @@ -770,9 +771,10 @@ function DatasetView({ const onGroupHeaderClicked = useCallback( (col: number, event: { preventDefault: () => void }) => { const column = columns[col]; - if (!column || !("path" in column)) return; + const fieldId = + column && "path" in column ? pathField(column.path) : undefined; + if (fieldId === undefined) return; event.preventDefault(); - const { fieldId } = column.path; const next = new Set(expanded); if (!next.delete(fieldId)) next.add(fieldId); updateLayout({ ...layout, expanded: next }); diff --git a/src/client/components/dataset-grid.test.ts b/src/client/components/dataset-grid.test.ts index 9e12be3..d23371f 100644 --- a/src/client/components/dataset-grid.test.ts +++ b/src/client/components/dataset-grid.test.ts @@ -26,7 +26,6 @@ import { menuFieldId, parseDatasetLayout, RowPager, - readPath, serializeDatasetLayout, } from "./dataset-grid"; @@ -158,38 +157,6 @@ describe("menuFieldId", () => { }); }); -describe("readPath", () => { - it("reads a whole cell, or absent for a sparse row", () => { - expect(readPath(row({ "1": seeded }), { fieldId: "1" })).toBe(seeded); - expect(readPath(row({}), { fieldId: "1" })).toBe("absent"); - expect(readPath(row({}), { fieldId: "1", key: "title" })).toBe("absent"); - }); - - it("reads an object's property by key", () => { - expect( - readPath(row({ "1": seeded }), { fieldId: "1", key: "title" }), - ).toEqual(text("Milk")); - }); - - it("reads a typed-in object's property, wrapped as the value cell it would be", () => { - expect( - readPath(row({ "2": info }), { fieldId: "2", key: "title" }), - ).toEqual({ kind: "value", value: { kind: "primitive", value: "Say hi" } }); - expect(readPath(row({ "2": info }), { fieldId: "2", key: "nope" })).toBe( - "n/a", - ); - }); - - it("is n/a when the row's cell has no such key", () => { - expect(readPath(row({ "1": blank }), { fieldId: "1", key: "title" })).toBe( - "n/a", - ); - expect( - readPath(row({ "1": text("plain") }), { fieldId: "1", key: "title" }), - ).toBe("n/a"); - }); -}); - describe("cellView", () => { it("is loading until the row's page arrives", () => { expect(cellView(undefined, columns()[0])).toEqual({ kind: "loading" }); diff --git a/src/client/components/dataset-grid.ts b/src/client/components/dataset-grid.ts index 7f67838..e7559f0 100644 --- a/src/client/components/dataset-grid.ts +++ b/src/client/components/dataset-grid.ts @@ -18,6 +18,12 @@ * answers. */ +import { + type ColumnPath, + fieldPath, + pathField, + readColumnPath, +} from "../../shared/column-paths"; import { committedCell, type EditableBase, @@ -34,13 +40,6 @@ import type { import { previewCell, previewPropValue } from "./dataset-preview"; import { DISCLOSURE_COLLAPSED, DISCLOSURE_EXPANDED } from "./grid-sprites"; -/** Where a column reads from: a field, or one key one level inside it. */ -export interface CellPath { - fieldId: string; - /** An object property's name. */ - key?: string; -} - /** One column of the dataset table. */ export type DatasetColumn = { /** Stable across layouts — resized widths are remembered by it. */ @@ -63,9 +62,10 @@ export type DatasetColumn = { | { /** A field in one column. */ role: "whole"; - path: CellPath; + path: ColumnPath; } - | { role: "key"; path: Required } + /** One key inside a field. */ + | { role: "key"; path: ColumnPath } /** Where the row came from. */ | { role: "source" } ); @@ -105,7 +105,7 @@ export function buildColumns({ const fieldShape = shape[field.id]; const fieldKeys = fieldShape?.keys ?? []; const type = shortType(field.def.type.syntax); - const path = { fieldId: field.id }; + const path = fieldPath(field.id); const base = primitiveBase(field.def.type); const whole: DatasetColumn = { id: field.id, @@ -131,7 +131,7 @@ export function buildColumns({ title: key, group, role: "key", - path: { fieldId: field.id, key }, + path: fieldPath(field.id, [key]), width: WIDTHS.key, }); } @@ -153,7 +153,7 @@ export function buildColumns({ export function menuFieldId( column: DatasetColumn | undefined, ): string | undefined { - return column?.role === "whole" ? column.path.fieldId : undefined; + return column?.role === "whole" ? pathField(column.path) : undefined; } /** @@ -164,10 +164,9 @@ export function menuFieldId( export function fieldIdsByGroup(columns: DatasetColumn[]): Map { const byGroup = new Map(); for (const column of columns) { - if (column.group !== undefined && "path" in column) { - if (!byGroup.has(column.group)) { - byGroup.set(column.group, column.path.fieldId); - } + const fieldId = "path" in column ? pathField(column.path) : undefined; + if (column.group !== undefined && fieldId !== undefined) { + if (!byGroup.has(column.group)) byGroup.set(column.group, fieldId); } } return byGroup; @@ -192,31 +191,6 @@ export function groupHeader( }; } -/** - * What a path reads in a row: the input there, `"absent"` when the row has - * nothing there, or `"n/a"` when the field's cell has no such key. - * - * A typed-in object's property is a `PropValue`, not an `ExecutionInput`; it - * comes back wrapped as the value cell it would have been on its own. - */ -export function readPath( - row: DatasetRow, - path: CellPath, -): ExecutionInput | "absent" | "n/a" { - const cell = row.cells[path.fieldId]; - if (!cell) return "absent"; - const { key } = path; - if (key === undefined) return cell; - if (cell.kind === "value") { - const { value } = cell; - return value.kind === "object" && Object.hasOwn(value.properties, key) - ? { kind: "value", value: value.properties[key] } - : "n/a"; - } - const inner = cell.kind === "object" ? cell.properties : undefined; - return inner && Object.hasOwn(inner, key) ? inner[key] : "n/a"; -} - /** A cell the grid types into in place — see {@link editableCell}. */ export interface EditableCell { /** The type it's typed into as. */ @@ -241,7 +215,8 @@ export function editableCell( ): EditableCell | undefined { if (!row || column?.role !== "whole" || !column.base) return undefined; const { base } = column; - const { fieldId } = column.path; + const fieldId = pathField(column.path); + if (fieldId === undefined) return undefined; const cell = row.cells[fieldId]; if (!cell) return { base, fieldId }; if (cell.kind !== "value" || cell.value.kind !== "primitive") @@ -471,7 +446,7 @@ export function cellView( ? { kind: "text", text: "playground", tone: "dim" } : { kind: "empty" }; default: { - const found = readPath(row, column.path); + const found = readColumnPath(row, column.path); const e = editableCell(row, column); if (e) { return { diff --git a/src/shared/column-paths.test.ts b/src/shared/column-paths.test.ts new file mode 100644 index 0000000..5965340 --- /dev/null +++ b/src/shared/column-paths.test.ts @@ -0,0 +1,251 @@ +// SPDX-License-Identifier: AGPL-3.0-only +// Copyright (c) 2026 Alexander Corrado + +import { describe, expect, it } from "vitest"; +import { + childInputs, + columnPathId, + fieldPath, + InvalidColumnPathError, + instancePath, + MAX_REFERENCE_HOPS, + type PathRow, + parseColumnPath, + readColumnPath, + resolveColumnPath, +} from "./column-paths.ts"; +import type { ExecutionInput, PropValue } from "./types.ts"; + +const prim = (value: string | number | boolean): PropValue => ({ + kind: "primitive", + value, +}); +const text = (value: string): ExecutionInput => ({ + kind: "value", + value: prim(value), +}); + +/** A typed-in object value, as a trace's recorded inputs store one. */ +const info: ExecutionInput = { + kind: "value", + value: { kind: "object", properties: { title: prim("Say hi") } }, +}; +/** An object grafting a resource instance in beside a typed-in value. */ +const seeded: ExecutionInput = { + kind: "object", + properties: { title: text("Milk"), owner: { kind: "instance", name: "db" } }, +}; + +/** + * asgard's planTask row: the prompt's `toolsContext` points at + * `seededTask.taskId`, and `seededTask`'s arguments hold the title. + */ +const planTask: PathRow = { + cells: { + "0": { + kind: "object", + properties: { + create_task: { + kind: "object", + properties: { + db: { kind: "instance", name: "db" }, + rootTaskId: { + kind: "instance", + name: "seededTask", + output: "taskId", + }, + }, + }, + }, + }, + }, + resources: { + db: { uri: "file:///db.ts#db" }, + seededTask: { + uri: "file:///tasks.ts#seededTask", + args: { + title: text("Web site for Evalution"), + parentId: { kind: "instance", name: "root", output: "taskId" }, + }, + }, + root: { uri: "file:///tasks.ts#seededTask", args: { title: text("Root") } }, + }, +}; + +describe("readColumnPath", () => { + it("reads a whole cell, or absent for a sparse row", () => { + expect(readColumnPath({ cells: { "1": seeded } }, fieldPath("1"))).toBe( + seeded, + ); + expect(readColumnPath({ cells: {} }, fieldPath("1"))).toBe("absent"); + expect(readColumnPath({ cells: {} }, fieldPath("1", ["title"]))).toBe( + "absent", + ); + }); + + it("steps into an object cell's property", () => { + expect( + readColumnPath({ cells: { "1": seeded } }, fieldPath("1", ["title"])), + ).toEqual(text("Milk")); + }); + + it("steps into a typed-in object's property, wrapped as a value cell", () => { + expect( + readColumnPath({ cells: { "2": info } }, fieldPath("2", ["title"])), + ).toEqual(text("Say hi")); + expect( + readColumnPath({ cells: { "2": info } }, fieldPath("2", ["nope"])), + ).toBe("n/a"); + }); + + it("steps through an instance reference into its arguments, ignoring its output", () => { + expect( + readColumnPath( + planTask, + fieldPath("0", ["create_task", "rootTaskId", "title"]), + ), + ).toEqual(text("Web site for Evalution")); + }); + + it("follows a path through two references", () => { + expect( + readColumnPath( + planTask, + fieldPath("0", ["create_task", "rootTaskId", "parentId", "title"]), + ), + ).toEqual(text("Root")); + }); + + it("reads an instance root, and its arguments", () => { + expect(readColumnPath(planTask, instancePath("seededTask"))).toEqual({ + kind: "instance", + name: "seededTask", + }); + expect( + readColumnPath(planTask, instancePath("seededTask", ["title"])), + ).toEqual(text("Web site for Evalution")); + expect(readColumnPath(planTask, instancePath("nope"))).toBe("absent"); + }); + + it("ends at anything else, so a further step is n/a", () => { + const row: PathRow = { + cells: { + str: text("plain"), + arr: { + kind: "value", + value: { kind: "array", elements: [prim(1)] } as PropValue, + }, + col: { kind: "dataset", field: "x" }, + slot: { kind: "input", half: "function", path: "taskId" }, + dangling: { kind: "instance", name: "missing" }, + }, + }; + for (const field of ["str", "arr", "col", "slot", "dangling"]) { + expect(readColumnPath(row, fieldPath(field, ["any"]))).toBe("n/a"); + } + expect( + readColumnPath( + planTask, + fieldPath("0", ["create_task", "rootTaskId", "title", "length"]), + ), + ).toBe("n/a"); + }); + + it("gives up on a reference cycle after the hop limit", () => { + const row: PathRow = { + cells: { "0": { kind: "instance", name: "a" } }, + resources: { + a: { uri: "r", args: { next: { kind: "instance", name: "b" } } }, + b: { uri: "r", args: { next: { kind: "instance", name: "a" } } }, + }, + }; + const within = Array(MAX_REFERENCE_HOPS).fill("next"); + expect(readColumnPath(row, fieldPath("0", within))).toEqual({ + kind: "instance", + name: "a", + }); + expect(readColumnPath(row, fieldPath("0", [...within, "next"]))).toBe( + "n/a", + ); + }); +}); + +describe("resolveColumnPath", () => { + it("says which instance argument a path ends at", () => { + expect( + resolveColumnPath( + planTask, + fieldPath("0", ["create_task", "rootTaskId", "title"]), + ), + ).toEqual({ + value: text("Web site for Evalution"), + argument: { instance: "seededTask", arg: "title" }, + }); + expect( + resolveColumnPath(planTask, instancePath("seededTask", ["title"])) + .argument, + ).toEqual({ instance: "seededTask", arg: "title" }); + // Ending at a property, or a missing argument, names none. + expect( + resolveColumnPath({ cells: { "1": seeded } }, fieldPath("1", ["title"])) + .argument, + ).toBeUndefined(); + expect( + resolveColumnPath(planTask, instancePath("seededTask", ["nope"])), + ).toEqual({ value: "n/a" }); + }); +}); + +describe("childInputs", () => { + it("lists what a step can reach, in order", () => { + expect(childInputs(planTask, planTask.cells["0"]).map(([k]) => k)).toEqual([ + "create_task", + ]); + expect( + childInputs(planTask, { + kind: "instance", + name: "seededTask", + output: "taskId", + }).map(([k]) => k), + ).toEqual(["title", "parentId"]); + expect(childInputs(planTask, text("x"))).toEqual([]); + }); +}); + +describe("columnPathId", () => { + it("keeps the ids fields and keys had before paths", () => { + expect(columnPathId(fieldPath("1"))).toBe("1"); + expect(columnPathId(fieldPath("1", ["title"]))).toBe("1/title"); + }); + + it("marks an instance root and percent-encodes / and % in keys", () => { + expect(columnPathId(instancePath("seededTask", ["title"]))).toBe( + "@seededTask/title", + ); + expect(columnPathId(fieldPath("1", ["a/b", "50%"]))).toBe("1/a%2Fb/50%25"); + }); +}); + +describe("parseColumnPath", () => { + it("accepts a field or instance root with string keys", () => { + expect(parseColumnPath({ root: { field: "1" }, keys: ["a"] })).toEqual( + fieldPath("1", ["a"]), + ); + expect(parseColumnPath({ root: { instance: "t" }, keys: [] })).toEqual( + instancePath("t"), + ); + }); + + it("rejects a malformed root or keys", () => { + for (const bad of [ + null, + { root: {}, keys: [] }, + { root: { field: "" }, keys: [] }, + { root: { field: "1" } }, + { root: { field: "1" }, keys: [""] }, + { root: { field: "1" }, keys: [3] }, + ]) { + expect(() => parseColumnPath(bad)).toThrow(InvalidColumnPathError); + } + }); +}); diff --git a/src/shared/column-paths.ts b/src/shared/column-paths.ts new file mode 100644 index 0000000..b4de00a --- /dev/null +++ b/src/shared/column-paths.ts @@ -0,0 +1,227 @@ +// SPDX-License-Identifier: AGPL-3.0-only +// Copyright (c) 2026 Alexander Corrado + +/** + * Column paths: where a dataset column reads from in a row. Shared by the + * grid, the server (which describes and validates paths) and dataset views, + * which must all agree on what a path reads. Pure — no runtime + * dependencies. See `specs/datasets.md` §Q.1. + */ + +import type { ExecutionInput, ResourceInstanceInput } from "./types.ts"; + +/** Where a column reads from in a row. */ +export interface ColumnPath { + /** A field's cell, or one of the row's resource instances by name. */ + root: { field: string } | { instance: string }; + /** Steps inward, by name. Empty for the root itself. */ + keys: string[]; +} + +/** The parts of a row a path reads: its cells, and its resource instances. */ +export interface PathRow { + cells: Record; + resources?: Record; +} + +/** + * What a path reads in a row: the input there, `"absent"` when the row has + * nothing at its root, or `"n/a"` when a step can't be taken. + */ +export type PathValue = ExecutionInput | "absent" | "n/a"; + +/** + * How many instance references a path may step through before it gives up. + * A row whose references form a cycle can't run, but nothing stops one from + * being stored. + */ +export const MAX_REFERENCE_HOPS = 16; + +/** + * Where a path landed, when its last step was an instance's argument — what + * an edit through it must change. See `specs/datasets.md` §Q.4. + */ +export interface InstanceArgument { + /** The instance whose argument it is. */ + instance: string; + /** The argument's name. */ + arg: string; +} + +/** What {@link resolveColumnPath} found. */ +export interface ResolvedPath { + value: PathValue; + /** Set when the last step read an instance's argument. */ + argument?: InstanceArgument; +} + +/** A field's whole column. */ +export function fieldPath(field: string, keys: string[] = []): ColumnPath { + return { root: { field }, keys }; +} + +/** An instance's column. */ +export function instancePath( + instance: string, + keys: string[] = [], +): ColumnPath { + return { root: { instance }, keys }; +} + +/** The field a path is rooted at, or `undefined` for an instance root. */ +export function pathField(path: ColumnPath): string | undefined { + return "field" in path.root ? path.root.field : undefined; +} + +/** + * The inputs one step inward from `input`, by name, in order — what a path + * can step to next: + * + * - a `value` holding an object `PropValue`: its properties, each wrapped as + * the value cell it would be on its own; + * - an `object` cell: its properties; + * - an `instance` reference: that instance's arguments, in the same row. Its + * `output` is ignored, since a step names an argument, not an output. + * + * Anything else — a template, a primitive, an array, a `dataset` or `input` + * reference, an instance the row doesn't declare — has nothing inside it. + */ +export function childInputs( + row: PathRow, + input: ExecutionInput, +): [string, ExecutionInput][] { + switch (input.kind) { + case "value": + return input.value.kind === "object" + ? Object.entries(input.value.properties).map(([key, value]) => [ + key, + { kind: "value", value }, + ]) + : []; + case "object": + return Object.entries(input.properties); + case "instance": + return Object.entries(row.resources?.[input.name]?.args ?? {}); + default: + return []; + } +} + +/** + * The input at a path's root: the field's cell, or a reference to the + * instance — which the next step then reads the arguments of. `"absent"` + * when the row has neither. + */ +function rootInput(row: PathRow, path: ColumnPath): ExecutionInput | "absent" { + if ("field" in path.root) return row.cells[path.root.field] ?? "absent"; + const { instance } = path.root; + return row.resources && Object.hasOwn(row.resources, instance) + ? { kind: "instance", name: instance } + : "absent"; +} + +/** + * What `path` reads in `row`, and — when its last step read an instance's + * argument — which one, so an edit can be sent back there. Each step is + * taken by {@link childInputs}'s rules; a step that can't be taken reads as + * `"n/a"`, as does a path that steps through more than + * {@link MAX_REFERENCE_HOPS} instance references. + */ +export function resolveColumnPath( + row: PathRow, + path: ColumnPath, +): ResolvedPath { + const root = rootInput(row, path); + if (root === "absent") return { value: root }; + let value: ExecutionInput = root; + let argument: InstanceArgument | undefined; + let hops = 0; + for (const key of path.keys) { + argument = undefined; + if (value.kind === "instance") { + if (++hops > MAX_REFERENCE_HOPS) return { value: "n/a" }; + argument = { instance: value.name, arg: key }; + } + const next = childInputs(row, value).find(([k]) => k === key); + if (!next) return { value: "n/a" }; + value = next[1]; + } + return argument ? { value, argument } : { value }; +} + +/** + * What `path` reads in `row`: the input there, `"absent"` when the row has + * nothing at its root, or `"n/a"` when a step can't be taken. See + * {@link resolveColumnPath}. + */ +export function readColumnPath(row: PathRow, path: ColumnPath): PathValue { + return resolveColumnPath(row, path).value; +} + +const encodeKey = (key: string) => + key.replaceAll("%", "%25").replaceAll("/", "%2F"); + +/** + * A path as one string, stable enough to key a column's remembered width by. + * A field root is its id, an instance root `@name`, and each key `/` plus + * the key with `/` and `%` percent-encoded — so a field's column is its id, + * and one of its keys `fieldId/key`, as they were before paths. + */ +export function columnPathId(path: ColumnPath): string { + const root = + "field" in path.root ? path.root.field : `@${path.root.instance}`; + return root + path.keys.map(key => `/${encodeKey(key)}`).join(""); +} + +/** Whether two paths name the same place. */ +export function samePath(a: ColumnPath, b: ColumnPath): boolean { + return columnPathId(a) === columnPathId(b); +} + +/** Thrown by {@link parseColumnPath} for anything that isn't a path. */ +export class InvalidColumnPathError extends Error { + override name = "InvalidColumnPathError"; +} + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +/** + * Checks that `value` is a well-formed {@link ColumnPath} — a root naming a + * field or an instance by a non-empty string, and `keys` of non-empty + * strings — and returns it. Whether the root exists is the caller's + * business. + * + * @param at - Where `value` sits, for the error message. + * @throws {InvalidColumnPathError} naming the first problem found. + */ +export function parseColumnPath(value: unknown, at = "path"): ColumnPath { + if (!isRecord(value)) + throw new InvalidColumnPathError(`${at} must be an object`); + const { root, keys } = value; + if (!isRecord(root)) { + throw new InvalidColumnPathError(`${at}.root must be an object`); + } + let parsedRoot: ColumnPath["root"]; + if (typeof root.field === "string" && root.field !== "") { + parsedRoot = { field: root.field }; + } else if (typeof root.instance === "string" && root.instance !== "") { + parsedRoot = { instance: root.instance }; + } else { + throw new InvalidColumnPathError( + `${at}.root must be { field } or { instance }, naming it by a non-empty string`, + ); + } + if (!Array.isArray(keys)) { + throw new InvalidColumnPathError(`${at}.keys must be an array`); + } + for (const [i, key] of keys.entries()) { + if (typeof key !== "string" || key === "") { + throw new InvalidColumnPathError( + `${at}.keys[${i}] must be a non-empty string`, + ); + } + } + return { root: parsedRoot, keys: [...keys] }; +} From 41c58bcd2f91066f54b9a87ec3eb9d8a6fa0b41d Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 15:28:58 +0000 Subject: [PATCH 2/8] Describe instances and paths, and store a shared column layout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit describeRows now summarizes the rows' resource instances, and describePath lists the keys one level inside any column path, walking each row by readColumnPath's rules (specs/datasets.md §Q.2). A dataset gains columns and rowLabel metadata, written by setColumns (PATCH .../columns) into a new nullable layout column; deleteField prunes the field's columns (§Q.3). Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01QMiMTviyLp3ciCvQf3mzkf --- src/client/api.ts | 39 ++++ src/client/components/DatasetView.tsx | 4 +- src/dataset/dataset-provider-contract.ts | 179 +++++++++++++++ src/dataset/dataset-provider.ts | 30 ++- src/dataset/dataset-types.ts | 28 +++ .../migration.sql | 4 + .../snapshot.json | 210 ++++++++++++++++++ src/dataset/db/migrations/bundled.ts | 9 + src/dataset/db/schema.ts | 5 + .../local-directory-dataset-provider.ts | 27 ++- src/dataset/turso-dataset-provider.ts | 160 ++++++++++++- src/server/api-routes.ts | 18 ++ src/server/handlers/datasets.test.ts | 116 ++++++++++ src/server/handlers/datasets.ts | 103 +++++++++ src/shared/column-paths.ts | 95 ++++++++ src/shared/types.ts | 2 + 16 files changed, 1024 insertions(+), 5 deletions(-) create mode 100644 src/dataset/db/migrations/20261012152455_column_layout/migration.sql create mode 100644 src/dataset/db/migrations/20261012152455_column_layout/snapshot.json diff --git a/src/client/api.ts b/src/client/api.ts index 5299df2..6771694 100644 --- a/src/client/api.ts +++ b/src/client/api.ts @@ -14,6 +14,7 @@ import type { TraceCheckResult, } from "../eval/eval-types"; import type { AgentContext, AgentInfo } from "../shared/agent"; +import type { ColumnPath, DatasetColumnSpec } from "../shared/column-paths"; import type { AddDatasetFieldRequest } from "../shared/dataset-fields"; import type { SetupTask } from "../shared/setup-task"; import type { @@ -24,6 +25,7 @@ import type { ConflictChoices, Dataset, DatasetField, + DatasetPathChild, DatasetProviderInfo, DatasetRow, DatasetRowsOverview, @@ -778,6 +780,43 @@ export async function deleteDatasetField( await throwIfError(res); } +/** + * The keys one level inside `path` in a dataset's rows, each with a sample + * value — what the column picker's path tree expands a node into. + */ +export async function describeDatasetPath( + providerId: string, + datasetId: string, + path: ColumnPath, +): Promise { + const res = await fetch(datasetUrl(providerId, datasetId, "/paths"), { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ path }), + }); + await throwIfError(res); + return res.json(); +} + +/** + * Replaces a dataset's shared column layout and row label (`null` clears + * it). Resolves to the updated dataset. + */ +export async function setDatasetColumns( + providerId: string, + datasetId: string, + columns: DatasetColumnSpec[], + rowLabel: ColumnPath | null, +): Promise { + const res = await fetch(datasetUrl(providerId, datasetId, "/columns"), { + method: "PATCH", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ columns, rowLabel }), + }); + await throwIfError(res); + return res.json(); +} + /** Deletes one row of a dataset. */ export async function deleteDatasetRow( providerId: string, diff --git a/src/client/components/DatasetView.tsx b/src/client/components/DatasetView.tsx index 26b1f9e..883de75 100644 --- a/src/client/components/DatasetView.tsx +++ b/src/client/components/DatasetView.tsx @@ -358,6 +358,7 @@ function DatasetView({ const [overview, setOverview] = useState({ rowCount: 0, fields: {}, + instances: {}, }); const [error, setError] = useState(null); /** Bumped after a local change, to refetch as a `version` bump would. */ @@ -444,12 +445,13 @@ function DatasetView({ useEffect(() => { let cancelled = false; getDataset(providerId, datasetId) - .then(({ dataset, rowCount, fields }) => { + .then(({ dataset, rowCount, fields, instances }) => { if (cancelled) return; setDataset(dataset); setOverview({ rowCount: rowCount + pendingAppends.current.count, fields, + instances, }); setError(appendError.current); appendError.current = null; diff --git a/src/dataset/dataset-provider-contract.ts b/src/dataset/dataset-provider-contract.ts index 46d8ddf..b581422 100644 --- a/src/dataset/dataset-provider-contract.ts +++ b/src/dataset/dataset-provider-contract.ts @@ -226,6 +226,7 @@ export function runDatasetProviderContractTests( expect(await provider.describeRows("nope")).toEqual({ rowCount: 0, fields: {}, + instances: {}, }); const dataset = await provider.createDataset({ name: "Shapes", @@ -294,6 +295,184 @@ export function runDatasetProviderContractTests( "2": { keys: ["db", "userId"] }, "3": { keys: ["title", "note"] }, }, + instances: {}, + }); + }); + + describe("column paths (§Q.2)", () => { + /** asgard's planTask rows: the title sits two references deep. */ + async function planTask(provider: DatasetProvider) { + const dataset = await provider.createDataset({ + name: "Plan task", + fields: [{ def: def("toolsContext", "Ctx") }, { def: def("note") }], + }); + const ctx = (task: string): ExecutionInput => ({ + kind: "object", + properties: { + create_task: { + kind: "object", + properties: { + db: { kind: "instance", name: "db" }, + rootTaskId: { kind: "instance", name: task, output: "taskId" }, + }, + }, + }, + }); + await provider.addRows(dataset.id, [ + { + cells: { "0": ctx("seededTask") }, + resources: { + db: { uri: "file:///db.ts#db" }, + seededTask: { + uri: "file:///tasks.ts#seededTask", + args: { title: text("Web site") }, + }, + }, + }, + // Another row names its focus task differently, and gives it more. + { + cells: { "0": ctx("focus"), "1": text("n") }, + resources: { + db: { uri: "file:///db.ts#db", args: { seed: text("1") } }, + focus: { + uri: "file:///tasks.ts#seededTask", + args: { title: text("Todo app"), owner: text("ann") }, + }, + }, + }, + { cells: { "1": text("no instances") } }, + ]); + return dataset; + } + + it("summarizes the rows' instances by name", async () => { + const provider = await makeProvider(); + const dataset = await planTask(provider); + expect((await provider.describeRows(dataset.id)).instances).toEqual({ + db: { uri: "file:///db.ts#db", rows: 2, args: ["seed"] }, + seededTask: { + uri: "file:///tasks.ts#seededTask", + rows: 1, + args: ["title"], + }, + focus: { + uri: "file:///tasks.ts#seededTask", + rows: 1, + args: ["title", "owner"], + }, + }); + }); + + it("describes the keys inside a path, resolving references per row", async () => { + const provider = await makeProvider(); + const dataset = await planTask(provider); + const at = (keys: string[]) => + provider.describePath(dataset.id, { root: { field: "0" }, keys }); + + expect(await at([])).toEqual([ + { + key: "create_task", + sample: expect.objectContaining({ kind: "object" }), + }, + ]); + expect((await at(["create_task"])).map(c => c.key)).toEqual([ + "db", + "rootTaskId", + ]); + // Each row's reference leads to its own instance; their arguments merge. + expect(await at(["create_task", "rootTaskId"])).toEqual([ + { key: "title", sample: text("Web site") }, + { key: "owner", sample: text("ann") }, + ]); + expect(await at(["create_task", "rootTaskId", "title"])).toEqual([]); + expect( + await provider.describePath(dataset.id, { + root: { instance: "focus" }, + keys: [], + }), + ).toEqual([ + { key: "title", sample: text("Todo app") }, + { key: "owner", sample: text("ann") }, + ]); + expect( + await provider.describePath("nope", { + root: { field: "0" }, + keys: [], + }), + ).toEqual([]); + }); + }); + + describe("setColumns (§Q.3)", () => { + it("stores the layout and row label as dataset metadata", async () => { + const provider = await makeProvider(); + const dataset = await provider.createDataset({ + name: "Layout", + fields: [{ def: def("a") }, { def: def("b") }], + }); + expect(dataset.columns).toBeUndefined(); + const columns = [ + { kind: "path" as const, path: { root: { field: "1" }, keys: [] } }, + { kind: "source" as const, hidden: true as const }, + // A newer client's column, kept as it was written. + { kind: "evalResults", hidden: true as const, run: "r1" }, + { + kind: "path" as const, + path: { root: { instance: "task" }, keys: ["title"] }, + title: "Task", + added: true as const, + }, + ]; + const rowLabel = { root: { instance: "task" }, keys: ["title"] }; + const events: DatasetChangeEvent[] = []; + provider.watch?.(e => events.push(e)); + + const updated = await provider.setColumns( + dataset.id, + columns, + rowLabel, + ); + expect(updated).toMatchObject({ id: dataset.id, columns, rowLabel }); + expect(await provider.getDataset(dataset.id)).toEqual(updated); + if (provider.watch) { + expect(events).toContainEqual({ + type: "update", + datasetId: dataset.id, + }); + } + + const cleared = await provider.setColumns(dataset.id, [], undefined); + expect(cleared.columns).toEqual([]); + expect(cleared.rowLabel).toBeUndefined(); + await expect( + provider.setColumns("missing", [], undefined), + ).rejects.toBeInstanceOf(DatasetNotFoundError); + }); + + it("drops a deleted field's columns, and the row label if it read from it", async () => { + const provider = await makeProvider(); + if (!provider.deleteField) return; + const dataset = await provider.createDataset({ + name: "Prune", + fields: [{ def: def("a") }, { def: def("b") }], + }); + await provider.setColumns( + dataset.id, + [ + { kind: "path", path: { root: { field: "0" }, keys: [] } }, + { kind: "path", path: { root: { field: "0" }, keys: ["k"] } }, + { kind: "path", path: { root: { field: "1" }, keys: [] } }, + { kind: "source" }, + ], + { root: { field: "0" }, keys: ["k"] }, + ); + await provider.deleteField(dataset.id, "0"); + const pruned = await provider.getDataset(dataset.id); + expect(pruned?.columns).toEqual([ + { kind: "path", path: { root: { field: "1" }, keys: [] } }, + { kind: "source" }, + ]); + expect(pruned?.rowLabel).toBeUndefined(); }); }); diff --git a/src/dataset/dataset-provider.ts b/src/dataset/dataset-provider.ts index 8d40979..eda4cbd 100644 --- a/src/dataset/dataset-provider.ts +++ b/src/dataset/dataset-provider.ts @@ -1,6 +1,7 @@ // SPDX-License-Identifier: AGPL-3.0-only // Copyright (c) 2026 Alexander Corrado +import type { ColumnPath, DatasetColumnSpec } from "../shared/column-paths.ts"; import type { PropDefinition } from "../shared/types.ts"; import type { SqlQueryOptions, @@ -11,6 +12,7 @@ import type { Dataset, DatasetChangeEvent, DatasetField, + DatasetPathChild, DatasetRow, DatasetRowsOverview, DatasetRowUpdate, @@ -97,6 +99,18 @@ export interface DatasetProvider { */ describeRows(datasetId: string): Promise; + /** + * The keys one level inside `path`, merged across rows in order of first + * appearance, each with a sample of what the first row that has it holds + * there. Each row is walked by `readColumnPath`'s rules, so a path through + * an instance reference resolves per row. Empty for an unknown dataset, or + * a path that leads nowhere. See `specs/datasets.md` §Q.2. + */ + describePath( + datasetId: string, + path: ColumnPath, + ): Promise; + /** Creates a dataset, minting its id and field ids. */ createDataset(input: CreateDatasetInput): Promise; @@ -154,12 +168,26 @@ export interface DatasetProvider { /** * Deletes a field and every row's cell for it. Its id is never reused. An - * unknown field is rejected with a {@link DatasetValidationError}. + * unknown field is rejected with a {@link DatasetValidationError}. The + * field's columns are dropped from the layout, and the row label with them + * if it read from the field. * * Optional — as {@link renameField}. */ deleteField?(datasetId: string, fieldId: string): Promise; + /** + * Replaces the dataset's column layout and row label — metadata, so rows + * are untouched. `columns` is stored as given (any validation is the + * caller's); an undefined `rowLabel` clears it. Resolves to the updated + * dataset. See `specs/datasets.md` §Q.3. + */ + setColumns( + datasetId: string, + columns: DatasetColumnSpec[], + rowLabel: ColumnPath | undefined, + ): Promise; + /** * Runs one read-only SQL query against a `rows` view of a dataset's rows — * a column per field, named after it, plus `_id`, `_created_at`, diff --git a/src/dataset/dataset-types.ts b/src/dataset/dataset-types.ts index d8fde81..6643088 100644 --- a/src/dataset/dataset-types.ts +++ b/src/dataset/dataset-types.ts @@ -6,6 +6,7 @@ * to exercise prompts. See `specs/datasets.md`. */ +import type { ColumnPath, DatasetColumnSpec } from "../shared/column-paths.ts"; import type { ExecutionInput, PropDefinition, @@ -60,12 +61,37 @@ export interface Dataset { * survives moves and renames; the server resolves it on read. */ prompt?: PromptID; + /** + * The grid's columns, in order — a layout, not the schema, so a client + * reconciles it with the fields and rows it actually has on read. Absent + * until someone arranges the columns. See `specs/datasets.md` §Q.3. + */ + columns?: DatasetColumnSpec[]; + /** The path whose value names a row wherever a row is named. */ + rowLabel?: ColumnPath; /** Creation timestamp (ms). */ createdAt: number; /** Timestamp (ms) of the last change to the dataset or its rows. */ updatedAt: number; } +/** One key one level inside a path, as `DatasetProvider.describePath` finds it. */ +export interface DatasetPathChild { + key: string; + /** What the first row that has this key holds there. */ + sample?: ExecutionInput; +} + +/** One of the rows' resource instances, summarized across rows by name. */ +export interface DatasetInstanceShape { + /** The resource it instantiates, in the first row that declares it. */ + uri: string; + /** How many rows declare it. */ + rows: number; + /** Its argument names, merged across rows in order of first appearance. */ + args: string[]; +} + /** Where a {@link DatasetRow} came from, so the view can link back to it. */ export type DatasetRowSource = | { kind: "playground"; promptId: string; providerId: string } @@ -135,6 +161,8 @@ export interface DatasetRowsOverview { * show. A field whose cells are all plain values has no entry. */ fields: Record; + /** Instance name → its resource and argument names, merged across rows. */ + instances: Record; } /** Compact dataset entry for listings (sidebar / `GET /api/datasets`). */ diff --git a/src/dataset/db/migrations/20261012152455_column_layout/migration.sql b/src/dataset/db/migrations/20261012152455_column_layout/migration.sql new file mode 100644 index 0000000..6a7ed4f --- /dev/null +++ b/src/dataset/db/migrations/20261012152455_column_layout/migration.sql @@ -0,0 +1,4 @@ +-- SPDX-License-Identifier: AGPL-3.0-only +-- Copyright (c) 2026 Alexander Corrado + +ALTER TABLE `datasets` ADD `layout` text; diff --git a/src/dataset/db/migrations/20261012152455_column_layout/snapshot.json b/src/dataset/db/migrations/20261012152455_column_layout/snapshot.json new file mode 100644 index 0000000..62a04c9 --- /dev/null +++ b/src/dataset/db/migrations/20261012152455_column_layout/snapshot.json @@ -0,0 +1,210 @@ +{ + "version": "7", + "dialect": "sqlite", + "id": "c0a753c2-5254-4ed4-9cf6-fae325d3eaf4", + "prevIds": [ + "30558ebf-c3d8-4571-a95c-df42a830fa30" + ], + "ddl": [ + { + "name": "dataset_rows", + "entityType": "tables" + }, + { + "name": "datasets", + "entityType": "tables" + }, + { + "type": "text", + "notNull": false, + "autoincrement": false, + "default": null, + "generated": null, + "name": "id", + "entityType": "columns", + "table": "dataset_rows" + }, + { + "type": "text", + "notNull": true, + "autoincrement": false, + "default": null, + "generated": null, + "name": "dataset_id", + "entityType": "columns", + "table": "dataset_rows" + }, + { + "type": "blob", + "notNull": true, + "autoincrement": false, + "default": null, + "generated": null, + "name": "cells", + "entityType": "columns", + "table": "dataset_rows" + }, + { + "type": "blob", + "notNull": false, + "autoincrement": false, + "default": null, + "generated": null, + "name": "source", + "entityType": "columns", + "table": "dataset_rows" + }, + { + "type": "blob", + "notNull": false, + "autoincrement": false, + "default": null, + "generated": null, + "name": "resources", + "entityType": "columns", + "table": "dataset_rows" + }, + { + "type": "real", + "notNull": true, + "autoincrement": false, + "default": null, + "generated": null, + "name": "created_at", + "entityType": "columns", + "table": "dataset_rows" + }, + { + "type": "text", + "notNull": false, + "autoincrement": false, + "default": null, + "generated": null, + "name": "id", + "entityType": "columns", + "table": "datasets" + }, + { + "type": "text", + "notNull": true, + "autoincrement": false, + "default": null, + "generated": null, + "name": "name", + "entityType": "columns", + "table": "datasets" + }, + { + "type": "text", + "notNull": true, + "autoincrement": false, + "default": null, + "generated": null, + "name": "fields", + "entityType": "columns", + "table": "datasets" + }, + { + "type": "integer", + "notNull": true, + "autoincrement": false, + "default": null, + "generated": null, + "name": "next_field_id", + "entityType": "columns", + "table": "datasets" + }, + { + "type": "text", + "notNull": false, + "autoincrement": false, + "default": null, + "generated": null, + "name": "prompt", + "entityType": "columns", + "table": "datasets" + }, + { + "type": "text", + "notNull": false, + "autoincrement": false, + "default": null, + "generated": null, + "name": "layout", + "entityType": "columns", + "table": "datasets" + }, + { + "type": "real", + "notNull": true, + "autoincrement": false, + "default": null, + "generated": null, + "name": "created_at", + "entityType": "columns", + "table": "datasets" + }, + { + "type": "real", + "notNull": true, + "autoincrement": false, + "default": null, + "generated": null, + "name": "updated_at", + "entityType": "columns", + "table": "datasets" + }, + { + "columns": [ + "dataset_id" + ], + "tableTo": "datasets", + "columnsTo": [ + "id" + ], + "onUpdate": "NO ACTION", + "onDelete": "CASCADE", + "nameExplicit": false, + "name": "fk_dataset_rows_dataset_id_datasets_id_fk", + "entityType": "fks", + "table": "dataset_rows" + }, + { + "columns": [ + "id" + ], + "nameExplicit": false, + "name": "dataset_rows_pk", + "table": "dataset_rows", + "entityType": "pks" + }, + { + "columns": [ + "id" + ], + "nameExplicit": false, + "name": "datasets_pk", + "table": "datasets", + "entityType": "pks" + }, + { + "columns": [ + { + "value": "dataset_id", + "isExpression": false + }, + { + "value": "created_at", + "isExpression": false + } + ], + "isUnique": false, + "where": null, + "origin": "manual", + "name": "idx_dataset_rows_dataset", + "entityType": "indexes", + "table": "dataset_rows" + } + ], + "renames": [] +} \ No newline at end of file diff --git a/src/dataset/db/migrations/bundled.ts b/src/dataset/db/migrations/bundled.ts index c84b44d..a2e3aa7 100644 --- a/src/dataset/db/migrations/bundled.ts +++ b/src/dataset/db/migrations/bundled.ts @@ -28,5 +28,14 @@ export const bundledMigrations: MigrationMeta[] = [ "sql": [ "-- SPDX\u002DLicense-Identifier: AGPL-3.0-only\n-- Copyright (c) 2026 Alexander Corrado\n\nALTER TABLE `dataset_rows` ADD `resources` blob;\n" ] + }, + { + "name": "20261012152455_column_layout", + "hash": "a226b20ea4b58a7dd558132c94611a2fb09e97e408e34ba6237387293c8fe869", + "folderMillis": 1791818695000, + "bps": true, + "sql": [ + "-- SPDX\u002DLicense-Identifier: AGPL-3.0-only\n-- Copyright (c) 2026 Alexander Corrado\n\nALTER TABLE `datasets` ADD `layout` text;\n" + ] } ]; diff --git a/src/dataset/db/schema.ts b/src/dataset/db/schema.ts index 04a6d55..c9d5c93 100644 --- a/src/dataset/db/schema.ts +++ b/src/dataset/db/schema.ts @@ -47,6 +47,11 @@ export const datasets = sqliteTable("datasets", { nextFieldId: integer("next_field_id").notNull(), /** JSON `PromptID`. */ prompt: text("prompt"), + /** + * JSON `{ columns?, rowLabel? }` — the grid's shared column layout and the + * row label (`specs/datasets.md` §Q.3). NULL until someone arranges them. + */ + layout: text("layout"), /** Creation timestamp (ms). */ createdAt: real("created_at").notNull(), /** Last-change timestamp (ms). */ diff --git a/src/dataset/local-directory-dataset-provider.ts b/src/dataset/local-directory-dataset-provider.ts index 83ba207..b3ed4f3 100644 --- a/src/dataset/local-directory-dataset-provider.ts +++ b/src/dataset/local-directory-dataset-provider.ts @@ -5,6 +5,7 @@ import { access, constants, readdir, stat, unlink } from "node:fs/promises"; import { join, resolve } from "node:path"; import type { Database } from "@tursodatabase/sync"; import { drizzle } from "drizzle-orm/tursodatabase-sync"; +import type { ColumnPath, DatasetColumnSpec } from "../shared/column-paths.ts"; import type { PropDefinition } from "../shared/types.ts"; import { assertSqliteFile, @@ -32,6 +33,7 @@ import type { Dataset, DatasetChangeEvent, DatasetField, + DatasetPathChild, DatasetRow, DatasetRowsOverview, DatasetRowUpdate, @@ -300,10 +302,19 @@ export class LocalDirectoryDatasetProvider implements DatasetProvider { async describeRows(datasetId: string): Promise { const entry = await this.entryFor(datasetId); - if (!entry?.ok) return { rowCount: 0, fields: {} }; + if (!entry?.ok) return { rowCount: 0, fields: {}, instances: {} }; return entry.inner.describeRows(entry.innerId); } + async describePath( + datasetId: string, + path: ColumnPath, + ): Promise { + const entry = await this.entryFor(datasetId); + if (!entry?.ok) return []; + return entry.inner.describePath(entry.innerId, path); + } + /** * Creates `.db` (`-2.db` on collision) and the dataset inside * it. Creates are chained, so two at once can't pick the same id or open @@ -411,6 +422,20 @@ export class LocalDirectoryDatasetProvider implements DatasetProvider { await entry.inner.deleteField(entry.innerId, fieldId); } + async setColumns( + datasetId: string, + columns: DatasetColumnSpec[], + rowLabel: ColumnPath | undefined, + ): Promise { + const entry = await this.requireEntry(datasetId); + const updated = await entry.inner.setColumns( + entry.innerId, + columns, + rowLabel, + ); + return this.withFileId(datasetId, updated); + } + async queryRows( datasetId: string, sql: string, diff --git a/src/dataset/turso-dataset-provider.ts b/src/dataset/turso-dataset-provider.ts index 8ebbbdb..4564010 100644 --- a/src/dataset/turso-dataset-provider.ts +++ b/src/dataset/turso-dataset-provider.ts @@ -11,6 +11,13 @@ import type { Database } from "@tursodatabase/sync"; import { and, asc, desc, eq, inArray, type SQL, sql } from "drizzle-orm"; import { drizzle } from "drizzle-orm/tursodatabase-sync"; +import { + type ColumnPath, + childInputs, + type DatasetColumnSpec, + pathField, + resolveColumnPath, +} from "../shared/column-paths.ts"; import { matchKey } from "../shared/dataset-fields.ts"; import { isValidInstanceName } from "../shared/instance-names.ts"; import type { @@ -46,6 +53,8 @@ import type { DatasetChangeEvent, DatasetField, DatasetFieldShape, + DatasetInstanceShape, + DatasetPathChild, DatasetRow, DatasetRowSource, DatasetRowsOverview, @@ -58,12 +67,21 @@ function parseJson(v: string | null): T | undefined { return v == null ? undefined : (JSON.parse(v) as T); } +/** What the `layout` column holds — see `Dataset.columns`. */ +interface StoredLayout { + columns?: DatasetColumnSpec[]; + rowLabel?: ColumnPath; +} + function rowToDataset(row: typeof datasets.$inferSelect): Dataset { + const layout = parseJson(row.layout); return { id: row.id, name: row.name, fields: JSON.parse(row.fields) as DatasetField[], ...(row.prompt && { prompt: parseJson(row.prompt) }), + ...(layout?.columns && { columns: layout.columns }), + ...(layout?.rowLabel && { rowLabel: layout.rowLabel }), createdAt: row.createdAt, updatedAt: row.updatedAt, }; @@ -323,7 +341,97 @@ export class TursoDatasetProvider implements DatasetProvider { fields[field] ??= { keys: [] }; fields[field].keys.push(key); } - return { rowCount: Number(count), fields }; + return { + rowCount: Number(count), + fields, + instances: await this.describeInstances(datasetId), + }; + } + + /** + * The rows' resource instances, by name, in order of first appearance: + * each one's resource, how many rows declare it, and its argument names + * merged across rows the way {@link describeRows} merges a field's keys. + */ + private async describeInstances( + datasetId: string, + ): Promise> { + // `json_each(NULL)` yields a row in Turso, hence the IS NOT NULL. + // Grouped by resource too, since one name may instantiate different + // resources in different rows; the first row's is the one reported. + const declared = await this.db.all<{ + name: string; + uri: string | null; + rows: number; + }>(sql` + SELECT inst.key AS name, json_extract(inst.value, '$.uri') AS uri, + count(*) AS rows + FROM ${datasetRows}, json_each(${datasetRows.resources}) AS inst + WHERE ${datasetRows.datasetId} = ${datasetId} + AND ${datasetRows.resources} IS NOT NULL + GROUP BY inst.key, uri + ORDER BY min(${datasetRows}.rowid), min(inst.id) + `); + const args = await this.db.all<{ name: string; arg: string }>(sql` + SELECT inst.key AS name, arg.key AS arg + FROM ${datasetRows}, + json_each(${datasetRows.resources}) AS inst, + json_each(inst.value, '$.args') AS arg + WHERE ${datasetRows.datasetId} = ${datasetId} + AND ${datasetRows.resources} IS NOT NULL + GROUP BY inst.key, arg.key + ORDER BY inst.key, min(${datasetRows}.rowid), min(arg.id) + `); + + const instances: Record = {}; + for (const { name, uri, rows } of declared) { + const known = instances[name]; + if (known) known.rows += Number(rows); + else instances[name] = { uri: uri ?? "", rows: Number(rows), args: [] }; + } + for (const { name, arg } of args) instances[name]?.args.push(arg); + return instances; + } + + async describePath( + datasetId: string, + path: ColumnPath, + ): Promise { + // Only rows with something at the root can have anything inside it. The + // rest of the walk is `resolveColumnPath`'s, row by row, so it agrees + // with what the grid reads — references included, which resolve per row. + const field = pathField(path); + const rootKey = + field !== undefined + ? `$."${field}"` + : `$."${"instance" in path.root ? path.root.instance : ""}"`; + const rootColumn = + field !== undefined ? datasetRows.cells : datasetRows.resources; + const rows = await this.db.all<{ + cells: string | null; + resources: string | null; + }>(sql` + SELECT json(${datasetRows.cells}) AS cells, + json(${datasetRows.resources}) AS resources + FROM ${datasetRows} + WHERE ${datasetRows.datasetId} = ${datasetId} + AND json_type(${rootColumn}, ${rootKey}) IS NOT NULL + ORDER BY ${datasetRows.createdAt}, ${datasetRows}.rowid + `); + + const children = new Map(); + for (const stored of rows) { + const row = { + cells: parseJson>(stored.cells) ?? {}, + resources: parseJson(stored.resources), + }; + const { value } = resolveColumnPath(row, path); + if (typeof value === "string") continue; + for (const [key, sample] of childInputs(row, value)) { + if (!children.has(key)) children.set(key, { key, sample }); + } + } + return [...children.values()]; } async createDataset( @@ -662,7 +770,12 @@ export class TursoDatasetProvider implements DatasetProvider { async deleteField(datasetId: string, fieldId: string): Promise { await this.serializeWrite(() => this.db.transaction(async tx => { - const fields = await this.fieldsIn(tx, datasetId); + const [row] = await tx + .select({ fields: datasets.fields, layout: datasets.layout }) + .from(datasets) + .where(eq(datasets.id, datasetId)); + if (!row) throw new DatasetNotFoundError(datasetId); + const fields = JSON.parse(row.fields) as DatasetField[]; if (!fields.some(f => f.id === fieldId)) { throw noSuchField(datasetId, fieldId); } @@ -672,6 +785,7 @@ export class TursoDatasetProvider implements DatasetProvider { .update(datasets) .set({ fields: JSON.stringify(fields.filter(f => f.id !== fieldId)), + layout: layoutWithoutField(row.layout, fieldId), updatedAt: Date.now(), }) .where(eq(datasets.id, datasetId)); @@ -686,6 +800,25 @@ export class TursoDatasetProvider implements DatasetProvider { this.emit({ type: "update", datasetId }); } + async setColumns( + datasetId: string, + columns: DatasetColumnSpec[], + rowLabel: ColumnPath | undefined, + ): Promise { + const dataset = await this.serializeWrite(async () => { + const layout: StoredLayout = { columns, ...(rowLabel && { rowLabel }) }; + const updated = await this.db + .update(datasets) + .set({ layout: JSON.stringify(layout), updatedAt: Date.now() }) + .where(eq(datasets.id, datasetId)) + .returning(); + if (updated.length === 0) throw new DatasetNotFoundError(datasetId); + return rowToDataset(updated[0]); + }); + this.emit({ type: "update", datasetId }); + return dataset; + } + /** * Runs one read-only query against a `rows` view of the dataset (see * `./dataset-query.ts`). Queued on the write chain, as @@ -718,6 +851,29 @@ export class TursoDatasetProvider implements DatasetProvider { } } +/** + * A stored layout without the columns that read from `fieldId`, nor the row + * label if it did — so a deleted field doesn't linger as a dead entry. + */ +function layoutWithoutField( + stored: string | null, + fieldId: string, +): string | null { + const layout = parseJson(stored); + if (!layout) return stored; + const readsField = (path: ColumnPath) => pathField(path) === fieldId; + const pruned: StoredLayout = { + ...(layout.columns && { + columns: layout.columns.filter( + c => !(c.kind === "path" && readsField(c.path as ColumnPath)), + ), + }), + ...(layout.rowLabel && + !readsField(layout.rowLabel) && { rowLabel: layout.rowLabel }), + }; + return JSON.stringify(pruned); +} + function noSuchField(datasetId: string, fieldId: string): Error { return new DatasetValidationError( `Dataset ${datasetId} has no field with id "${fieldId}"`, diff --git a/src/server/api-routes.ts b/src/server/api-routes.ts index 9c08e7c..0940bcd 100644 --- a/src/server/api-routes.ts +++ b/src/server/api-routes.ts @@ -38,12 +38,14 @@ import { handleDeleteDataset, handleDeleteField, handleDeleteRow, + handleDescribePath, handleGetDataset, handleListDatasets, handleListRows, handleQueryRows, handleRenameDataset, handleRenameField, + handleSetColumns, handleUpdateRows, } from "./handlers/datasets.ts"; import { @@ -900,6 +902,22 @@ export function setupRoutes({ ), ); + // POST /api/datasets/:providerId/:id/paths - The keys one level inside a column path + app.post( + "/api/datasets/:providerId/:id/paths", + datasetRoute(async (provider, { id }, c) => + handleDescribePath(provider, id, await jsonBody(c)), + ), + ); + + // PATCH /api/datasets/:providerId/:id/columns - The shared column layout and row label + app.patch( + "/api/datasets/:providerId/:id/columns", + datasetRoute(async (provider, { id }, c) => + handleSetColumns(provider, id, await jsonBody(c), resolvePromptLink), + ), + ); + // POST /api/datasets/:providerId/:id/query - One read-only SQL query over the rows app.post( "/api/datasets/:providerId/:id/query", diff --git a/src/server/handlers/datasets.test.ts b/src/server/handlers/datasets.test.ts index 4707f97..2695256 100644 --- a/src/server/handlers/datasets.test.ts +++ b/src/server/handlers/datasets.test.ts @@ -10,6 +10,8 @@ import type { ExecutionInput, PropDefinition } from "../../shared/types.ts"; import { type FieldSourcePrompt, handleAddField, + handleDescribePath, + handleSetColumns, handleUpdateRows, type LookupFieldSourcePrompt, primitiveFieldDef, @@ -446,3 +448,117 @@ describe("handleUpdateRows", () => { expect(res.status).toBe(404); }); }); + +describe("column layout handlers (§Q)", () => { + let client: Database | undefined; + + afterEach(async () => { + await client?.close(); + client = undefined; + }); + + /** A dataset of `title` and `ctx`, with one row declaring `task`. */ + async function seeded() { + client = await connect({ path: ":memory:", url: () => null }); + await runDatasetMigrations(drizzle({ client })); + const provider = new TursoDatasetProvider({ client }); + const dataset = await provider.createDataset({ + name: "Layout", + fields: [ + { def: primitiveFieldDef("title", "string") }, + { def: primitiveFieldDef("ctx", "string") }, + ], + }); + await provider.addRows(dataset.id, [ + { + cells: { "1": { kind: "instance", name: "task", output: "taskId" } }, + resources: { + task: { + uri: "file:///t.ts#task", + args: { + title: { + kind: "value", + value: { kind: "primitive", value: "Todo" }, + }, + }, + }, + }, + }, + ]); + const set = (body: unknown) => + handleSetColumns(provider, dataset.id, body, p => p); + return { provider, dataset, set }; + } + + it("stores columns and a row label whose roots exist", async () => { + const { provider, dataset, set } = await seeded(); + const columns = [ + { kind: "source", hidden: true }, + { kind: "path", path: { root: { field: "0" }, keys: [] } }, + { + kind: "path", + path: { root: { instance: "task" }, keys: ["title"] }, + added: true, + }, + { kind: "fromTheFuture", whatever: 1 }, + ]; + const rowLabel = { root: { field: "1" }, keys: ["title"] }; + const res = await set({ columns, rowLabel }); + expect(res.status).toBe(200); + const stored = await provider.getDataset(dataset.id); + expect(stored?.columns).toEqual(columns); + expect(stored?.rowLabel).toEqual(rowLabel); + + expect((await set({ columns: [], rowLabel: null })).status).toBe(200); + expect((await provider.getDataset(dataset.id))?.rowLabel).toBeUndefined(); + }); + + it("rejects an unknown root, an empty key, or a malformed body", async () => { + const { set } = await seeded(); + const path = (root: unknown, keys: unknown = []) => ({ + columns: [{ kind: "path", path: { root, keys } }], + }); + expect((await set(path({ field: "9" }))).status).toBe(400); + expect((await set(path({ instance: "nope" }))).status).toBe(400); + expect((await set(path({ field: "0" }, [""]))).status).toBe(400); + expect( + (await set({ columns: [], rowLabel: { root: { field: "9" }, keys: [] } })) + .status, + ).toBe(400); + expect((await set({ columns: {} })).status).toBe(400); + expect((await set({ columns: [{ hidden: true }] })).status).toBe(400); + expect( + (await handleSetColumns(client as never, "x", null, p => p)).status, + ).toBe(400); + }); + + it("describes a path, refusing an unknown field", async () => { + const { provider, dataset } = await seeded(); + const describe = (path: unknown) => + handleDescribePath(provider, dataset.id, { path }); + const res = await describe({ root: { field: "1" }, keys: [] }); + expect(res).toEqual({ + status: 200, + body: [ + { + key: "title", + sample: { + kind: "value", + value: { kind: "primitive", value: "Todo" }, + }, + }, + ], + }); + expect((await describe({ root: { field: "9" }, keys: [] })).status).toBe( + 400, + ); + expect((await describe({ root: {}, keys: [] })).status).toBe(400); + expect( + ( + await handleDescribePath(provider, "nope", { + path: { root: { field: "0" }, keys: [] }, + }) + ).status, + ).toBe(404); + }); +}); diff --git a/src/server/handlers/datasets.ts b/src/server/handlers/datasets.ts index 55edee8..825f35e 100644 --- a/src/server/handlers/datasets.ts +++ b/src/server/handlers/datasets.ts @@ -22,6 +22,13 @@ import type { DatasetRowUpdate, DatasetSummary, } from "../../dataset/dataset-types.ts"; +import { + type ColumnPath, + InvalidColumnPathError, + parseColumnPath, + parseColumnSpecs, + pathField, +} from "../../shared/column-paths.ts"; import { fitsPrimitiveBase, InvalidCellError, @@ -35,6 +42,7 @@ import { type PrimitiveFieldType, portableDef, } from "../../shared/dataset-fields.ts"; +import { isValidInstanceName } from "../../shared/instance-names.ts"; import type { CheckInfo, ExecutionInput, @@ -75,6 +83,7 @@ function failure(err: unknown): DatasetHandlerResult { if ( err instanceof DatasetValidationError || err instanceof InvalidCellError || + err instanceof InvalidColumnPathError || err instanceof BadRequest ) { return { status: 400, body: { error: message } }; @@ -739,3 +748,97 @@ export async function handleQueryRows( failure, ); } + +/** + * Checks that `path`'s root exists: a field in `dataset`'s schema, or an + * instance some row declares. + */ +function checkPathRoot( + path: ColumnPath, + dataset: Dataset, + instances: ReadonlySet, + at: string, +): void { + const field = pathField(path); + if (field !== undefined) { + if (!dataset.fields.some(f => f.id === field)) { + throw new BadRequest(`${at}: the dataset has no field "${field}"`); + } + } else if ("instance" in path.root && !instances.has(path.root.instance)) { + throw new BadRequest( + `${at}: no row declares a resource instance "${path.root.instance}"`, + ); + } +} + +/** + * `POST /api/datasets/:providerId/:id/paths` — body `{ path }`: the keys one + * level inside `path`, each with a sample (see + * {@link DatasetProvider.describePath}). See `specs/datasets.md` §Q.2. + */ +export async function handleDescribePath( + provider: DatasetProvider, + datasetId: string, + body: unknown, +): Promise { + try { + const path = parseColumnPath(isRecord(body) ? body.path : undefined); + const dataset = await provider.getDataset(datasetId); + if (!dataset) throw new DatasetNotFoundError(datasetId); + const field = pathField(path); + if (field !== undefined && !dataset.fields.some(f => f.id === field)) { + throw new BadRequest(`path: the dataset has no field "${field}"`); + } + if ("instance" in path.root && !isValidInstanceName(path.root.instance)) { + throw new BadRequest( + `path: "${path.root.instance}" is not a valid resource name`, + ); + } + return { status: 200, body: await provider.describePath(datasetId, path) }; + } catch (err) { + return failure(err); + } +} + +/** + * `PATCH /api/datasets/:providerId/:id/columns` — body `{ columns, rowLabel }`: + * replace the grid's shared column layout and row label (`null` clears the + * label). Each path's root must be a field of the dataset or an instance + * some row declares, and its keys non-empty strings. Columns of a kind this + * version doesn't know are stored as they are. See `specs/datasets.md` §Q.3. + */ +export async function handleSetColumns( + provider: DatasetProvider, + datasetId: string, + body: unknown, + resolvePrompt: ResolvePromptLink, +): Promise { + try { + if (!isRecord(body)) + throw new BadRequest("body must be { columns, rowLabel }"); + const columns = parseColumnSpecs(body.columns); + const rowLabel = + body.rowLabel === undefined || body.rowLabel === null + ? undefined + : parseColumnPath(body.rowLabel, "rowLabel"); + const dataset = await provider.getDataset(datasetId); + if (!dataset) throw new DatasetNotFoundError(datasetId); + const paths = columns.flatMap((c, i) => + c.kind === "path" + ? [{ path: c.path as ColumnPath, at: `columns[${i}].path` }] + : [], + ); + if (rowLabel) paths.push({ path: rowLabel, at: "rowLabel" }); + // Instance roots are checked against the rows only when there are any. + const instances = paths.some(({ path }) => "instance" in path.root) + ? new Set(Object.keys((await provider.describeRows(datasetId)).instances)) + : new Set(); + for (const { path, at } of paths) { + checkPathRoot(path, dataset, instances, at); + } + const updated = await provider.setColumns(datasetId, columns, rowLabel); + return { status: 200, body: withResolvedPrompt(updated, resolvePrompt) }; + } catch (err) { + return failure(err); + } +} diff --git a/src/shared/column-paths.ts b/src/shared/column-paths.ts index b4de00a..7fc9233 100644 --- a/src/shared/column-paths.ts +++ b/src/shared/column-paths.ts @@ -18,6 +18,48 @@ export interface ColumnPath { keys: string[]; } +/** + * One column of a dataset's layout (`Dataset.columns`), discriminated by `kind`. Kinds + * this version doesn't know (a newer client's) are stored and served as they + * were written — see {@link ForeignColumnSpec}. + */ +export type DatasetColumnSpec = KnownColumnSpec | ForeignColumnSpec; + +/** A column of the layout whose `kind` this version knows. */ +export type KnownColumnSpec = + | { + kind: "path"; + path: ColumnPath; + /** Shown instead of the path's last key. */ + title?: string; + hidden?: true; + /** Added from the picker, as opposed to derived from a field or one of its keys. */ + added?: true; + } + /** Where the row came from. Moved and hidden like any column, but never removed. */ + | { kind: "source"; hidden?: true }; + +/** + * A column of a `kind` this version doesn't know, written by a newer one. Kept + * in its place, and written back intact, so an older client arranging the + * columns doesn't drop it. + */ +export interface ForeignColumnSpec { + kind: string; + hidden?: true; + [property: string]: unknown; +} + +/** The kinds of {@link KnownColumnSpec}. */ +const KNOWN_COLUMN_KINDS = new Set(["path", "source"]); + +/** Whether `spec` is a column this version knows how to draw. */ +export function isKnownColumn( + spec: DatasetColumnSpec, +): spec is KnownColumnSpec { + return KNOWN_COLUMN_KINDS.has(spec.kind); +} + /** The parts of a row a path reads: its cells, and its resource instances. */ export interface PathRow { cells: Record; @@ -225,3 +267,56 @@ export function parseColumnPath(value: unknown, at = "path"): ColumnPath { } return { root: parsedRoot, keys: [...keys] }; } + +/** + * Checks that `value` is a well-formed column layout and returns it. Each + * entry must be an object with a string `kind`. A `path` entry's path is + * checked by {@link parseColumnPath}, and its `title`, `hidden` and `added` + * by type; a `source` entry carries only `hidden`. An entry of a kind this + * version doesn't know is kept as it was written, so a newer client's + * column survives a write from an older one. Whether a path's root exists + * is the caller's business. + * + * @param at - Where `value` sits, for the error message. + * @throws {InvalidColumnPathError} naming the first problem found. + */ +export function parseColumnSpecs( + value: unknown, + at = "columns", +): DatasetColumnSpec[] { + if (!Array.isArray(value)) { + throw new InvalidColumnPathError(`${at} must be an array`); + } + return value.map((entry, i): DatasetColumnSpec => { + const here = `${at}[${i}]`; + if (!isRecord(entry) || typeof entry.kind !== "string" || !entry.kind) { + throw new InvalidColumnPathError(`${here} must be an object with a kind`); + } + if (entry.hidden !== undefined && entry.hidden !== true) { + throw new InvalidColumnPathError(`${here}.hidden must be true if set`); + } + const hidden = entry.hidden === true ? { hidden: true as const } : {}; + switch (entry.kind) { + case "source": + return { kind: "source", ...hidden }; + case "path": { + if (entry.title !== undefined && typeof entry.title !== "string") { + throw new InvalidColumnPathError(`${here}.title must be a string`); + } + if (entry.added !== undefined && entry.added !== true) { + throw new InvalidColumnPathError(`${here}.added must be true if set`); + } + return { + kind: "path", + path: parseColumnPath(entry.path, `${here}.path`), + ...(typeof entry.title === "string" && + entry.title !== "" && { title: entry.title }), + ...hidden, + ...(entry.added === true && { added: true as const }), + }; + } + default: + return { ...entry, kind: entry.kind }; + } + }); +} diff --git a/src/shared/types.ts b/src/shared/types.ts index 8daf9dc..9b6a60b 100644 --- a/src/shared/types.ts +++ b/src/shared/types.ts @@ -13,6 +13,8 @@ export type { DatasetChangeType, DatasetField, DatasetFieldShape, + DatasetInstanceShape, + DatasetPathChild, DatasetProviderInfo, DatasetRow, DatasetRowSource, From 2eaab83690994ee8cbef61c33cbe9c2567d7f8f0 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 15:33:43 +0000 Subject: [PATCH 3/8] Reconcile the shared column layout and build the grid from it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit reconcileColumns fits a stored layout to the dataset's fields and rows, appending what's new, dropping what's gone and keeping unknown kinds in place (specs/datasets.md §Q.3). The grid's columns now follow it, including added columns, with the move rules header drags need and inline editing of an instance's argument through an added column (§Q.4). Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01QMiMTviyLp3ciCvQf3mzkf --- src/client/components/dataset-columns.test.ts | 245 +++++++++++ src/client/components/dataset-columns.ts | 363 ++++++++++++++++ src/client/components/dataset-grid.test.ts | 260 ++++++++++- src/client/components/dataset-grid.ts | 402 ++++++++++++++---- 4 files changed, 1196 insertions(+), 74 deletions(-) create mode 100644 src/client/components/dataset-columns.test.ts create mode 100644 src/client/components/dataset-columns.ts diff --git a/src/client/components/dataset-columns.test.ts b/src/client/components/dataset-columns.test.ts new file mode 100644 index 0000000..55c9018 --- /dev/null +++ b/src/client/components/dataset-columns.test.ts @@ -0,0 +1,245 @@ +// SPDX-License-Identifier: AGPL-3.0-only +// Copyright (c) 2026 Alexander Corrado + +import { describe, expect, it } from "vitest"; +import { + type DatasetColumnSpec, + fieldPath, + instancePath, +} from "../../shared/column-paths"; +import type { DatasetField } from "../../shared/types"; +import { + addColumn, + type ColumnLayout, + findPath, + isHidden, + moveColumn, + moveKey, + reconcileColumns, + removeColumn, + reorderVisible, + serializeColumns, + setHidden, +} from "./dataset-columns"; + +const field = (id: string, name: string): DatasetField => + ({ + id, + def: { name, type: { kind: "primitive", syntax: "string" } }, + }) as DatasetField; + +const FIELDS = [field("0", "ticket"), field("1", "task"), field("2", "info")]; +const SHAPE = { "1": { keys: ["title", "owner"] } }; + +const path = ( + p: ReturnType, + extra: Partial> = {}, +): DatasetColumnSpec => ({ kind: "path", path: p, ...extra }); + +/** Today's layout: every field in schema order, keys shown, then the source. */ +const DEFAULT: ColumnLayout = [ + { kind: "field", fieldId: "0", hidden: false, keys: [] }, + { + kind: "field", + fieldId: "1", + hidden: false, + keys: [ + { key: "title", hidden: false }, + { key: "owner", hidden: false }, + ], + }, + { kind: "field", fieldId: "2", hidden: false, keys: [] }, + { kind: "source", hidden: false }, +]; + +describe("reconcileColumns", () => { + it("gives a dataset with no stored columns today's layout", () => { + expect(reconcileColumns(undefined, FIELDS, SHAPE)).toEqual(DEFAULT); + expect(reconcileColumns([], FIELDS, SHAPE)).toEqual(DEFAULT); + }); + + it("keeps the stored order and hidden flags", () => { + const stored = [ + { kind: "source" as const }, + path(fieldPath("2"), { hidden: true }), + path(fieldPath("1")), + path(fieldPath("1", ["owner"]), { hidden: true }), + path(fieldPath("1", ["title"])), + path(fieldPath("0")), + ]; + expect(reconcileColumns(stored, FIELDS, SHAPE)).toEqual([ + { kind: "source", hidden: false }, + { kind: "field", fieldId: "2", hidden: true, keys: [] }, + { + kind: "field", + fieldId: "1", + hidden: false, + keys: [ + { key: "owner", hidden: true }, + { key: "title", hidden: false }, + ], + }, + { kind: "field", fieldId: "0", hidden: false, keys: [] }, + ]); + }); + + it("drops a field that no longer exists, and any column rooted at it", () => { + const stored = [ + path(fieldPath("9")), + path(fieldPath("9", ["x", "y"]), { added: true }), + ...serializeColumns(DEFAULT), + ]; + expect(reconcileColumns(stored, FIELDS, SHAPE)).toEqual(DEFAULT); + }); + + it("appends a new field at the end, and a new key at the end of its field", () => { + const stored = [ + path(fieldPath("1")), + path(fieldPath("1", ["owner"])), + { kind: "source" as const }, + path(fieldPath("0")), + ]; + expect(reconcileColumns(stored, FIELDS, SHAPE)).toEqual([ + { + kind: "field", + fieldId: "1", + hidden: false, + keys: [ + { key: "owner", hidden: false }, + { key: "title", hidden: false }, + ], + }, + { kind: "source", hidden: false }, + { kind: "field", fieldId: "0", hidden: false, keys: [] }, + { kind: "field", fieldId: "2", hidden: false, keys: [] }, + ]); + }); + + it("drops a key no row holds any more, but keeps an added path", () => { + const deep = fieldPath("1", ["gone", "deeper"]); + const stored = [ + path(fieldPath("1", ["gone"])), + path(fieldPath("2", ["note"]), { added: true }), + path(deep, { added: true, title: "Deep" }), + ]; + const layout = reconcileColumns(stored, FIELDS, SHAPE); + expect(layout.filter(c => c.kind === "added")).toEqual([ + { kind: "added", path: fieldPath("2", ["note"]), hidden: false }, + { kind: "added", path: deep, hidden: false, title: "Deep" }, + ]); + expect(findPath(layout, fieldPath("1", ["gone"]))).toBeUndefined(); + }); + + it("keeps the first of a repeated path", () => { + const stored = [ + path(fieldPath("0"), { hidden: true }), + path(fieldPath("0")), + path(instancePath("t", ["title"]), { added: true }), + path(instancePath("t", ["title"]), { added: true, hidden: true }), + { kind: "source" as const, hidden: true as const }, + { kind: "source" as const }, + ]; + const layout = reconcileColumns(stored, FIELDS, SHAPE); + expect(layout.slice(0, 3)).toEqual([ + { kind: "field", fieldId: "0", hidden: true, keys: [] }, + { kind: "added", path: instancePath("t", ["title"]), hidden: false }, + { kind: "source", hidden: true }, + ]); + expect(layout).toHaveLength(5); + }); + + it("treats an added path the rows now derive as the field's own key", () => { + const stored = [path(fieldPath("1", ["owner"]), { added: true })]; + expect(reconcileColumns(stored, FIELDS, SHAPE)[0]).toEqual({ + kind: "field", + fieldId: "1", + hidden: false, + keys: [ + { key: "owner", hidden: false }, + { key: "title", hidden: false }, + ], + }); + }); + + it("keeps a column of an unknown kind in its place, and writes it back intact", () => { + const future = { kind: "evalResults", run: "r1", hidden: true as const }; + const stored = [path(fieldPath("0")), future, path(fieldPath("2"))]; + const layout = reconcileColumns(stored, FIELDS, SHAPE); + expect(layout[1]).toEqual({ kind: "unknown", spec: future, hidden: true }); + const shown = setHidden(layout, { index: 1 }, false); + expect(serializeColumns(shown)[1]).toEqual({ + kind: "evalResults", + run: "r1", + }); + expect(serializeColumns(layout)[1]).toEqual(future); + }); +}); + +describe("serializeColumns", () => { + it("round-trips through reconcileColumns", () => { + const layout = addColumn( + setHidden(DEFAULT, { index: 1, key: "owner" }, true), + instancePath("seededTask", ["title"]), + ); + const stored = serializeColumns(layout); + expect(stored).toContainEqual({ + kind: "path", + path: instancePath("seededTask", ["title"]), + added: true, + }); + expect(reconcileColumns(stored, FIELDS, SHAPE)).toEqual(layout); + }); +}); + +describe("layout edits", () => { + it("hides a field with its keys, or a key on its own", () => { + const hidden = setHidden(DEFAULT, { index: 1 }, true); + expect(isHidden(hidden, { index: 1 })).toBe(true); + expect(isHidden(hidden, { index: 1, key: "title" })).toBe(true); + const key = setHidden(DEFAULT, { index: 1, key: "title" }, true); + expect(isHidden(key, { index: 1 })).toBe(false); + expect(isHidden(key, { index: 1, key: "title" })).toBe(true); + expect(isHidden(key, { index: 1, key: "owner" })).toBe(false); + }); + + it("moves top-level columns, and keys only within their field", () => { + expect(moveColumn(DEFAULT, 3, 0).map(c => c.kind)).toEqual([ + "source", + "field", + "field", + "field", + ]); + const keys = moveKey(DEFAULT, 1, 1, 0)[1]; + expect(keys.kind === "field" && keys.keys.map(k => k.key)).toEqual([ + "owner", + "title", + ]); + }); + + it("removes only added columns", () => { + const added = addColumn(DEFAULT, instancePath("t", ["title"])); + expect(removeColumn(added, 4)).toEqual(DEFAULT); + expect(removeColumn(DEFAULT, 0)).toBe(DEFAULT); + expect(removeColumn(DEFAULT, 3)).toBe(DEFAULT); + }); + + it("shows a path that already has a column rather than adding it twice", () => { + const hidden = setHidden( + setHidden(DEFAULT, { index: 1 }, true), + { index: 1, key: "owner" }, + true, + ); + const shown = addColumn(hidden, fieldPath("1", ["owner"])); + expect(shown).toHaveLength(DEFAULT.length); + expect(isHidden(shown, { index: 1, key: "owner" })).toBe(false); + }); +}); + +describe("reorderVisible", () => { + it("rearranges the visible items, leaving hidden ones in place", () => { + const list = ["a", "(b)", "c", "(d)", "e"]; + expect( + reorderVisible(list, x => !x.startsWith("("), ["e", "a", "c"]), + ).toEqual(["e", "(b)", "a", "(d)", "c"]); + }); +}); diff --git a/src/client/components/dataset-columns.ts b/src/client/components/dataset-columns.ts new file mode 100644 index 0000000..274a7b2 --- /dev/null +++ b/src/client/components/dataset-columns.ts @@ -0,0 +1,363 @@ +// SPDX-License-Identifier: AGPL-3.0-only +// Copyright (c) 2026 Alexander Corrado + +/** + * A dataset's shared column layout, as the grid and the Columns picker edit + * it: one ordered list of columns, each shown or hidden, with a field's keys + * nested under it. Stored flat as `Dataset.columns` and reconciled with the + * dataset's actual fields and rows on read, the way `parseTraceColumns` + * reconciles the trace list's. Pure. See `specs/datasets.md` §Q.3. + */ + +import { + type ColumnPath, + columnPathId, + type DatasetColumnSpec, + type ForeignColumnSpec, + fieldPath, + isKnownColumn, +} from "../../shared/column-paths"; +import type { DatasetField, DatasetRowsOverview } from "../../shared/types"; + +/** One key of a field, nested under it in the layout. */ +export interface LayoutKey { + key: string; + hidden: boolean; + /** Kept as stored; the grid titles a key by its name. */ + title?: string; +} + +/** One top-level column of the layout. */ +export type LayoutColumn = + /** A field, derived from the schema, with the keys its cells hold. */ + | { + kind: "field"; + fieldId: string; + hidden: boolean; + keys: LayoutKey[]; + /** Kept as stored; the grid titles a field by its name. */ + title?: string; + } + /** A column added from the picker at any other path. */ + | { kind: "added"; path: ColumnPath; hidden: boolean; title?: string } + /** Where the row came from. */ + | { kind: "source"; hidden: boolean } + /** A column of a kind this version doesn't know, kept in its place. */ + | { kind: "unknown"; spec: ForeignColumnSpec; hidden: boolean }; + +/** The layout: top-level columns in order, a field's keys nested under it. */ +export type ColumnLayout = LayoutColumn[]; + +/** + * Makes a stored layout fit what the dataset actually has: + * + * - an entry for a field that no longer exists, or for a field's key no row + * holds any more, is dropped; + * - a field, or a field key, that isn't listed is appended — a field at the + * end, a key at the end of its field — shown, in schema and row order, so + * a dataset with no stored layout at all looks as it always has; + * - a repeated path keeps its first entry; + * - `source` is appended last when the stored list doesn't mention it; + * - an entry of a kind this version doesn't know is kept in its place. + * + * A path entry naming a field or one of its keys is that field's column + * whatever its `added` flag says, so a field's columns are never listed + * twice. Any other path is an added column. + * + * @param stored - `Dataset.columns`, if the dataset has a layout. + * @param fields - The dataset's schema. + * @param shape - The keys each field's cells hold (`DatasetRowsOverview.fields`). + */ +export function reconcileColumns( + stored: readonly DatasetColumnSpec[] | undefined, + fields: readonly DatasetField[], + shape: DatasetRowsOverview["fields"], +): ColumnLayout { + const fieldIds = new Set(fields.map(f => f.id)); + const layout: ColumnLayout = []; + const byField = new Map>(); + const seen = new Set(); + let hasSource = false; + + const fieldEntry = (fieldId: string) => { + let entry = byField.get(fieldId); + if (!entry) { + entry = { kind: "field", fieldId, hidden: false, keys: [] }; + byField.set(fieldId, entry); + layout.push(entry); + } + return entry; + }; + + for (const spec of stored ?? []) { + const hidden = spec.hidden === true; + if (!isKnownColumn(spec)) { + layout.push({ kind: "unknown", spec, hidden }); + continue; + } + if (spec.kind === "source") { + if (hasSource) continue; + hasSource = true; + layout.push({ kind: "source", hidden }); + continue; + } + const { path, title } = spec; + const id = columnPathId(path); + if (seen.has(id)) continue; + const field = "field" in path.root ? path.root.field : undefined; + if (field !== undefined && !fieldIds.has(field)) continue; + const keys = shape[field ?? ""]?.keys ?? []; + if (field !== undefined && path.keys.length === 0) { + seen.add(id); + const entry = fieldEntry(field); + entry.hidden = hidden; + if (title !== undefined) entry.title = title; + } else if ( + field !== undefined && + path.keys.length === 1 && + keys.includes(path.keys[0]) + ) { + // A field's key, derived from the rows — even if it was added from + // the picker before any row held it. + seen.add(id); + fieldEntry(field).keys.push({ + key: path.keys[0], + hidden, + ...(title !== undefined && { title }), + }); + } else if (field !== undefined && path.keys.length === 1 && !spec.added) { + } else { + seen.add(id); + layout.push({ + kind: "added", + path, + hidden, + ...(title !== undefined && { title }), + }); + } + } + + for (const field of fields) { + if (!byField.has(field.id)) fieldEntry(field.id); + const entry = byField.get(field.id); + if (!entry) continue; + const listed = new Set(entry.keys.map(k => k.key)); + for (const key of shape[field.id]?.keys ?? []) { + if (!listed.has(key)) entry.keys.push({ key, hidden: false }); + } + } + if (!hasSource) layout.push({ kind: "source", hidden: false }); + return layout; +} + +const hiddenFlag = (hidden: boolean) => + hidden ? { hidden: true as const } : {}; + +/** + * A layout as it's stored: flat, a field's whole column followed by one entry + * per key, and an unknown column's entry as it was read, with its `hidden` + * flag as now set. + */ +export function serializeColumns(layout: ColumnLayout): DatasetColumnSpec[] { + return layout.flatMap(serializeColumn); +} + +function serializeColumn(column: LayoutColumn): DatasetColumnSpec[] { + switch (column.kind) { + case "field": + return [ + { + kind: "path", + path: fieldPath(column.fieldId), + ...(column.title !== undefined && { title: column.title }), + ...hiddenFlag(column.hidden), + }, + ...column.keys.map( + (k): DatasetColumnSpec => ({ + kind: "path", + path: fieldPath(column.fieldId, [k.key]), + ...(k.title !== undefined && { title: k.title }), + ...hiddenFlag(k.hidden), + }), + ), + ]; + case "added": + return [ + { + kind: "path", + path: column.path, + ...(column.title !== undefined && { title: column.title }), + ...hiddenFlag(column.hidden), + added: true, + }, + ]; + case "source": + return [{ kind: "source", ...hiddenFlag(column.hidden) }]; + case "unknown": { + const { hidden: _, ...rest } = column.spec; + return [{ ...rest, ...hiddenFlag(column.hidden) }]; + } + } +} + +/** Where a column sits in the layout: a top-level column, or a field's key. */ +export type LayoutSlot = { index: number; key?: string }; + +/** The path a top-level column reads, or `undefined` for one that isn't a path. */ +export function layoutColumnPath(column: LayoutColumn): ColumnPath | undefined { + if (column.kind === "field") return fieldPath(column.fieldId); + if (column.kind === "added") return column.path; + return undefined; +} + +/** The path the column at `slot` reads, or `undefined` for one that isn't a path. */ +export function slotPath( + layout: ColumnLayout, + slot: LayoutSlot, +): ColumnPath | undefined { + const column = layout[slot.index]; + if (!column) return undefined; + if (slot.key !== undefined) { + return column.kind === "field" + ? fieldPath(column.fieldId, [slot.key]) + : undefined; + } + return layoutColumnPath(column); +} + +/** + * Hides or shows the column at `slot`. Hiding a field hides all its keys + * with it, since the grid draws none of them; a key hides on its own. + */ +export function setHidden( + layout: ColumnLayout, + slot: LayoutSlot, + hidden: boolean, +): ColumnLayout { + return layout.map((column, i) => { + if (i !== slot.index) return column; + if (slot.key === undefined) return { ...column, hidden }; + if (column.kind !== "field") return column; + return { + ...column, + keys: column.keys.map(k => (k.key === slot.key ? { ...k, hidden } : k)), + }; + }); +} + +/** Whether the column at `slot` is hidden, its field's hiding included. */ +export function isHidden(layout: ColumnLayout, slot: LayoutSlot): boolean { + const column = layout[slot.index]; + if (!column) return false; + if (column.hidden || slot.key === undefined) return column.hidden; + return ( + column.kind === "field" && + !!column.keys.find(k => k.key === slot.key)?.hidden + ); +} + +function move(list: readonly T[], from: number, to: number): T[] { + if ( + from === to || + from < 0 || + from >= list.length || + to < 0 || + to >= list.length + ) { + return [...list]; + } + const next = [...list]; + const [moved] = next.splice(from, 1); + next.splice(to, 0, moved); + return next; +} + +/** Moves the top-level column at `from` to `to`, as dragging its picker row does. */ +export function moveColumn( + layout: ColumnLayout, + from: number, + to: number, +): ColumnLayout { + return move(layout, from, to); +} + +/** Moves a field's key within that field — the only place a key can go. */ +export function moveKey( + layout: ColumnLayout, + index: number, + from: number, + to: number, +): ColumnLayout { + return layout.map((column, i) => + i === index && column.kind === "field" + ? { ...column, keys: move(column.keys, from, to) } + : column, + ); +} + +/** + * Removes an added column. A field's columns, the source and an unknown + * column can only be hidden, so for those this changes nothing. + */ +export function removeColumn( + layout: ColumnLayout, + index: number, +): ColumnLayout { + return layout[index]?.kind === "added" + ? layout.filter((_, i) => i !== index) + : layout; +} + +/** Where a path already has a column in the layout, if it does. */ +export function findPath( + layout: ColumnLayout, + path: ColumnPath, +): LayoutSlot | undefined { + const id = columnPathId(path); + for (const [index, column] of layout.entries()) { + const top = layoutColumnPath(column); + if (top && columnPathId(top) === id) return { index }; + if (column.kind === "field") { + for (const { key } of column.keys) { + if (columnPathId(fieldPath(column.fieldId, [key])) === id) { + return { index, key }; + } + } + } + } + return undefined; +} + +/** + * Adds a column at `path`, appended at the top level and shown. A path that + * already has a column — a field, one of its keys, or one added before — is + * shown where it is instead, so no path is listed twice. + */ +export function addColumn( + layout: ColumnLayout, + path: ColumnPath, +): ColumnLayout { + const existing = findPath(layout, path); + if (existing) { + let next = setHidden(layout, existing, false); + if (existing.key !== undefined) { + next = setHidden(next, { index: existing.index }, false); + } + return next; + } + return [...layout, { kind: "added", path, hidden: false }]; +} + +/** + * The order `visible` items take within `list` once the visible ones are + * rearranged into `order`: each visible item's place is filled by the next + * one in `order`, so hidden items keep their places between them. + */ +export function reorderVisible( + list: readonly T[], + isVisible: (item: T) => boolean, + order: readonly T[], +): T[] { + const queue = [...order]; + return list.map(item => (isVisible(item) ? (queue.shift() ?? item) : item)); +} diff --git a/src/client/components/dataset-grid.test.ts b/src/client/components/dataset-grid.test.ts index d23371f..db4a43b 100644 --- a/src/client/components/dataset-grid.test.ts +++ b/src/client/components/dataset-grid.test.ts @@ -2,17 +2,27 @@ // Copyright (c) 2026 Alexander Corrado import { describe, expect, it, vi } from "vitest"; +import { fieldPath, instancePath } from "../../shared/column-paths"; import type { DatasetField, DatasetRow, ExecutionInput, } from "../../shared/types"; import { + addColumn, + type ColumnLayout, + reconcileColumns, + setHidden, +} from "./dataset-columns"; +import { + addedTitle, applyUpdate, buildColumns, + canMoveColumn, cellEdits, cellView, clearEdits, + columnPath, type DatasetColumn, DEFAULT_LAYOUT, editableCell, @@ -24,7 +34,9 @@ import { layoutStorageKey, layoutWithoutField, menuFieldId, + movedLayout, parseDatasetLayout, + pathLabel, RowPager, serializeDatasetLayout, } from "./dataset-grid"; @@ -77,7 +89,7 @@ const row = ( function columns(expanded: string[] = []) { return buildColumns({ fields: FIELDS, - shape: SHAPE, + layout: reconcileColumns(undefined, FIELDS, SHAPE), expanded: new Set(expanded), shortType: s => s, }); @@ -277,7 +289,7 @@ describe("editableCell", () => { const whole = (f: DatasetField) => buildColumns({ fields: [f], - shape: {}, + layout: reconcileColumns(undefined, [f], {}), expanded: new Set(), shortType: s => s, })[0]; @@ -377,7 +389,7 @@ describe("grid edits", () => { ]; const cols = buildColumns({ fields: FLAGS, - shape: {}, + layout: reconcileColumns(undefined, FLAGS, {}), expanded: new Set(), shortType: s => s, }); @@ -668,3 +680,245 @@ describe("dataset layout storage", () => { ).toEqual({ expanded: new Set(["1", "2"]), widths: { a: 100 } }); }); }); + +describe("columns from a layout (§Q.3)", () => { + const planTask: DatasetRow = row( + { + "0": text("t-1"), + "1": { kind: "instance", name: "seededTask", output: "taskId" }, + }, + { + id: "p", + resources: { + seededTask: { + uri: "file:///tasks.ts#seededTask", + args: { + title: text("Web site"), + count: { + kind: "value", + value: { kind: "primitive", value: 3 }, + }, + note: { + kind: "value", + value: { kind: "template", value: ["Hi ", { expr: "x" }] }, + } as ExecutionInput, + }, + }, + }, + }, + ); + + const titled = addColumn( + reconcileColumns(undefined, FIELDS, SHAPE), + fieldPath("1", ["title"]), + ); + const layout = addColumn( + addColumn(titled, instancePath("seededTask", ["title"])), + fieldPath("1", ["nested", "deep"]), + ); + const build = (l: ColumnLayout, expanded: string[] = []) => + buildColumns({ + fields: FIELDS, + layout: l, + expanded: new Set(expanded), + shortType: s => s, + }); + + it("lays out added columns after the fields, titled by their last key", () => { + const cols = build(layout); + expect(cols.map(c => [c.id, c.title, c.role])).toEqual([ + ["0", "ticket", "whole"], + ["1", "task", "whole"], + ["2", "info", "whole"], + ["source", "source", "source"], + ["@seededTask/title", "title", "added"], + ["1/nested/deep", "deep", "added"], + ]); + }); + + it("leaves hidden columns out, and a field's hidden keys out of its group", () => { + const hidden = setHidden( + setHidden(layout, { index: 0 }, true), + { index: 1, key: "owner" }, + true, + ); + expect(build(hidden, ["1"]).map(c => c.id)).toEqual([ + "1/title", + "2", + "source", + "@seededTask/title", + "1/nested/deep", + ]); + }); + + it("shows a column of an unknown kind as a dimmed placeholder", () => { + const withFuture = reconcileColumns( + [{ kind: "evalResults" }, { kind: "path", path: fieldPath("0") }], + FIELDS, + SHAPE, + ); + const [unknown] = build(withFuture); + expect(unknown).toMatchObject({ + title: "Unknown column", + type: "evalResults", + role: "unknown", + }); + expect(cellView(planTask, unknown)).toEqual({ kind: "unknown" }); + expect(editableCell(planTask, unknown)).toBeUndefined(); + expect(columnPath(unknown)).toBeUndefined(); + }); + + it("reads an added column through references", () => { + const cols = build(layout); + expect(cellView(planTask, byId(cols, "@seededTask/title"))).toMatchObject({ + kind: "edit", + base: "string", + value: "Web site", + }); + expect(cellView(planTask, byId(cols, "1/nested/deep"))).toEqual({ + kind: "n/a", + }); + }); + + it("names a path for a reader", () => { + expect(pathLabel(fieldPath("1", ["create_task", "title"]), FIELDS)).toBe( + "task › create_task › title", + ); + expect(pathLabel(instancePath("t", ["title"]), FIELDS)).toBe("◆ t › title"); + expect(addedTitle(instancePath("t"), FIELDS)).toBe("t"); + expect(addedTitle(fieldPath("2"), FIELDS)).toBe("info"); + }); + + describe("editing through an added column (§Q.4)", () => { + const added = (p: ReturnType) => + build(addColumn(reconcileColumns(undefined, FIELDS, {}), p)).at(-1); + + it("edits a primitive instance argument reached directly or through a reference", () => { + const spec = planTask.resources?.seededTask; + for (const p of [ + instancePath("seededTask", ["title"]), + fieldPath("1", ["title"]), + ]) { + expect(editableCell(planTask, added(p))).toEqual({ + base: "string", + instance: "seededTask", + arg: "title", + spec, + value: "Web site", + }); + } + expect( + editableCell(planTask, added(instancePath("seededTask", ["count"]))), + ).toMatchObject({ base: "number", value: 3 }); + }); + + it("leaves a template argument, a field's key, and the instance itself read-only", () => { + for (const p of [ + instancePath("seededTask", ["note"]), + instancePath("seededTask"), + instancePath("seededTask", ["missing"]), + ]) { + expect(editableCell(planTask, added(p))).toBeUndefined(); + } + const keyed = build(reconcileColumns(undefined, FIELDS, SHAPE), ["1"]); + expect(editableCell(planTask, byId(keyed, "1/title"))).toBeUndefined(); + }); + + it("sends an argument edit as the whole instance with the argument changed", () => { + const cols = [added(instancePath("seededTask", ["title"]))].filter( + c => c !== undefined, + ); + const edits = cellEdits( + [{ col: 0, row: 0, value: "Todo app" }], + cols, + () => planTask, + ); + const [update] = groupEdits(edits); + expect(update.cells).toEqual({}); + expect(update.resources?.seededTask?.args?.title).toEqual( + text("Todo app"), + ); + expect(update.resources?.seededTask?.args?.count).toEqual( + planTask.resources?.seededTask.args?.count, + ); + expect(applyUpdate(planTask, update).resources?.seededTask).toEqual( + update.resources?.seededTask, + ); + + // Two arguments of one instance in one batch both land; clearing removes. + const spec = planTask.resources?.seededTask; + if (!spec) throw new Error("no spec"); + const [both] = groupEdits([ + { + rowId: "p", + instance: "seededTask", + arg: "title", + spec, + cell: text("A"), + }, + { rowId: "p", instance: "seededTask", arg: "count", spec, cell: null }, + ]); + expect(both.resources?.seededTask.args).toEqual({ + title: text("A"), + note: spec.args?.note, + }); + }); + }); +}); + +describe("moving columns in the grid (§Q.3)", () => { + const layout = reconcileColumns(undefined, FIELDS, SHAPE); + // ticket | task/title task/owner | info | source + const cols = buildColumns({ + fields: FIELDS, + layout, + expanded: new Set(["1"]), + shortType: s => s, + }); + + it("lets a key move only within its own group", () => { + expect(canMoveColumn(cols, 1, 2)).toBe(true); + expect(canMoveColumn(cols, 1, 0)).toBe(false); + expect(canMoveColumn(cols, 2, 3)).toBe(false); + }); + + it("keeps any other column out of an expanded group", () => { + expect(canMoveColumn(cols, 0, 1)).toBe(false); + expect(canMoveColumn(cols, 0, 2)).toBe(true); + expect(canMoveColumn(cols, 4, 2)).toBe(false); + expect(canMoveColumn(cols, 4, 1)).toBe(true); + expect(canMoveColumn(cols, 4, 0)).toBe(true); + }); + + it("writes a top-level move to the layout, hidden columns staying put", () => { + const moved = movedLayout(layout, cols, 4, 0); + expect(moved.map(c => c.kind)).toEqual([ + "source", + "field", + "field", + "field", + ]); + const hidden = setHidden(layout, { index: 2 }, true); + const shown = buildColumns({ + fields: FIELDS, + layout: hidden, + expanded: new Set(), + shortType: s => s, + }); + // ticket | task | source → source first; info, hidden, keeps its place. + expect( + movedLayout(hidden, shown, 2, 0).map(c => + c.kind === "field" ? c.fieldId : c.kind, + ), + ).toEqual(["source", "0", "2", "1"]); + }); + + it("writes a key move to its field's keys", () => { + const moved = movedLayout(layout, cols, 2, 1)[1]; + expect(moved.kind === "field" && moved.keys.map(k => k.key)).toEqual([ + "owner", + "title", + ]); + expect(movedLayout(layout, cols, 1, 0)).toBe(layout); + }); +}); diff --git a/src/client/components/dataset-grid.ts b/src/client/components/dataset-grid.ts index e7559f0..4caa0fe 100644 --- a/src/client/components/dataset-grid.ts +++ b/src/client/components/dataset-grid.ts @@ -6,11 +6,13 @@ * it has, what each cell shows, and the paged cache rows are read through. * Pure and grid-agnostic — `DatasetView` maps it onto Glide Data Grid. * - * A column is a *path* into a row: a whole field, or one key inside it — an - * object's property, or a property of a typed-in object value. The keys come - * from {@link DatasetRowsOverview}, found in the rows rather than the schema, - * and merged by name. A path is also what sorting and filtering will name - * when they arrive. + * A column is a {@link ColumnPath} into a row: a whole field, one key inside + * it — an object's property, or a property of a typed-in object value — or + * any other path a column was added at. A field's keys come from + * {@link DatasetRowsOverview}, found in the rows rather than the schema, and + * merged by name. Which columns show, and in what order, is the dataset's + * shared {@link ColumnLayout}. A path is also what sorting and filtering will + * name when they arrive. * * Some cells are typed into in place — see {@link editableCell}. An edit is * reported per cell and sent per row ({@link groupEdits}), and the page it @@ -20,9 +22,11 @@ import { type ColumnPath, + columnPathId, fieldPath, pathField, readColumnPath, + resolveColumnPath, } from "../../shared/column-paths"; import { committedCell, @@ -36,13 +40,22 @@ import type { DatasetRowUpdate, ExecutionInput, PropValue, + ResourceInstanceInput, } from "../../shared/types"; +import { + type ColumnLayout, + type LayoutSlot, + reorderVisible, +} from "./dataset-columns"; import { previewCell, previewPropValue } from "./dataset-preview"; import { DISCLOSURE_COLLAPSED, DISCLOSURE_EXPANDED } from "./grid-sprites"; /** One column of the dataset table. */ export type DatasetColumn = { - /** Stable across layouts — resized widths are remembered by it. */ + /** + * Stable across layouts — resized widths are remembered by it. A path's + * column is keyed by its serialized path ({@link columnPathId}). + */ id: string; title: string; /** Shown after the title, dimmed — the field's type, shortened. */ @@ -58,6 +71,8 @@ export type DatasetColumn = { * as — see {@link editableCell}. Absent for any other column. */ base?: EditableBase; + /** Where the column sits in the {@link ColumnLayout} it was built from. */ + slot: LayoutSlot; } & ( | { /** A field in one column. */ @@ -66,11 +81,18 @@ export type DatasetColumn = { } /** One key inside a field. */ | { role: "key"; path: ColumnPath } + /** A column added from the picker, at any other path. */ + | { role: "added"; path: ColumnPath } /** Where the row came from. */ | { role: "source" } + /** + * A column of a kind this version doesn't know — a newer client's. Drawn + * as a dimmed placeholder, so the layout around it stays where it was. + */ + | { role: "unknown"; kind: string } ); -const WIDTHS = { whole: 240, key: 160, source: 96 }; +const WIDTHS = { whole: 240, key: 160, added: 200, source: 96, unknown: 120 }; /** * A field's group header, which is its name: only fields with keys have one, @@ -82,73 +104,140 @@ export function fieldGroup(field: DatasetField): string { return field.def.name; } +/** The title of a column added at `path`: its last key, or else its root. */ +export function addedTitle( + path: ColumnPath, + fields: readonly DatasetField[], +): string { + const last = path.keys.at(-1); + if (last !== undefined) return last; + if ("instance" in path.root) return path.root.instance; + const { field } = path.root; + return fields.find(f => f.id === field)?.def.name ?? field; +} + +/** A path as a reader names it: `toolsContext › create_task › title`. */ +export function pathLabel( + path: ColumnPath, + fields: readonly DatasetField[], +): string { + const root = + "instance" in path.root + ? `◆ ${path.root.instance}` + : (fields.find(f => f.id === pathField(path))?.def.name ?? + pathField(path) ?? + ""); + return [root, ...path.keys].join(" › "); +} + /** - * The table's columns, left to right: each field — split into its keys when - * `expanded` has it — and then the source. + * The table's columns, left to right, as `layout` orders them, leaving out + * hidden ones: each field — split into its shown keys when `expanded` has it + * — each added column, the source, and a placeholder for any column of a + * kind this version doesn't know. */ export function buildColumns({ fields, - shape, + layout, expanded, shortType, }: { - fields: DatasetField[]; - /** What each field's cells hold — see {@link DatasetRowsOverview.fields}. */ - shape: DatasetRowsOverview["fields"]; + fields: readonly DatasetField[]; + /** + * The reconciled layout ({@link reconcileColumns}), whose fields carry the + * keys their cells hold. + */ + layout: ColumnLayout; /** Ids of the fields shown split into their keys. */ expanded: ReadonlySet; /** Shortens a type's syntax for a header. */ shortType: (syntax: string) => string; }): DatasetColumn[] { + const byId = new Map(fields.map(f => [f.id, f])); const columns: DatasetColumn[] = []; - for (const field of fields) { - const fieldShape = shape[field.id]; - const fieldKeys = fieldShape?.keys ?? []; - const type = shortType(field.def.type.syntax); - const path = fieldPath(field.id); - const base = primitiveBase(field.def.type); - const whole: DatasetColumn = { - id: field.id, - title: field.def.name, - type, - role: "whole", - path, - width: WIDTHS.whole, - ...(base && { base }), - }; - if (fieldKeys.length === 0) { - columns.push(whole); - continue; - } - const group = fieldGroup(field); - if (!expanded.has(field.id)) { - columns.push({ ...whole, group }); - continue; - } - for (const key of fieldKeys) { - columns.push({ - id: `${field.id}/${key}`, - title: key, - group, - role: "key", - path: fieldPath(field.id, [key]), - width: WIDTHS.key, - }); + for (const [index, entry] of layout.entries()) { + if (entry.hidden) continue; + const slot = { index }; + switch (entry.kind) { + case "field": { + const field = byId.get(entry.fieldId); + if (!field) break; + const type = shortType(field.def.type.syntax); + const base = primitiveBase(field.def.type); + const whole: DatasetColumn = { + id: field.id, + title: field.def.name, + type, + role: "whole", + path: fieldPath(field.id), + width: WIDTHS.whole, + slot, + ...(base && { base }), + }; + if (entry.keys.length === 0) { + columns.push(whole); + break; + } + const group = fieldGroup(field); + const shown = entry.keys.filter(k => !k.hidden); + if (!expanded.has(field.id) || shown.length === 0) { + columns.push({ ...whole, group }); + break; + } + for (const { key } of shown) { + const path = fieldPath(field.id, [key]); + columns.push({ + id: columnPathId(path), + title: key, + group, + role: "key", + path, + width: WIDTHS.key, + slot: { index, key }, + }); + } + break; + } + case "added": + columns.push({ + id: columnPathId(entry.path), + title: entry.title ?? addedTitle(entry.path, fields), + role: "added", + path: entry.path, + width: WIDTHS.added, + slot, + }); + break; + case "source": + columns.push({ + id: "source", + title: "source", + role: "source", + width: WIDTHS.source, + slot, + }); + break; + case "unknown": + columns.push({ + id: `?${index}`, + title: "Unknown column", + type: entry.spec.kind, + role: "unknown", + kind: entry.spec.kind, + width: WIDTHS.unknown, + slot, + }); + break; } } - columns.push({ - id: "source", - title: "source", - role: "source", - width: WIDTHS.source, - }); return columns; } /** * The field a column's header menu renames or deletes, or `undefined` for a - * column that has no menu: the source, and the per-key columns of an expanded - * field, which are paths inside a field rather than fields of their own. + * column that isn't a field: the source, an added column, and the per-key + * columns of an expanded field, which are paths inside a field rather than + * fields of their own. */ export function menuFieldId( column: DatasetColumn | undefined, @@ -156,6 +245,103 @@ export function menuFieldId( return column?.role === "whole" ? pathField(column.path) : undefined; } +/** + * The path a column reads, which can name a row — `undefined` for the source + * and an unknown column, which aren't paths. + */ +export function columnPath( + column: DatasetColumn | undefined, +): ColumnPath | undefined { + return column && "path" in column ? column.path : undefined; +} + +function moved(list: readonly T[], from: number, to: number): T[] { + const next = [...list]; + const [item] = next.splice(from, 1); + next.splice(to, 0, item); + return next; +} + +/** + * Whether the grid may move the column at `from` to `to` — Glide's indexes, + * which remove the column and insert it at `to`. A field's keys are always + * columns of that field, drawn under one group header, so: + * + * - a key column can only land inside its own field's group; + * - any other column can't land inside an expanded field's group. + * + * An expanded field can't be dragged as a whole, since Glide moves single + * columns: collapse it first, or use the picker. + */ +export function canMoveColumn( + columns: readonly DatasetColumn[], + from: number, + to: number, +): boolean { + const column = columns[from]; + if (!column || !columns[to]) return false; + if (from === to) return true; + if (column.role === "key") { + const target = columns[to]; + return target.role === "key" && target.slot.index === column.slot.index; + } + // Every expanded field's keys must stay side by side. + const after = moved(columns, from, to); + const ended = new Set(); + let open: number | undefined; + for (const c of after) { + const group = c.role === "key" ? c.slot.index : undefined; + if (group === open) continue; + if (open !== undefined) ended.add(open); + if (group !== undefined && ended.has(group)) return false; + open = group; + } + return true; +} + +/** + * `layout` after the grid moved the column at `from` to `to` (see + * {@link canMoveColumn}): a key reorders within its field, any other column + * among the top-level columns. Hidden columns keep their places between the + * shown ones. + */ +export function movedLayout( + layout: ColumnLayout, + columns: readonly DatasetColumn[], + from: number, + to: number, +): ColumnLayout { + const column = columns[from]; + if (!column || from === to || !canMoveColumn(columns, from, to)) { + return layout; + } + const after = moved(columns, from, to); + const { index } = column.slot; + const entry = layout[index]; + if (column.role === "key" && entry?.kind === "field") { + const order = after.flatMap(c => + c.role === "key" && c.slot.index === index ? [c.slot.key] : [], + ); + const shown = new Set(order); + const keys = reorderVisible( + entry.keys, + k => shown.has(k.key), + order.flatMap(key => entry.keys.filter(k => k.key === key)), + ); + return layout.map((e, i) => (i === index ? { ...entry, keys } : e)); + } + const order: number[] = []; + for (const c of after) { + if (order.at(-1) !== c.slot.index) order.push(c.slot.index); + } + const shown = new Set(order); + return reorderVisible( + layout.map((_, i) => i), + i => shown.has(i), + order, + ).map(i => layout[i]); +} + /** * Which field each group header belongs to. Glide hands its callbacks a * group's name, which is a field's name; two fields sharing a name would @@ -191,29 +377,53 @@ export function groupHeader( }; } +/** + * What an in-place edit changes: a field's cell, or — through an added + * column — one argument of one of the row's resource instances, whose + * current spec the edit is applied to. See `specs/datasets.md` §Q.4. + */ +export type EditTarget = + | { fieldId: string } + | { instance: string; arg: string; spec: ResourceInstanceInput }; + /** A cell the grid types into in place — see {@link editableCell}. */ -export interface EditableCell { +export type EditableCell = EditTarget & { /** The type it's typed into as. */ base: EditableBase; - /** The field it belongs to. */ - fieldId: string; /** What it holds, or `undefined` when it's empty. */ value?: string | number | boolean; +}; + +/** A primitive's base, when it's one the grid can type into. */ +function primitiveEditBase(value: unknown): EditableBase | undefined { + const type = typeof value; + return type === "string" || type === "number" || type === "boolean" + ? type + : undefined; } /** * The cell of `column` in `row` if it's typed into in place, or `undefined` - * when it's read-only in the grid. Editable means: a field's whole column - * (not a key inside it), a field typed `string`, `number`, or `boolean`, and - * a cell that's empty or holds a plain primitive of that type. Anything else - * — a template, say, whose interpolations a text box would flatten — is - * edited in the details pane instead. See `specs/datasets.md` §P.2. + * when it's read-only in the grid. Editable means either: + * + * - a field's whole column (not a key inside it), a field typed `string`, + * `number`, or `boolean`, and a cell that's empty or holds a plain + * primitive of that type (`specs/datasets.md` §P.2); or + * - an added column whose path ends at one of the row's instances' + * arguments — directly, or through references — that holds a plain + * `string`, `number`, or `boolean`, typed in as that (§Q.4). + * + * Anything else — a template, say, whose interpolations a text box would + * flatten, or a value nested inside an argument — is edited in the details + * pane instead. */ export function editableCell( row: DatasetRow | undefined, column: DatasetColumn | undefined, ): EditableCell | undefined { - if (!row || column?.role !== "whole" || !column.base) return undefined; + if (!row || !column) return undefined; + if (column.role === "added") return editableArgument(row, column.path); + if (column.role !== "whole" || !column.base) return undefined; const { base } = column; const fieldId = pathField(column.path); if (fieldId === undefined) return undefined; @@ -227,6 +437,29 @@ export function editableCell( : undefined; } +/** {@link editableCell} for an added column at `path`. */ +function editableArgument( + row: DatasetRow, + path: ColumnPath, +): EditableCell | undefined { + const { value: found, argument } = resolveColumnPath(row, path); + const spec = argument && row.resources?.[argument.instance]; + if (!argument || !spec || typeof found !== "object") return undefined; + if (found.kind !== "value" || found.value.kind !== "primitive") { + return undefined; + } + const { value } = found.value; + const base = primitiveEditBase(value); + if (!base) return undefined; + return { + base, + instance: argument.instance, + arg: argument.arg, + spec, + value: value as string | number | boolean, + }; +} + /** What an in-place editor holds: empty is `undefined` (or `null`, or `""`). */ export type EditorValue = string | number | boolean | null | undefined; @@ -253,12 +486,18 @@ export function editedCell( return committedCell({ kind: "primitive", value: value ?? undefined }); } -/** One cell's edit, located by row and field. */ -export interface CellEdit { +/** One cell's edit, located by row and what it changes. */ +export type CellEdit = EditTarget & { rowId: string; - fieldId: string; /** The new cell, or `null` to clear it. */ cell: ExecutionInput | null; +}; + +/** `e`'s target alone, without its base and value. */ +function targetOf(e: EditableCell): EditTarget { + return "fieldId" in e + ? { fieldId: e.fieldId } + : { instance: e.instance, arg: e.arg, spec: e.spec }; } /** One edited grid cell, as the grid reports it: column and row indexes. */ @@ -285,7 +524,7 @@ export function cellEdits( if (!e) continue; const cell = editedCell(e.base, value); if (cell === undefined) continue; - out.push({ rowId: row.id, fieldId: e.fieldId, cell }); + out.push({ rowId: row.id, ...targetOf(e), cell }); } return out; } @@ -315,7 +554,7 @@ export function clearEdits( for (let col = x; col < x + width; col++) { const e = row && editableCell(row, columns[col]); if (e && e.value !== undefined) { - out.push({ rowId: row.id, fieldId: e.fieldId, cell: null }); + out.push({ rowId: row.id, ...targetOf(e), cell: null }); } } } @@ -326,17 +565,34 @@ export function clearEdits( /** * A batch of cell edits — one keystroke's, a paste's over a range, a fill's — * as one update per row, rows in the order first edited. A cell edited twice - * keeps its last value. + * keeps its last value. An edit to an instance's argument replaces that + * whole instance in the update's `resources`, with the argument set — or + * removed, when cleared — and every other edit to the same instance in the + * batch applied on top. */ export function groupEdits(edits: readonly CellEdit[]): DatasetRowUpdate[] { const byRow = new Map(); - for (const { rowId, fieldId, cell } of edits) { + for (const edit of edits) { + const { rowId, cell } = edit; let update = byRow.get(rowId); if (!update) { update = { rowId, cells: {} }; byRow.set(rowId, update); } - update.cells[fieldId] = cell; + if ("fieldId" in edit) { + update.cells[edit.fieldId] = cell; + continue; + } + const resources = (update.resources ??= {}); + const current = resources[edit.instance] ?? edit.spec; + const { args: _, ...spec } = current; + const args = { ...current.args }; + if (cell === null) delete args[edit.arg]; + else args[edit.arg] = cell; + resources[edit.instance] = { + ...spec, + ...(Object.keys(args).length > 0 && { args }), + }; } return [...byRow.values()]; } @@ -378,6 +634,8 @@ export type CellView = | { kind: "text"; text: string; tone?: "dim" | "link" } /** A resource instance, drawn as a chip. */ | { kind: "chip"; text: string } + /** A cell of a column this version doesn't know: empty and dimmed. */ + | { kind: "unknown" } /** * A cell typed into in place (see {@link editableCell}): `value` is what * its editor starts from, `text` what's drawn — `—` when it's empty. @@ -445,6 +703,8 @@ export function cellView( : row.source?.kind === "playground" ? { kind: "text", text: "playground", tone: "dim" } : { kind: "empty" }; + case "unknown": + return { kind: "unknown" }; default: { const found = readColumnPath(row, column.path); const e = editableCell(row, column); From b4cf071c32303ff1212111047d3c1e0f9cd98bd5 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 15:48:24 +0000 Subject: [PATCH 4/8] Add the Columns picker, header drag, row label and argument edits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dataset view gains a Columns button whose popover shows, hides, reorders and removes columns, discloses a field's keys, adds a column from a tree of the rows' paths, and sets the row label. Headers can be dragged, with moves that would split a field's keys vetoed, and their menu gains Hide column and Use as row label. The details pane names a row by its label, and an added column ending at an instance's argument is typed into in place (specs/datasets.md §Q.3, §Q.4). Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01QMiMTviyLp3ciCvQf3mzkf --- src/client/components/DatasetColumnMenu.tsx | 79 ++- src/client/components/DatasetColumnPicker.tsx | 505 ++++++++++++++++++ src/client/components/DatasetView.tsx | 161 +++++- src/client/components/TraceList.tsx | 42 +- .../__fixtures__/datasetView.pw.tsx | 291 ++++++++++ src/client/components/dataset-grid.test.ts | 20 +- src/client/components/dataset-grid.ts | 26 +- src/client/components/trace/icons.tsx | 41 ++ src/client/styles.css | 134 +++++ src/index.ts | 8 + 10 files changed, 1230 insertions(+), 77 deletions(-) create mode 100644 src/client/components/DatasetColumnPicker.tsx diff --git a/src/client/components/DatasetColumnMenu.tsx b/src/client/components/DatasetColumnMenu.tsx index 95c5ea2..4107ccd 100644 --- a/src/client/components/DatasetColumnMenu.tsx +++ b/src/client/components/DatasetColumnMenu.tsx @@ -8,8 +8,11 @@ import type { DatasetField } from "../../shared/types"; import { useAnchoredPopover } from "./use-anchored-popover"; interface Props { - /** The field the menu is for. */ - field: DatasetField; + /** + * The field the column is, when it's a field's whole column — which is + * what offers renaming and deleting. + */ + field?: DatasetField; /** The column header's place on screen, which the menu opens below. */ anchor: Rectangle; /** @@ -19,12 +22,20 @@ interface Props { onRename: (name: string) => Promise; /** The field is to be deleted; the caller confirms, then closes the menu. */ onDelete: () => void; + /** Hides the column. */ + onHide: () => void; + /** + * Whether the column is the row label, and makes it so or stops it being + * so — absent for a column that can't name a row (the source, say). + */ + rowLabel?: { isLabel: boolean; toggle: () => void }; /** Called when the menu is dismissed or has done its job. */ onClose: () => void; } /** - * The menu a dataset grid's column header opens: rename the field, or delete + * The menu a dataset grid's column header opens: hide the column, make it + * the row label, and — for a field's own column — rename the field or delete * it. Renaming swaps the menu for a name box in place. Placed against `anchor` * — the header cell's rectangle, which Glide reports but draws on a canvas — * through an invisible fixed element standing in for a trigger. @@ -34,10 +45,12 @@ export function DatasetColumnMenu({ anchor, onRename, onDelete, + onHide, + rowLabel, onClose, }: Props) { const [renaming, setRenaming] = useState(false); - const [name, setName] = useState(field.def.name); + const [name, setName] = useState(field?.def.name ?? ""); const [error, setError] = useState(null); const [busy, setBusy] = useState(false); const { triggerRef, popoverRef, style } = useAnchoredPopover( @@ -47,7 +60,7 @@ export function DatasetColumnMenu({ const submit = async () => { const trimmed = name.trim(); if (busy) return; - if (!trimmed || trimmed === field.def.name) return onClose(); + if (!trimmed || trimmed === field?.def.name) return onClose(); setBusy(true); try { await onRename(trimmed); @@ -125,18 +138,54 @@ export function DatasetColumnMenu({ type="button" role="menuitem" className="trace-header-menu-item" - onClick={() => setRenaming(true)} - > - Rename field… - - + {rowLabel && ( + + )} + {field && ( + <> + + + + )} )} , diff --git a/src/client/components/DatasetColumnPicker.tsx b/src/client/components/DatasetColumnPicker.tsx new file mode 100644 index 0000000..2298dee --- /dev/null +++ b/src/client/components/DatasetColumnPicker.tsx @@ -0,0 +1,505 @@ +// SPDX-License-Identifier: AGPL-3.0-only +// Copyright (c) 2026 Alexander Corrado + +import { useState } from "react"; +import { createPortal } from "react-dom"; +import { + type ColumnPath, + columnPathId, + fieldPath, + instancePath, +} from "../../shared/column-paths"; +import type { + DatasetField, + DatasetPathChild, + DatasetRowsOverview, +} from "../../shared/types"; +import { + type ColumnLayout, + type LayoutColumn, + type LayoutSlot, + moveColumn, + moveKey, + removeColumn, + setHidden, + slotPath, +} from "./dataset-columns"; +import { addedTitle, pathLabel } from "./dataset-grid"; +import { previewCell } from "./dataset-preview"; +import { ColumnsIcon, GrabHandleIcon } from "./trace/icons.tsx"; +import { useAnchoredPopover } from "./use-anchored-popover"; + +interface Props { + /** The reconciled layout ({@link reconcileColumns}). */ + layout: ColumnLayout; + fields: readonly DatasetField[]; + /** The rows' resource instances, offered as roots by "Add column…". */ + instances: DatasetRowsOverview["instances"]; + /** The path that names a row, if any. */ + rowLabel: ColumnPath | undefined; + /** Writes a changed layout. A drag writes once, on drop. */ + onChange: (layout: ColumnLayout) => void; + /** Sets the row label, or clears it with `undefined`. */ + onRowLabel: (path: ColumnPath | undefined) => void; + /** Adds a column at `path`. */ + onAdd: (path: ColumnPath) => void; + /** The keys one level inside a path, for "Add column…"'s tree. */ + describePath: (path: ColumnPath) => Promise; +} + +/** A slot as one string, to compare and key rows by. */ +const slotKey = (slot: LayoutSlot) => + slot.key === undefined ? `${slot.index}` : `${slot.index}/${slot.key}`; + +/** + * The dataset view's "Columns" button and its popover: one row per column, + * in the layout's order, each with a grab handle, a checkbox that hides or + * shows it, and its title. A field with keys discloses them as indented + * rows, which reorder only within that field; an added column's row has a + * ✕ that removes it; and each path's overflow menu sets the row label. + * "Add column…" at the bottom browses the rows' paths as a tree. Reordering + * is a live swap off the grab handle, as the trace list's picker does, but + * written once, when the drag ends. See `specs/datasets.md` §Q.3. + */ +export function DatasetColumnPicker({ + layout, + fields, + instances, + rowLabel, + onChange, + onRowLabel, + onAdd, + describePath, +}: Props) { + const [open, setOpen] = useState(false); + const [adding, setAdding] = useState(false); + /** The layout as a drag in progress has it; written when the drag ends. */ + const [draft, setDraft] = useState(null); + const [drag, setDrag] = useState(null); + /** Fields whose keys are disclosed, by id. */ + const [disclosed, setDisclosed] = useState>(new Set()); + /** The row whose overflow menu is open. */ + const [menuFor, setMenuFor] = useState(null); + const close = () => { + setOpen(false); + setAdding(false); + setMenuFor(null); + }; + const { triggerRef, popoverRef, style } = + useAnchoredPopover({ + open, + onClose: close, + matchTriggerWidth: false, + }); + + const shown = draft ?? layout; + const labelId = rowLabel && columnPathId(rowLabel); + const fieldName = (id: string) => + fields.find(f => f.id === id)?.def.name ?? id; + + const dragOver = (target: LayoutSlot) => { + if (!drag || !draft) return false; + if (drag.key === undefined && target.key === undefined) { + if (target.index !== drag.index) { + setDraft(moveColumn(draft, drag.index, target.index)); + setDrag({ index: target.index }); + } + return true; + } + // A key moves only within its own field. + const entry = draft[drag.index]; + if ( + drag.key === undefined || + target.key === undefined || + target.index !== drag.index || + entry?.kind !== "field" + ) { + return false; + } + if (target.key !== drag.key) { + const from = entry.keys.findIndex(k => k.key === drag.key); + const to = entry.keys.findIndex(k => k.key === target.key); + setDraft(moveKey(draft, drag.index, from, to)); + } + return true; + }; + + const endDrag = () => { + if (draft && draft !== layout) onChange(draft); + setDraft(null); + setDrag(null); + }; + + const pickerRow = ({ + slot, + title, + detail, + hidden, + removable, + disclosure, + dim, + }: { + slot: LayoutSlot; + title: string; + /** Dimmed beside the title. */ + detail?: string; + hidden: boolean; + removable?: boolean; + disclosure?: { open: boolean; toggle: () => void }; + dim?: boolean; + }) => { + const key = slotKey(slot); + const path = slotPath(shown, slot); + const isLabel = !!path && columnPathId(path) === labelId; + return ( +
{ + if (dragOver(slot)) e.preventDefault(); + }} + onDrop={e => e.preventDefault()} + > + { + e.dataTransfer.effectAllowed = "move"; + e.dataTransfer.setData("text/plain", key); + setDraft(layout); + setDrag(slot); + setMenuFor(null); + }} + onDragEnd={endDrag} + title="Drag to reorder" + > + + + {disclosure ? ( + + ) : ( + slot.key === undefined && ( + + ) + )} + + {path && ( + + + {menuFor === key && ( +
+ +
+ )} + + )} + {removable && ( + + )} +
+ ); + }; + + const topRow = (column: LayoutColumn, index: number) => { + const slot = { index }; + switch (column.kind) { + case "field": { + const isOpen = disclosed.has(column.fieldId); + const row = pickerRow({ + slot, + title: fieldName(column.fieldId), + hidden: column.hidden, + ...(column.keys.length > 0 && { + disclosure: { + open: isOpen, + toggle: () => { + const next = new Set(disclosed); + if (!next.delete(column.fieldId)) next.add(column.fieldId); + setDisclosed(next); + }, + }, + }), + }); + if (!isOpen) return row; + return [ + row, + ...column.keys.map(k => + pickerRow({ + slot: { index, key: k.key }, + title: k.key, + hidden: k.hidden, + }), + ), + ]; + } + case "added": + return pickerRow({ + slot, + title: column.title ?? addedTitle(column.path, fields), + detail: pathLabel(column.path, fields), + hidden: column.hidden, + removable: true, + }); + case "source": + return pickerRow({ slot, title: "source", hidden: column.hidden }); + case "unknown": + return pickerRow({ + slot, + title: "Unknown column", + detail: column.spec.kind, + hidden: column.hidden, + dim: true, + }); + } + }; + + return ( + <> + + {open && + createPortal( +
+ {adding ? ( + { + onAdd(path); + setAdding(false); + }} + onBack={() => setAdding(false)} + /> + ) : ( + <> + {shown.flatMap((column, i) => topRow(column, i))} + + + )} +
, + document.body, + )} + + ); +} + +/** + * "Add column…": the rows' paths as a tree — the fields, then the resource + * instances — each node expanding a level at a time, with a sample of what + * the first row holds there dimmed beside it. Picking a node adds a column + * at its path. See `specs/datasets.md` §Q.2. + */ +function PathTree({ + fields, + instances, + describePath, + onPick, + onBack, +}: { + fields: readonly DatasetField[]; + instances: DatasetRowsOverview["instances"]; + describePath: (path: ColumnPath) => Promise; + onPick: (path: ColumnPath) => void; + onBack: () => void; +}) { + return ( +
+ + {fields.map(f => ( + + ))} + {Object.entries(instances).map(([name, shape]) => ( + + ))} +
+ ); +} + +/** One node of {@link PathTree}, fetching its children when first expanded. */ +function PathNode({ + path, + label, + detail, + depth, + describePath, + onPick, +}: { + path: ColumnPath; + label: string; + detail?: string; + depth: number; + describePath: (path: ColumnPath) => Promise; + onPick: (path: ColumnPath) => void; +}) { + const [expanded, setExpanded] = useState(false); + const [children, setChildren] = useState(null); + const [error, setError] = useState(null); + + const toggle = () => { + const next = !expanded; + setExpanded(next); + if (next && children === null) { + describePath(path).then(setChildren, (err: Error) => + setError(err.message), + ); + } + }; + + return ( +
+
+ + +
+ {expanded && ( +
+ {error &&
{error}
} + {children === null && !error && ( +
+ Loading… +
+ )} + {children?.map(child => ( + + ))} +
+ )} +
+ ); +} diff --git a/src/client/components/DatasetView.tsx b/src/client/components/DatasetView.tsx index 883de75..ef36da9 100644 --- a/src/client/components/DatasetView.tsx +++ b/src/client/components/DatasetView.tsx @@ -38,7 +38,11 @@ import { } from "react"; import { createPortal } from "react-dom"; import { shortSyntax } from "ts-proppy/react"; -import { pathField } from "../../shared/column-paths"; +import { + type ColumnPath, + columnPathId, + pathField, +} from "../../shared/column-paths"; import type { Dataset, DatasetField, @@ -53,24 +57,36 @@ import { deleteDataset, deleteDatasetField, deleteDatasetRow, + describeDatasetPath, getDataset, getDatasetRows, renameDataset, renameDatasetField, + setDatasetColumns, updateDatasetRows, } from "../api"; import { useStructurallyStable } from "../hooks/useStructurallyStable"; import { DatasetAddField } from "./DatasetAddField"; import { DatasetColumnMenu } from "./DatasetColumnMenu"; +import { DatasetColumnPicker } from "./DatasetColumnPicker"; import { DatasetRowDetails } from "./DatasetRowDetails"; import { DetailsPane, DetailsPaneHeader, useIsWide } from "./DetailsPane"; +import { + addColumn, + type ColumnLayout, + reconcileColumns, + serializeColumns, + setHidden, +} from "./dataset-columns"; import { buildColumns, type CellEdit, type CellView, + canMoveColumn, cellEdits, cellView, clearEdits, + columnPath, type DatasetColumn, type DatasetLayout, DEFAULT_LAYOUT, @@ -84,8 +100,10 @@ import { layoutStorageKey, layoutWithoutField, menuFieldId, + movedLayout, parseDatasetLayout, RowPager, + rowLabelText, serializeDatasetLayout, } from "./dataset-grid"; import { datasetInputSources } from "./dataset-row-editing"; @@ -308,6 +326,13 @@ function toGridCell(view: CellView, theme: Partial): GridCell { bgCell: theme.bgCellMedium, }, }); + case "unknown": + return text("", { + themeOverride: { + textDark: theme.textLight, + bgCell: theme.bgCellMedium, + }, + }); case "chip": return { kind: GridCellKind.Bubble, @@ -412,9 +437,9 @@ function DatasetView({ onClose: closeMenu, matchTriggerWidth: false, }); - /** The column header menu that's open: its field, and where it opened from. */ + /** The column header menu that's open: its column, and where it opened from. */ const [columnMenu, setColumnMenu] = useState<{ - fieldId: string; + columnId: string; anchor: Rectangle; } | null>(null); const closeColumnMenu = useCallback(() => setColumnMenu(null), []); @@ -451,7 +476,7 @@ function DatasetView({ setOverview({ rowCount: rowCount + pendingAppends.current.count, fields, - instances, + instances: instances ?? {}, }); setError(appendError.current); appendError.current = null; @@ -473,17 +498,53 @@ function DatasetView({ dataset ? datasetInputSources(dataset.fields, linkedPrompt) : undefined, ); + /** The dataset's shared column layout, fitted to its fields and rows. */ + const columnLayout = useMemo( + () => + dataset + ? reconcileColumns(dataset.columns, dataset.fields, overview.fields) + : [], + [dataset, overview.fields], + ); const columns = useMemo( () => dataset ? buildColumns({ fields: dataset.fields, - shape: overview.fields, + layout: columnLayout, expanded, shortType: syntax => shortSyntax(syntax, 24), }) : [], - [dataset, overview.fields, expanded], + [dataset, columnLayout, expanded], + ); + const rowLabel = dataset?.rowLabel; + + /** + * Writes the shared layout and row label. Optimistic, as a cell edit is: + * the grid redraws at once, and the `dataset-changed` refetch confirms. A + * failure says why and reloads the dataset as the server has it. + */ + const saveColumns = useCallback( + (next: ColumnLayout, label: ColumnPath | undefined = rowLabel) => { + const serialized = serializeColumns(next); + setDataset(d => { + if (!d) return d; + const { rowLabel: _, ...rest } = d; + return { + ...rest, + columns: serialized, + ...(label && { rowLabel: label }), + }; + }); + setDatasetColumns(providerId, datasetId, serialized, label ?? null).catch( + (err: Error) => { + setError(`Couldn't save the columns: ${err.message}`); + setReload(n => n + 1); + }, + ); + }, + [providerId, datasetId, rowLabel], ); const fieldIdByGroup = useMemo(() => fieldIdsByGroup(columns), [columns]); const getGroupDetails = useCallback( @@ -501,10 +562,13 @@ function DatasetView({ id: c.id, title: c.title, width: widths[c.id] ?? c.width, - hasMenu: menuFieldId(c) !== undefined, + hasMenu: true, ...(c.group !== undefined && { group: c.group }), + ...(c.role === "unknown" && { + themeOverride: { textHeader: theme.textLight }, + }), })), - [columns, widths], + [columns, widths, theme], ); // biome-ignore lint/correctness/useExhaustiveDependencies: `loaded` redraws cells as their pages land. @@ -786,12 +850,27 @@ function DatasetView({ const onHeaderMenuClick = useCallback( (col: number, anchor: Rectangle) => { - const fieldId = menuFieldId(columns[col]); - if (fieldId !== undefined) setColumnMenu({ fieldId, anchor }); + const column = columns[col]; + if (column) setColumnMenu({ columnId: column.id, anchor }); }, [columns], ); + /** Refuses a header drag that would split a field's keys up (§Q.3). */ + const onColumnProposeMove = useCallback( + (from: number, to: number) => canMoveColumn(columns, from, to), + [columns], + ); + + /** A header was dragged to a new place: one write, on drop. */ + const onColumnMoved = useCallback( + (from: number, to: number) => { + const next = movedLayout(columnLayout, columns, from, to); + if (next !== columnLayout) saveColumns(next); + }, + [columnLayout, columns, saveColumns], + ); + const act = async (action: () => Promise) => { try { await action(); @@ -829,9 +908,16 @@ function DatasetView({ setRenaming(true); }; - const menuField = columnMenu - ? dataset.fields.find(f => f.id === columnMenu.fieldId) + const menuColumn = columnMenu + ? columns.find(c => c.id === columnMenu.columnId) : undefined; + const menuFieldIdValue = menuFieldId(menuColumn); + const menuField = + menuFieldIdValue !== undefined + ? dataset.fields.find(f => f.id === menuFieldIdValue) + : undefined; + const menuPath = columnPath(menuColumn); + const labelId = rowLabel && columnPathId(rowLabel); const renameField = async (field: DatasetField, name: string) => { const renamed = await renameDatasetField( @@ -926,10 +1012,20 @@ function DatasetView({ }), }); + const selectedLabel = + selectedRow && rowLabel ? rowLabelText(selectedRow, rowLabel) : undefined; + const rowDetails = selectedRow && selectedIndex !== null && ( <> Row {selectedIndex + 1}} + title={ + + Row {selectedIndex + 1} + {selectedLabel !== undefined && ( + · {selectedLabel} + )} + + } actions={ <> {linkedPrompt && ( @@ -1009,6 +1105,18 @@ function DatasetView({ )}
+ saveColumns(next)} + onRowLabel={path => saveColumns(columnLayout, path)} + onAdd={path => saveColumns(addColumn(columnLayout, path))} + describePath={path => + describeDatasetPath(providerId, datasetId, path) + } + />
{onAskAgent && ( @@ -1077,13 +1185,28 @@ function DatasetView({
{menu} - {columnMenu && menuField && ( + {columnMenu && menuColumn && ( renameField(menuField, name)} - onDelete={() => handleDeleteField(menuField)} + onRename={async name => { + if (menuField) await renameField(menuField, name); + }} + onDelete={() => menuField && handleDeleteField(menuField)} + onHide={() => + saveColumns(setHidden(columnLayout, menuColumn.slot, true)) + } + {...(menuPath && { + rowLabel: { + isLabel: columnPathId(menuPath) === labelId, + toggle: () => + saveColumns( + columnLayout, + columnPathId(menuPath) === labelId ? undefined : menuPath, + ), + }, + })} onClose={closeColumnMenu} /> )} @@ -1150,6 +1273,8 @@ function DatasetView({ onCellActivated={onCellActivated} onGroupHeaderClicked={onGroupHeaderClicked} onHeaderMenuClick={onHeaderMenuClick} + onColumnProposeMove={onColumnProposeMove} + onColumnMoved={onColumnMoved} onCellsEdited={onCellsEdited} validateCell={validateCell} onDelete={onDelete} diff --git a/src/client/components/TraceList.tsx b/src/client/components/TraceList.tsx index ee1f7a8..24a1c54 100644 --- a/src/client/components/TraceList.tsx +++ b/src/client/components/TraceList.tsx @@ -24,7 +24,9 @@ import { import { AnnotationsIcon, CalendarIcon, + ColumnsIcon, CostIcon, + GrabHandleIcon, ModelIcon, SpansIcon, StopwatchIcon, @@ -118,46 +120,6 @@ function AnnotationCountBadges({ ); } -function ColumnsIcon() { - return ( - - - - - - ); -} - -/** A 2×3 grid of dots — the column picker's drag-to-reorder grab handle. */ -function GrabHandleIcon() { - return ( - - - - - - - - - ); -} - /** * Trigger + popover for showing/hiding and reordering {@link TraceColumnState}s. * Reordering is native HTML5 drag-and-drop off each row's grab handle: the diff --git a/src/client/components/__fixtures__/datasetView.pw.tsx b/src/client/components/__fixtures__/datasetView.pw.tsx index 8f87419..1ad8882 100644 --- a/src/client/components/__fixtures__/datasetView.pw.tsx +++ b/src/client/components/__fixtures__/datasetView.pw.tsx @@ -1486,3 +1486,294 @@ test("clicking the grid closes a column header's menu", async ({ await clickGrid(page, 100, 60); await expect(menu).toHaveCount(0); }); + +// ─── Column layout and row label (specs/datasets.md §Q) ───────────────────── + +/** + * Answers `PATCH …/columns`, returning the body of every request — the + * layouts the view wrote. + */ +async function mockColumns(page: Page, dataset: Dataset) { + const sent: { columns: unknown[]; rowLabel: unknown }[] = []; + await page.route("**/api/datasets/local/tickets/columns", route => { + if (route.request().method() !== "PATCH") return route.fallback(); + const body = route.request().postDataJSON(); + sent.push(body); + return route.fulfill({ json: { ...dataset, ...body } }); + }); + return sent; +} + +/** The grid's column headers, left to right, as its accessible mirror has them. */ +async function headerOrder(scope: Locator) { + const texts = await scope + .getByRole("grid", { includeHidden: true }) + .getByRole("columnheader", { includeHidden: true }) + .allTextContents(); + return texts.filter(t => t !== ""); +} + +/** Drags a column header from `fromX` to `toX`, at height `y`, in steps. */ +async function dragHeader(page: Page, fromX: number, toX: number, y: number) { + const box = await page.getByTestId("data-grid-canvas").boundingBox(); + if (!box) throw new Error("grid canvas not laid out"); + await page.mouse.move(box.x + fromX, box.y + y); + await page.mouse.down(); + for (let i = 1; i <= 8; i++) { + await page.mouse.move(box.x + fromX + ((toX - fromX) * i) / 8, box.y + y); + } + await page.mouse.up(); +} + +const path = (field: string, keys: string[] = []) => ({ + kind: "path" as const, + path: { root: { field }, keys }, +}); + +test("dragging a header moves the column, saving the shared layout once", async ({ + mount, + page, +}) => { + await page.evaluate(() => localStorage.clear()); + await mockDataset(page, SUPPORT, SUPPORT_ROWS, SUPPORT_SHAPE); + const sent = await mockColumns(page, SUPPORT); + const component = await mount( + , + ); + await expect + .poll(() => headerOrder(component)) + .toEqual(["ticket", "task", "source"]); + + // `task` onto `ticket`. + await dragHeader( + page, + GRID.marker + 240 + 60, + GRID.marker + 40, + GRID.header / 2, + ); + + await expect + .poll(() => headerOrder(component)) + .toEqual(["task", "ticket", "source"]); + expect(sent).toEqual([ + { + columns: [path("1"), path("0"), { kind: "source" }], + rowLabel: null, + }, + ]); +}); + +test("a key's header moves within its field's group, and is refused outside it", async ({ + mount, + page, +}) => { + await page.evaluate(() => localStorage.clear()); + const dataset: Dataset = { + ...SUPPORT, + fields: [ + { id: "0", def: TICKET }, + { + id: "1", + def: { + name: "taskInfo", + type: { kind: "object", syntax: "{ title?: string }" }, + optional: false, + } as PropDefinition, + }, + ], + }; + await mockDataset( + page, + dataset, + [{ id: "r1", cells: { "0": text("Hi") }, createdAt: 1 }], + { "1": { keys: ["title", "description"] } }, + ); + const sent = await mockColumns(page, dataset); + const component = await mount( + , + ); + await clickGrid(page, GRID.marker + 240 + 60, GRID.group / 2); + await expect + .poll(() => headerOrder(component)) + .toEqual(["ticket", "title", "description", "source"]); + const y = GRID.group + GRID.header / 2; + + // `title` out onto `ticket`: refused, so nothing moves and nothing is sent. + await dragHeader(page, GRID.marker + 240 + 40, GRID.marker + 40, y); + await page.waitForTimeout(300); + expect(await headerOrder(component)).toEqual([ + "ticket", + "title", + "description", + "source", + ]); + expect(sent).toEqual([]); + + // `description` onto `title`, within the group: moved. + await dragHeader( + page, + GRID.marker + 240 + 160 + 40, + GRID.marker + 240 + 40, + y, + ); + await expect + .poll(() => headerOrder(component)) + .toEqual(["ticket", "description", "title", "source"]); + expect(sent).toEqual([ + { + columns: [ + path("0"), + path("1"), + path("1", ["description"]), + path("1", ["title"]), + { kind: "source" }, + ], + rowLabel: null, + }, + ]); +}); + +test("an added column edits a referenced instance's argument inline, and can name the row", async ({ + mount, + page, +}) => { + await page.setViewportSize({ width: 1200, height: 700 }); + const titlePath = { root: { field: "1" }, keys: ["title"] }; + const dataset: Dataset = { + ...SUPPORT, + columns: [ + path("0"), + path("1"), + { kind: "path", path: titlePath, added: true }, + { kind: "source" }, + ], + rowLabel: titlePath, + }; + await mockDataset(page, dataset, SUPPORT_ROWS, SUPPORT_SHAPE); + const sent = await mockUpdates(page); + const component = await mount( + , + ); + await expect + .poll(() => headerOrder(component)) + .toEqual(["ticket", "task", "title", "source"]); + // `task` › `title` steps through the cell's reference to `seededTask`. + await expect(inGrid(component, "gridcell", "Buy milk")).toBeAttached(); + + await clickCell(page, 240 + 240 + 40, 0, false); + await typeIntoCell(page, "Buy oat milk"); + + await expect + .poll(() => sent) + .toEqual([ + { + updates: [ + { + rowId: "r1", + cells: {}, + resources: { + seededTask: { + uri: ".evalution/playground/tasks.ts#seededTask", + args: { title: text("Buy oat milk") }, + }, + }, + }, + ], + }, + ]); + await expect(inGrid(component, "gridcell", "Buy oat milk")).toBeAttached(); + + // The row label names the row in the details pane; a row with nothing + // there is named by its number alone. + const pane = component.getByRole("region", { name: "Row details" }); + await expect(pane).toContainText("Row 2"); + await expect(pane).not.toContainText("·"); + await clickCell(page, 40, 0, false); + await expect(pane).toContainText("Row 1 · Buy oat milk"); +}); + +test("the Columns picker hides a column, adds one from the path tree, and sets the row label", async ({ + mount, + page, +}) => { + await mockDataset(page, SUPPORT, SUPPORT_ROWS, SUPPORT_SHAPE); + const sent = await mockColumns(page, SUPPORT); + await page.route("**/api/datasets/local/tickets/paths", route => + route.fulfill({ + json: [{ key: "title", sample: text("Buy milk") }], + }), + ); + const component = await mount( + , + ); + await expect.poll(() => headerOrder(component)).toContain("task"); + + await component.getByRole("button", { name: "Columns" }).click(); + const picker = page.getByRole("dialog", { name: "Columns" }); + await picker.getByLabel("Show source").uncheck(); + await expect.poll(() => headerOrder(component)).toEqual(["ticket", "task"]); + expect(sent.at(-1)?.columns).toEqual([ + path("0"), + path("1"), + { kind: "source", hidden: true }, + ]); + + await picker.getByRole("button", { name: "Add column…" }).click(); + await picker.getByRole("button", { name: "Expand task" }).click(); + await expect(picker.getByText('"Buy milk"')).toBeVisible(); + await picker.getByRole("button", { name: /^title/ }).click(); + await expect + .poll(() => headerOrder(component)) + .toEqual(["ticket", "task", "title"]); + expect(sent.at(-1)?.columns).toContainEqual({ + ...path("1", ["title"]), + added: true, + }); + + await picker.getByRole("button", { name: "More for title" }).click(); + await picker.getByRole("button", { name: "Use as row label" }).click(); + expect(sent.at(-1)?.rowLabel).toEqual({ + root: { field: "1" }, + keys: ["title"], + }); + await expect(picker.getByTitle("Row label")).toBeVisible(); + + // An added column can be removed; a field's can only be hidden. + await expect( + picker.getByRole("button", { name: "Remove column task" }), + ).toHaveCount(0); + await picker.getByRole("button", { name: "Remove column title" }).click(); + await expect.poll(() => headerOrder(component)).toEqual(["ticket", "task"]); +}); + +test("a column of an unknown kind keeps its place as a dimmed placeholder", async ({ + mount, + page, +}) => { + const dataset: Dataset = { + ...SUPPORT, + columns: [path("0"), { kind: "evalResults", run: "r9" }, path("1")], + }; + await mockDataset(page, dataset, SUPPORT_ROWS, SUPPORT_SHAPE); + const sent = await mockColumns(page, dataset); + const component = await mount( + , + ); + await expect + .poll(() => headerOrder(component)) + .toEqual(["ticket", "Unknown column", "task", "source"]); + + // Hiding another column writes the unknown one back intact. + await component.getByRole("button", { name: "Columns" }).click(); + const picker = page.getByRole("dialog", { name: "Columns" }); + await expect( + picker.getByRole("button", { name: "Remove column Unknown column" }), + ).toHaveCount(0); + await picker.getByLabel("Show ticket").uncheck(); + expect(sent.at(-1)?.columns).toEqual([ + { ...path("0"), hidden: true }, + { kind: "evalResults", run: "r9" }, + path("1"), + { kind: "source" }, + ]); +}); diff --git a/src/client/components/dataset-grid.test.ts b/src/client/components/dataset-grid.test.ts index db4a43b..0277ce8 100644 --- a/src/client/components/dataset-grid.test.ts +++ b/src/client/components/dataset-grid.test.ts @@ -38,6 +38,7 @@ import { parseDatasetLayout, pathLabel, RowPager, + rowLabelText, serializeDatasetLayout, } from "./dataset-grid"; @@ -858,7 +859,7 @@ describe("columns from a layout (§Q.3)", () => { }, { rowId: "p", instance: "seededTask", arg: "count", spec, cell: null }, ]); - expect(both.resources?.seededTask.args).toEqual({ + expect(both.resources?.seededTask?.args).toEqual({ title: text("A"), note: spec.args?.note, }); @@ -922,3 +923,20 @@ describe("moving columns in the grid (§Q.3)", () => { expect(movedLayout(layout, cols, 1, 0)).toBe(layout); }); }); + +describe("rowLabelText", () => { + const r = row( + { "0": text("Web site"), "1": text(""), "2": info }, + { resources: { t: { uri: "u", args: { n: text("Todo") } } } }, + ); + it("names a row by the label's value, a string unquoted", () => { + expect(rowLabelText(r, fieldPath("0"))).toBe("Web site"); + expect(rowLabelText(r, instancePath("t", ["n"]))).toBe("Todo"); + expect(rowLabelText(r, fieldPath("2", ["title"]))).toBe("Say hi"); + }); + it("names it by nothing when there's nothing there", () => { + expect(rowLabelText(r, fieldPath("1"))).toBeUndefined(); + expect(rowLabelText(r, fieldPath("9"))).toBeUndefined(); + expect(rowLabelText(r, fieldPath("0", ["x"]))).toBeUndefined(); + }); +}); diff --git a/src/client/components/dataset-grid.ts b/src/client/components/dataset-grid.ts index 4caa0fe..c91f8d3 100644 --- a/src/client/components/dataset-grid.ts +++ b/src/client/components/dataset-grid.ts @@ -9,7 +9,7 @@ * A column is a {@link ColumnPath} into a row: a whole field, one key inside * it — an object's property, or a property of a typed-in object value — or * any other path a column was added at. A field's keys come from - * {@link DatasetRowsOverview}, found in the rows rather than the schema, and + * `DatasetRowsOverview`, found in the rows rather than the schema, and * merged by name. Which columns show, and in what order, is the dataset's * shared {@link ColumnLayout}. A path is also what sorting and filtering will * name when they arrive. @@ -36,7 +36,6 @@ import { import type { DatasetField, DatasetRow, - DatasetRowsOverview, DatasetRowUpdate, ExecutionInput, PropValue, @@ -583,7 +582,8 @@ export function groupEdits(edits: readonly CellEdit[]): DatasetRowUpdate[] { update.cells[edit.fieldId] = cell; continue; } - const resources = (update.resources ??= {}); + const resources = update.resources ?? {}; + update.resources = resources; const current = resources[edit.instance] ?? edit.spec; const { args: _, ...spec } = current; const args = { ...current.args }; @@ -723,6 +723,26 @@ export function cellView( } } +/** + * What names `row` when `path` is the dataset's row label: the value there, + * a string as itself rather than quoted, or `undefined` when the row has + * nothing there to name it by. + */ +export function rowLabelText( + row: DatasetRow, + path: ColumnPath, +): string | undefined { + const found = readColumnPath(row, path); + if (typeof found === "string") return undefined; + if (found.kind === "value" && found.value.kind === "primitive") { + const { value } = found.value; + return value === undefined || value === null || value === "" + ? undefined + : String(value); + } + return previewCell(found); +} + /** Fetches `limit` rows starting at `offset`. */ export type FetchRows = ( offset: number, diff --git a/src/client/components/trace/icons.tsx b/src/client/components/trace/icons.tsx index 4b6dfba..53a2091 100644 --- a/src/client/components/trace/icons.tsx +++ b/src/client/components/trace/icons.tsx @@ -504,3 +504,44 @@ export function EvalsIcon({ size = 11 }: { size?: number }) { ); } + +/** Three columns in a box — the column picker's trigger. */ +export function ColumnsIcon() { + return ( + + + + + + ); +} + +/** A 2×3 grid of dots — the column picker's drag-to-reorder grab handle. */ +export function GrabHandleIcon() { + return ( + + + + + + + + + ); +} diff --git a/src/client/styles.css b/src/client/styles.css index 8041b41..3c2a463 100644 --- a/src/client/styles.css +++ b/src/client/styles.css @@ -4110,6 +4110,140 @@ pre { cursor: grabbing; } +/* ─── Dataset column picker (specs/datasets.md §Q.3) ─────────────────────── */ + +.dataset-column-picker { + min-width: 240px; + max-width: 360px; + max-height: min(70vh, 520px); + overflow-y: auto; +} + +.dataset-column-picker-key { + padding-left: 28px; +} + +.dataset-column-picker-dim .trace-column-picker-check { + color: var(--dropdown-text-muted); +} + +.dataset-column-picker-disclosure { + display: inline-flex; + align-items: center; + justify-content: center; + width: 14px; + flex-shrink: 0; + padding: 0; + background: none; + border: none; + color: var(--dropdown-text-muted); + font-size: 10px; + cursor: pointer; +} + +.dataset-column-picker-title { + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + +.dataset-column-picker-detail { + min-width: 0; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + color: var(--dropdown-text-muted); + font-size: 11px; +} + +.dataset-column-picker-badge { + flex-shrink: 0; + padding: 0 5px; + border-radius: 4px; + background: var(--dropdown-option-hover); + color: var(--dropdown-text-muted); + font-size: 10.5px; + line-height: 16px; +} + +.dataset-column-picker-more { + position: relative; + display: inline-flex; +} + +.dataset-column-picker-icon-btn { + flex-shrink: 0; + padding: 0 4px; + background: none; + border: none; + border-radius: 4px; + color: var(--dropdown-text-muted); + font-size: 12px; + line-height: 18px; + cursor: pointer; +} + +.dataset-column-picker-icon-btn:hover { + color: var(--dropdown-text); + background: var(--dropdown-option-hover); +} + +.dataset-column-picker-menu { + position: absolute; + top: 100%; + right: 0; + z-index: 1; + white-space: nowrap; +} + +.dataset-column-picker-add, +.dataset-path-tree-back { + width: 100%; + margin-top: 2px; +} + +.dataset-path-tree-node { + display: flex; + align-items: center; + gap: 4px; + padding: 3px 6px; + border-radius: 6px; +} + +.dataset-path-tree-node:hover { + background: var(--dropdown-option-hover); +} + +.dataset-path-tree-pick { + display: flex; + align-items: baseline; + gap: 6px; + flex: 1; + min-width: 0; + padding: 0; + background: none; + border: none; + color: var(--dropdown-text); + font-size: 12.5px; + text-align: left; + cursor: pointer; +} + +.dataset-path-tree-note { + padding: 3px 6px; + color: var(--dropdown-text-muted); + font-size: 11.5px; +} + +.dataset-columns-btn-active { + color: var(--trace-text); + border-color: var(--trace-text-dim); +} + +.dataset-row-label { + color: var(--trace-text-dim); +} + /* ── Annotation count badges (table/card annotations column) ── */ .trace-annotation-counts { diff --git a/src/index.ts b/src/index.ts index 438fc7d..ae8f27d 100644 --- a/src/index.ts +++ b/src/index.ts @@ -35,6 +35,8 @@ export type { DatasetChangeType, DatasetField, DatasetFieldShape, + DatasetInstanceShape, + DatasetPathChild, DatasetProviderInfo, DatasetRow, DatasetRowSource, @@ -249,6 +251,12 @@ export { VercelAISDKTelemetry, type VercelAISDKTelemetryOptions, } from "./sdk/vercel-ai-sdk/telemetry.ts"; +export type { + ColumnPath, + DatasetColumnSpec, + ForeignColumnSpec, + KnownColumnSpec, +} from "./shared/column-paths.ts"; export { isValidInstanceName } from "./shared/instance-names.ts"; export type { AddPromptContext, From b3ae5cf7cf1a32de475cf7f0277c53c96029e1dc Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 20:51:29 +0000 Subject: [PATCH 5/8] Use fieldEntry directly when appending missing fields fieldEntry already returns a field's existing entry, so the lookup before it was redundant. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01QMiMTviyLp3ciCvQf3mzkf --- src/client/components/dataset-columns.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/client/components/dataset-columns.ts b/src/client/components/dataset-columns.ts index 274a7b2..b7ee286 100644 --- a/src/client/components/dataset-columns.ts +++ b/src/client/components/dataset-columns.ts @@ -126,6 +126,7 @@ export function reconcileColumns( ...(title !== undefined && { title }), }); } else if (field !== undefined && path.keys.length === 1 && !spec.added) { + // A derived key no row holds any more: dropped. } else { seen.add(id); layout.push({ @@ -138,9 +139,7 @@ export function reconcileColumns( } for (const field of fields) { - if (!byField.has(field.id)) fieldEntry(field.id); - const entry = byField.get(field.id); - if (!entry) continue; + const entry = fieldEntry(field.id); const listed = new Set(entry.keys.map(k => k.key)); for (const key of shape[field.id]?.keys ?? []) { if (!listed.has(key)) entry.keys.push({ key, hidden: false }); From 5f3b9b4017a50e115f971f92c122b9926c0286ab Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 21:16:57 +0000 Subject: [PATCH 6/8] Address review: stale instance roots, argument clears, row label - A layout whose instance no row declares any more can still be saved: instance roots the stored layout or row label already has are accepted, and only new ones are looked up in the rows. - Clearing an instance argument in the grid is a no-op rather than deleting it, which left the cell read-only. - Removing the row label's column clears the label in the same write, and "Stop using as row label" now actually clears it (saveColumns' default parameter swallowed the explicit undefined). - parseColumnPath checks instance names; isPathColumn and pathInstance replace casts and inline root checks; reconcileColumns splits on the root first. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01QMiMTviyLp3ciCvQf3mzkf --- specs/datasets.md | 8 ++- src/client/components/DatasetColumnPicker.tsx | 9 ++- src/client/components/DatasetView.tsx | 22 +++++-- .../__fixtures__/datasetView.pw.tsx | 12 +++- src/client/components/dataset-columns.ts | 54 +++++++++--------- src/client/components/dataset-grid.test.ts | 14 +++++ src/client/components/dataset-grid.ts | 26 +++++---- src/dataset/turso-dataset-provider.ts | 11 ++-- src/server/handlers/datasets.test.ts | 39 +++++++++++++ src/server/handlers/datasets.ts | 57 ++++++++++++------- src/shared/column-paths.test.ts | 2 + src/shared/column-paths.ts | 27 +++++++-- 12 files changed, 205 insertions(+), 76 deletions(-) diff --git a/specs/datasets.md b/specs/datasets.md index e85ad49..028f426 100644 --- a/specs/datasets.md +++ b/specs/datasets.md @@ -1021,7 +1021,9 @@ that one `DatasetRowUpdate` can address: - **An instance's argument**, reached directly (`seededTask` › `title`) or through references (`toolsContext` › … › `rootTaskId` › `title`). The edit replaces that instance in the update's `resources` with the argument changed. Editing the focus task's title from the grid is the case - this exists for. + this exists for. Clearing one in place (Delete, or emptying the editor) does nothing: it would + delete the argument, and an argument that isn't there has nothing to type into. The details pane + removes an argument. Anything nested inside an argument's value object stays read-only for now. `resources` updates replace a whole instance, which is fine for a top-level argument but would mean rebuilding a nested @@ -1037,7 +1039,9 @@ value from a path. The details pane already edits those. `dataset-provider-contract.ts`. 3. `columns` and `rowLabel`: the migration, `setColumns`, the route, validation, and `deleteField` pruning its columns. The server checks that a path's root exists in the schema or - in some row, and that `keys` are non-empty strings. + in some row, and that `keys` are non-empty strings. An instance root the stored layout or row + label already has is accepted even when no row declares it any more, so a layout can always be + saved; it reads `"absent"` until a row declares the instance again. 4. `reconcileColumns`, as a pure function next to `parseTraceColumns`'s pattern. Unit tests cover a dropped field, a new field and a new key being appended, a duplicate path, and a stored list with no `columns`, which must produce today's layout. diff --git a/src/client/components/DatasetColumnPicker.tsx b/src/client/components/DatasetColumnPicker.tsx index 2298dee..a069c56 100644 --- a/src/client/components/DatasetColumnPicker.tsx +++ b/src/client/components/DatasetColumnPicker.tsx @@ -20,7 +20,6 @@ import { type LayoutSlot, moveColumn, moveKey, - removeColumn, setHidden, slotPath, } from "./dataset-columns"; @@ -43,6 +42,11 @@ interface Props { onRowLabel: (path: ColumnPath | undefined) => void; /** Adds a column at `path`. */ onAdd: (path: ColumnPath) => void; + /** + * Removes the added column at `index` — and the row label with it, if + * that column was the label, since no menu would be left to clear it. + */ + onRemove: (index: number) => void; /** The keys one level inside a path, for "Add column…"'s tree. */ describePath: (path: ColumnPath) => Promise; } @@ -69,6 +73,7 @@ export function DatasetColumnPicker({ onChange, onRowLabel, onAdd, + onRemove, describePath, }: Props) { const [open, setOpen] = useState(false); @@ -246,7 +251,7 @@ export function DatasetColumnPicker({ className="dataset-column-picker-icon-btn" aria-label={`Remove column ${title}`} title="Remove column" - onClick={() => onChange(removeColumn(layout, slot.index))} + onClick={() => onRemove(slot.index)} > ✕ diff --git a/src/client/components/DatasetView.tsx b/src/client/components/DatasetView.tsx index ef36da9..1309a74 100644 --- a/src/client/components/DatasetView.tsx +++ b/src/client/components/DatasetView.tsx @@ -75,8 +75,10 @@ import { addColumn, type ColumnLayout, reconcileColumns, + removeColumn, serializeColumns, setHidden, + slotPath, } from "./dataset-columns"; import { buildColumns, @@ -521,12 +523,13 @@ function DatasetView({ const rowLabel = dataset?.rowLabel; /** - * Writes the shared layout and row label. Optimistic, as a cell edit is: + * Writes the shared layout and row label (`null` clears the label; left + * out, it stays as it is). Optimistic, as a cell edit is: * the grid redraws at once, and the `dataset-changed` refetch confirms. A * failure says why and reloads the dataset as the server has it. */ const saveColumns = useCallback( - (next: ColumnLayout, label: ColumnPath | undefined = rowLabel) => { + (next: ColumnLayout, label: ColumnPath | null = rowLabel ?? null) => { const serialized = serializeColumns(next); setDataset(d => { if (!d) return d; @@ -537,7 +540,7 @@ function DatasetView({ ...(label && { rowLabel: label }), }; }); - setDatasetColumns(providerId, datasetId, serialized, label ?? null).catch( + setDatasetColumns(providerId, datasetId, serialized, label).catch( (err: Error) => { setError(`Couldn't save the columns: ${err.message}`); setReload(n => n + 1); @@ -1111,8 +1114,17 @@ function DatasetView({ instances={overview.instances} rowLabel={rowLabel} onChange={next => saveColumns(next)} - onRowLabel={path => saveColumns(columnLayout, path)} + onRowLabel={path => saveColumns(columnLayout, path ?? null)} onAdd={path => saveColumns(addColumn(columnLayout, path))} + onRemove={index => { + const path = slotPath(columnLayout, { index }); + saveColumns( + removeColumn(columnLayout, index), + path && columnPathId(path) === labelId + ? null + : (rowLabel ?? null), + ); + }} describePath={path => describeDatasetPath(providerId, datasetId, path) } @@ -1203,7 +1215,7 @@ function DatasetView({ toggle: () => saveColumns( columnLayout, - columnPathId(menuPath) === labelId ? undefined : menuPath, + columnPathId(menuPath) === labelId ? null : menuPath, ), }, })} diff --git a/src/client/components/__fixtures__/datasetView.pw.tsx b/src/client/components/__fixtures__/datasetView.pw.tsx index 1ad8882..fb856b0 100644 --- a/src/client/components/__fixtures__/datasetView.pw.tsx +++ b/src/client/components/__fixtures__/datasetView.pw.tsx @@ -1738,12 +1738,22 @@ test("the Columns picker hides a column, adds one from the path tree, and sets t }); await expect(picker.getByTitle("Row label")).toBeVisible(); - // An added column can be removed; a field's can only be hidden. + await picker.getByRole("button", { name: "More for title" }).click(); + await picker.getByRole("button", { name: "Stop using as row label" }).click(); + expect(sent.at(-1)?.rowLabel).toBeNull(); + await expect(picker.getByTitle("Row label")).toHaveCount(0); + await picker.getByRole("button", { name: "More for title" }).click(); + await picker.getByRole("button", { name: "Use as row label" }).click(); + await expect(picker.getByTitle("Row label")).toBeVisible(); + + // An added column can be removed, taking the row label with it; a field's + // can only be hidden. await expect( picker.getByRole("button", { name: "Remove column task" }), ).toHaveCount(0); await picker.getByRole("button", { name: "Remove column title" }).click(); await expect.poll(() => headerOrder(component)).toEqual(["ticket", "task"]); + expect(sent.at(-1)?.rowLabel).toBeNull(); }); test("a column of an unknown kind keeps its place as a dimmed placeholder", async ({ diff --git a/src/client/components/dataset-columns.ts b/src/client/components/dataset-columns.ts index b7ee286..557bb60 100644 --- a/src/client/components/dataset-columns.ts +++ b/src/client/components/dataset-columns.ts @@ -16,6 +16,7 @@ import { type ForeignColumnSpec, fieldPath, isKnownColumn, + pathField, } from "../../shared/column-paths"; import type { DatasetField, DatasetRowsOverview } from "../../shared/types"; @@ -104,38 +105,39 @@ export function reconcileColumns( const { path, title } = spec; const id = columnPathId(path); if (seen.has(id)) continue; - const field = "field" in path.root ? path.root.field : undefined; + const field = pathField(path); if (field !== undefined && !fieldIds.has(field)) continue; - const keys = shape[field ?? ""]?.keys ?? []; - if (field !== undefined && path.keys.length === 0) { + const [key, ...deeper] = path.keys; + if (field !== undefined && key === undefined) { + // The field's whole column. seen.add(id); const entry = fieldEntry(field); entry.hidden = hidden; if (title !== undefined) entry.title = title; - } else if ( - field !== undefined && - path.keys.length === 1 && - keys.includes(path.keys[0]) - ) { - // A field's key, derived from the rows — even if it was added from - // the picker before any row held it. - seen.add(id); - fieldEntry(field).keys.push({ - key: path.keys[0], - hidden, - ...(title !== undefined && { title }), - }); - } else if (field !== undefined && path.keys.length === 1 && !spec.added) { - // A derived key no row holds any more: dropped. - } else { - seen.add(id); - layout.push({ - kind: "added", - path, - hidden, - ...(title !== undefined && { title }), - }); + continue; + } + if (field !== undefined && deeper.length === 0) { + if (shape[field]?.keys.includes(key)) { + // One of the field's keys, derived from the rows — even if it was + // added from the picker before any row held it. + seen.add(id); + fieldEntry(field).keys.push({ + key, + hidden, + ...(title !== undefined && { title }), + }); + continue; + } + // A derived key no row holds any more. + if (!spec.added) continue; } + seen.add(id); + layout.push({ + kind: "added", + path, + hidden, + ...(title !== undefined && { title }), + }); } for (const field of fields) { diff --git a/src/client/components/dataset-grid.test.ts b/src/client/components/dataset-grid.test.ts index 0277ce8..890e406 100644 --- a/src/client/components/dataset-grid.test.ts +++ b/src/client/components/dataset-grid.test.ts @@ -864,6 +864,20 @@ describe("columns from a layout (§Q.3)", () => { note: spec.args?.note, }); }); + + it("doesn't clear an argument in place, so its cell stays editable", () => { + const cols = [added(instancePath("seededTask", ["title"]))].filter( + c => c !== undefined, + ); + const range = [{ x: 0, y: 0, width: 1, height: 1 }]; + expect(clearEdits(range, cols, () => planTask)).toEqual([]); + expect( + cellEdits([{ col: 0, row: 0, value: "" }], cols, () => planTask), + ).toEqual([]); + expect(editableCell(planTask, cols[0])).toMatchObject({ + value: "Web site", + }); + }); }); }); diff --git a/src/client/components/dataset-grid.ts b/src/client/components/dataset-grid.ts index c91f8d3..a56c5ff 100644 --- a/src/client/components/dataset-grid.ts +++ b/src/client/components/dataset-grid.ts @@ -25,6 +25,7 @@ import { columnPathId, fieldPath, pathField, + pathInstance, readColumnPath, resolveColumnPath, } from "../../shared/column-paths"; @@ -110,8 +111,8 @@ export function addedTitle( ): string { const last = path.keys.at(-1); if (last !== undefined) return last; - if ("instance" in path.root) return path.root.instance; - const { field } = path.root; + const field = pathField(path); + if (field === undefined) return pathInstance(path) ?? ""; return fields.find(f => f.id === field)?.def.name ?? field; } @@ -120,12 +121,11 @@ export function pathLabel( path: ColumnPath, fields: readonly DatasetField[], ): string { + const field = pathField(path); const root = - "instance" in path.root - ? `◆ ${path.root.instance}` - : (fields.find(f => f.id === pathField(path))?.def.name ?? - pathField(path) ?? - ""); + field === undefined + ? `◆ ${pathInstance(path)}` + : (fields.find(f => f.id === field)?.def.name ?? field); return [root, ...path.keys].join(" › "); } @@ -510,6 +510,8 @@ export interface GridEdit { * The grid's edits as {@link CellEdit}s, skipping any that land outside the * editable set ({@link editableCell}) or don't fit the cell's type — a paste * or fill over a range can cover both, and the rest of it still applies. + * Emptying an instance's argument is skipped too: it would delete the + * argument, leaving nothing in the grid to type into (see {@link clearEdits}). */ export function cellEdits( edits: readonly GridEdit[], @@ -522,7 +524,7 @@ export function cellEdits( const e = row && editableCell(row, columns[col]); if (!e) continue; const cell = editedCell(e.base, value); - if (cell === undefined) continue; + if (cell === undefined || (cell === null && !("fieldId" in e))) continue; out.push({ rowId: row.id, ...targetOf(e), cell }); } return out; @@ -538,8 +540,10 @@ export interface CellRange { /** * Clearing `ranges` (Delete or Backspace over a selection): a `null` for each - * editable cell in them that holds something. Read-only cells are left as - * they are, even inside the selection — the details pane clears those. + * editable field cell in them that holds something. Read-only cells are + * left as they are, even inside the selection — the details pane clears + * those. So are instances' arguments: clearing one would delete it, and an + * argument that isn't there has no editor to type a new value into. */ export function clearEdits( ranges: readonly CellRange[], @@ -552,7 +556,7 @@ export function clearEdits( const row = rowAt(index); for (let col = x; col < x + width; col++) { const e = row && editableCell(row, columns[col]); - if (e && e.value !== undefined) { + if (e && "fieldId" in e && e.value !== undefined) { out.push({ rowId: row.id, ...targetOf(e), cell: null }); } } diff --git a/src/dataset/turso-dataset-provider.ts b/src/dataset/turso-dataset-provider.ts index 4564010..b8fcc9d 100644 --- a/src/dataset/turso-dataset-provider.ts +++ b/src/dataset/turso-dataset-provider.ts @@ -15,7 +15,9 @@ import { type ColumnPath, childInputs, type DatasetColumnSpec, + isPathColumn, pathField, + pathInstance, resolveColumnPath, } from "../shared/column-paths.ts"; import { matchKey } from "../shared/dataset-fields.ts"; @@ -400,11 +402,10 @@ export class TursoDatasetProvider implements DatasetProvider { // Only rows with something at the root can have anything inside it. The // rest of the walk is `resolveColumnPath`'s, row by row, so it agrees // with what the grid reads — references included, which resolve per row. + // A root's name can't hold a `"`: a field's is an id, and an instance's + // is checked by `parseColumnPath`. const field = pathField(path); - const rootKey = - field !== undefined - ? `$."${field}"` - : `$."${"instance" in path.root ? path.root.instance : ""}"`; + const rootKey = `$."${field ?? pathInstance(path)}"`; const rootColumn = field !== undefined ? datasetRows.cells : datasetRows.resources; const rows = await this.db.all<{ @@ -865,7 +866,7 @@ function layoutWithoutField( const pruned: StoredLayout = { ...(layout.columns && { columns: layout.columns.filter( - c => !(c.kind === "path" && readsField(c.path as ColumnPath)), + c => !(isPathColumn(c) && readsField(c.path)), ), }), ...(layout.rowLabel && diff --git a/src/server/handlers/datasets.test.ts b/src/server/handlers/datasets.test.ts index 2695256..f80eb20 100644 --- a/src/server/handlers/datasets.test.ts +++ b/src/server/handlers/datasets.test.ts @@ -532,6 +532,45 @@ describe("column layout handlers (§Q)", () => { ).toBe(400); }); + it("still saves a layout whose instance no row declares any more", async () => { + const { provider, dataset, set } = await seeded(); + const title = { root: { instance: "task" }, keys: ["title"] }; + const column = { kind: "path", path: title, added: true }; + expect((await set({ columns: [column], rowLabel: title })).status).toBe( + 200, + ); + const rows = await provider.listRows(dataset.id); + await provider.deleteRows( + dataset.id, + rows.map(r => r.id), + ); + + // Stop using it as the row label; remove the column; hide it. + expect((await set({ columns: [column], rowLabel: null })).status).toBe(200); + expect((await set({ columns: [], rowLabel: title })).status).toBe(200); + expect( + (await set({ columns: [{ ...column, hidden: true }], rowLabel: null })) + .status, + ).toBe(200); + // A new root the rows don't declare is still refused. + const other = { root: { instance: "other" }, keys: [] }; + expect( + (await set({ columns: [{ ...column, hidden: true }], rowLabel: other })) + .status, + ).toBe(400); + }); + + it("refuses an instance root that isn't a valid resource name", async () => { + const { dataset, provider, set } = await seeded(); + const bad = { root: { instance: 'a"b' }, keys: [] }; + const res = await set({ columns: [], rowLabel: bad }); + expect(res.status).toBe(400); + expect(JSON.stringify(res.body)).toContain("not a valid resource name"); + expect( + (await handleDescribePath(provider, dataset.id, { path: bad })).status, + ).toBe(400); + }); + it("describes a path, refusing an unknown field", async () => { const { provider, dataset } = await seeded(); const describe = (path: unknown) => diff --git a/src/server/handlers/datasets.ts b/src/server/handlers/datasets.ts index 825f35e..dee8d05 100644 --- a/src/server/handlers/datasets.ts +++ b/src/server/handlers/datasets.ts @@ -25,9 +25,11 @@ import type { import { type ColumnPath, InvalidColumnPathError, + isPathColumn, parseColumnPath, parseColumnSpecs, pathField, + pathInstance, } from "../../shared/column-paths.ts"; import { fitsPrimitiveBase, @@ -42,7 +44,6 @@ import { type PrimitiveFieldType, portableDef, } from "../../shared/dataset-fields.ts"; -import { isValidInstanceName } from "../../shared/instance-names.ts"; import type { CheckInfo, ExecutionInput, @@ -750,8 +751,8 @@ export async function handleQueryRows( } /** - * Checks that `path`'s root exists: a field in `dataset`'s schema, or an - * instance some row declares. + * Checks that `path`'s root exists: a field in `dataset`'s schema, or one of + * `instances`. */ function checkPathRoot( path: ColumnPath, @@ -764,13 +765,25 @@ function checkPathRoot( if (!dataset.fields.some(f => f.id === field)) { throw new BadRequest(`${at}: the dataset has no field "${field}"`); } - } else if ("instance" in path.root && !instances.has(path.root.instance)) { + return; + } + const instance = pathInstance(path); + if (instance !== undefined && !instances.has(instance)) { throw new BadRequest( - `${at}: no row declares a resource instance "${path.root.instance}"`, + `${at}: no row declares a resource instance "${instance}"`, ); } } +/** The instances `dataset`'s stored layout and row label are rooted at. */ +function storedInstances(dataset: Dataset): Set { + const paths = (dataset.columns ?? []).flatMap(c => + isPathColumn(c) ? [c.path] : [], + ); + if (dataset.rowLabel) paths.push(dataset.rowLabel); + return new Set(paths.flatMap(p => pathInstance(p) ?? [])); +} + /** * `POST /api/datasets/:providerId/:id/paths` — body `{ path }`: the keys one * level inside `path`, each with a sample (see @@ -789,11 +802,6 @@ export async function handleDescribePath( if (field !== undefined && !dataset.fields.some(f => f.id === field)) { throw new BadRequest(`path: the dataset has no field "${field}"`); } - if ("instance" in path.root && !isValidInstanceName(path.root.instance)) { - throw new BadRequest( - `path: "${path.root.instance}" is not a valid resource name`, - ); - } return { status: 200, body: await provider.describePath(datasetId, path) }; } catch (err) { return failure(err); @@ -803,9 +811,11 @@ export async function handleDescribePath( /** * `PATCH /api/datasets/:providerId/:id/columns` — body `{ columns, rowLabel }`: * replace the grid's shared column layout and row label (`null` clears the - * label). Each path's root must be a field of the dataset or an instance - * some row declares, and its keys non-empty strings. Columns of a kind this - * version doesn't know are stored as they are. See `specs/datasets.md` §Q.3. + * label). Each path's root must be a field of the dataset, or an instance + * some row declares or the stored layout or row label already reads — so a + * layout whose instance no row declares any more can still be saved — and + * its keys non-empty strings. Columns of a kind this version doesn't know + * are stored as they are. See `specs/datasets.md` §Q.3. */ export async function handleSetColumns( provider: DatasetProvider, @@ -824,15 +834,22 @@ export async function handleSetColumns( const dataset = await provider.getDataset(datasetId); if (!dataset) throw new DatasetNotFoundError(datasetId); const paths = columns.flatMap((c, i) => - c.kind === "path" - ? [{ path: c.path as ColumnPath, at: `columns[${i}].path` }] - : [], + isPathColumn(c) ? [{ path: c.path, at: `columns[${i}].path` }] : [], ); if (rowLabel) paths.push({ path: rowLabel, at: "rowLabel" }); - // Instance roots are checked against the rows only when there are any. - const instances = paths.some(({ path }) => "instance" in path.root) - ? new Set(Object.keys((await provider.describeRows(datasetId)).instances)) - : new Set(); + // An instance the layout already reads stays valid however the rows + // change, so a layout can always be saved; only a new one is looked up + // in the rows. + const instances = storedInstances(dataset); + if ( + paths.some(({ path }) => { + const instance = pathInstance(path); + return instance !== undefined && !instances.has(instance); + }) + ) { + const overview = await provider.describeRows(datasetId); + for (const name of Object.keys(overview.instances)) instances.add(name); + } for (const { path, at } of paths) { checkPathRoot(path, dataset, instances, at); } diff --git a/src/shared/column-paths.test.ts b/src/shared/column-paths.test.ts index 5965340..99d415f 100644 --- a/src/shared/column-paths.test.ts +++ b/src/shared/column-paths.test.ts @@ -244,6 +244,8 @@ describe("parseColumnPath", () => { { root: { field: "1" } }, { root: { field: "1" }, keys: [""] }, { root: { field: "1" }, keys: [3] }, + { root: { instance: 'a"b' }, keys: [] }, + { root: { instance: "9lives" }, keys: [] }, ]) { expect(() => parseColumnPath(bad)).toThrow(InvalidColumnPathError); } diff --git a/src/shared/column-paths.ts b/src/shared/column-paths.ts index 7fc9233..2848373 100644 --- a/src/shared/column-paths.ts +++ b/src/shared/column-paths.ts @@ -4,10 +4,11 @@ /** * Column paths: where a dataset column reads from in a row. Shared by the * grid, the server (which describes and validates paths) and dataset views, - * which must all agree on what a path reads. Pure — no runtime - * dependencies. See `specs/datasets.md` §Q.1. + * which must all agree on what a path reads. Pure, and depends only on + * the rest of `shared/`. See `specs/datasets.md` §Q.1. */ +import { isValidInstanceName } from "./instance-names.ts"; import type { ExecutionInput, ResourceInstanceInput } from "./types.ts"; /** Where a column reads from in a row. */ @@ -60,6 +61,14 @@ export function isKnownColumn( return KNOWN_COLUMN_KINDS.has(spec.kind); } +/** A layout column that reads a {@link ColumnPath}. */ +export type PathColumnSpec = Extract; + +/** Whether `spec` is a column that reads a path. */ +export function isPathColumn(spec: DatasetColumnSpec): spec is PathColumnSpec { + return isKnownColumn(spec) && spec.kind === "path"; +} + /** The parts of a row a path reads: its cells, and its resource instances. */ export interface PathRow { cells: Record; @@ -115,6 +124,11 @@ export function pathField(path: ColumnPath): string | undefined { return "field" in path.root ? path.root.field : undefined; } +/** The instance a path is rooted at, or `undefined` for a field root. */ +export function pathInstance(path: ColumnPath): string | undefined { + return "instance" in path.root ? path.root.instance : undefined; +} + /** * The inputs one step inward from `input`, by name, in order — what a path * can step to next: @@ -231,8 +245,8 @@ function isRecord(value: unknown): value is Record { /** * Checks that `value` is a well-formed {@link ColumnPath} — a root naming a - * field or an instance by a non-empty string, and `keys` of non-empty - * strings — and returns it. Whether the root exists is the caller's + * field by a non-empty string or an instance by a valid instance name, and + * `keys` of non-empty strings — and returns it. Whether the root exists is the caller's * business. * * @param at - Where `value` sits, for the error message. @@ -249,6 +263,11 @@ export function parseColumnPath(value: unknown, at = "path"): ColumnPath { if (typeof root.field === "string" && root.field !== "") { parsedRoot = { field: root.field }; } else if (typeof root.instance === "string" && root.instance !== "") { + if (!isValidInstanceName(root.instance)) { + throw new InvalidColumnPathError( + `${at}.root.instance: "${root.instance}" is not a valid resource name`, + ); + } parsedRoot = { instance: root.instance }; } else { throw new InvalidColumnPathError( From 5e98c054d069c14835b0c4f8c6185d22f19fff01 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 23:09:36 +0000 Subject: [PATCH 7/8] Don't crash when Glide asks about a hidden column's hovered cell Hiding the last column from its header menu, with the pointer left over one of its cells, crashed the grid: Glide's cursor check reads the stale hovered cell on the redraw that drops the column, and cellView dereferenced the missing column. A column past the end now shows empty. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01QMiMTviyLp3ciCvQf3mzkf --- .../__fixtures__/datasetView.pw.tsx | 53 +++++++++++++++++++ src/client/components/dataset-grid.test.ts | 6 +++ src/client/components/dataset-grid.ts | 10 +++- 3 files changed, 67 insertions(+), 2 deletions(-) diff --git a/src/client/components/__fixtures__/datasetView.pw.tsx b/src/client/components/__fixtures__/datasetView.pw.tsx index fb856b0..16361a8 100644 --- a/src/client/components/__fixtures__/datasetView.pw.tsx +++ b/src/client/components/__fixtures__/datasetView.pw.tsx @@ -1692,6 +1692,59 @@ test("an added column edits a referenced instance's argument inline, and can nam await expect(pane).toContainText("Row 1 · Buy oat milk"); }); +test("hiding an added resource column from its header menu redraws without it", async ({ + mount, + page, +}) => { + await page.setViewportSize({ width: 1200, height: 700 }); + const errors: string[] = []; + page.on("pageerror", e => errors.push(e.message)); + page.on("console", m => { + if (m.type() === "error") errors.push(m.text()); + }); + const dataset: Dataset = { + ...SUPPORT, + columns: [ + path("0"), + path("1"), + { kind: "source" }, + // Last, where "Add column…" puts it. + { + kind: "path", + path: { root: { instance: "seededTask" }, keys: [] }, + added: true, + }, + ], + }; + await mockDataset(page, dataset, SUPPORT_ROWS, SUPPORT_SHAPE); + const sent = await mockColumns(page, dataset); + const component = await mount( + , + ); + await expect + .poll(() => headerOrder(component)) + .toEqual(["ticket", "task", "source", "seededTask"]); + + // The pointer is left over one of the column's cells, as it is when the + // menu item sat over the grid: Glide still asks about the hovered cell on + // the redraw that drops the column. + await openColumnMenu(page, GRID.marker + 240 + 240 + 96, 200); + const box = await page.getByTestId("data-grid-canvas").boundingBox(); + if (!box) throw new Error("grid canvas not laid out"); + await page.mouse.move( + box.x + GRID.marker + 240 + 240 + 96 + 150, + box.y + GRID.header + 2 * GRID.row + GRID.row / 2, + ); + await page + .getByRole("menuitem", { name: "Hide column" }) + .dispatchEvent("click"); + await expect + .poll(() => headerOrder(component)) + .toEqual(["ticket", "task", "source"]); + expect(sent.at(-1)?.columns[3]).toMatchObject({ hidden: true }); + expect(errors).toEqual([]); +}); + test("the Columns picker hides a column, adds one from the path tree, and sets the row label", async ({ mount, page, diff --git a/src/client/components/dataset-grid.test.ts b/src/client/components/dataset-grid.test.ts index 890e406..0d2f6d8 100644 --- a/src/client/components/dataset-grid.test.ts +++ b/src/client/components/dataset-grid.test.ts @@ -175,6 +175,12 @@ describe("cellView", () => { expect(cellView(undefined, columns()[0])).toEqual({ kind: "loading" }); }); + it("shows nothing past the last column, where Glide's stale hover can ask", () => { + const cols = columns(); + expect(cellView(row({}), cols[cols.length])).toEqual({ kind: "empty" }); + expect(cellView(undefined, cols[cols.length])).toEqual({ kind: "empty" }); + }); + it("previews a value, an empty cell, and a resource instance as a chip", () => { const cols = columns(); const r = row({ "0": text("Hi"), "1": task }); diff --git a/src/client/components/dataset-grid.ts b/src/client/components/dataset-grid.ts index a56c5ff..2ac4fab 100644 --- a/src/client/components/dataset-grid.ts +++ b/src/client/components/dataset-grid.ts @@ -694,11 +694,17 @@ function inputView(input: ExecutionInput): CellView { } } -/** What the cell of `column` shows for `row`, which is `undefined` while its page loads. */ +/** + * What the cell of `column` shows for `row`, which is `undefined` while its + * page loads. `column` is `undefined` past the last column: Glide still asks + * about the hovered cell on the redraw that drops its column (hiding the + * last one from its header menu, say), and that cell shows nothing. + */ export function cellView( row: DatasetRow | undefined, - column: DatasetColumn, + column: DatasetColumn | undefined, ): CellView { + if (!column) return { kind: "empty" }; if (!row) return { kind: "loading" }; switch (column.role) { case "source": From 7afed0ebd24da5b2b40ffdc49421f88622f9885c Mon Sep 17 00:00:00 2001 From: Alex Corrado Date: Tue, 13 Oct 2026 10:38:21 +0100 Subject: [PATCH 8/8] Minor UI tweaks --- src/client/components/AskAgentButton.tsx | 19 +--------- src/client/components/DatasetColumnMenu.tsx | 2 +- src/client/components/DatasetColumnPicker.tsx | 9 +++-- src/client/components/DatasetView.tsx | 35 +++++++++++-------- src/client/components/NewEvalDialog.tsx | 1 + src/client/components/TraceView.tsx | 18 +++++----- .../__fixtures__/DatasetViewHarness.tsx | 4 +++ .../__fixtures__/datasetView.pw.tsx | 33 ++++++++++++++++- src/client/components/trace/icons.tsx | 18 ++++++++++ src/client/styles.css | 2 +- 10 files changed, 95 insertions(+), 46 deletions(-) diff --git a/src/client/components/AskAgentButton.tsx b/src/client/components/AskAgentButton.tsx index ab6c782..7396796 100644 --- a/src/client/components/AskAgentButton.tsx +++ b/src/client/components/AskAgentButton.tsx @@ -5,6 +5,7 @@ import { useState } from "react"; import { createPortal } from "react-dom"; import { type AgentInfo, OTHER_AGENT_URL } from "../../shared/agent"; import ProviderIcon from "./ProviderIcon"; +import { ChevronDownIcon } from "./trace/icons"; import { useAnchoredPopover } from "./use-anchored-popover"; interface Props { @@ -148,21 +149,3 @@ function AgentIcon() { ); } - -function ChevronDownIcon() { - return ( - - ); -} diff --git a/src/client/components/DatasetColumnMenu.tsx b/src/client/components/DatasetColumnMenu.tsx index 4107ccd..2adbddd 100644 --- a/src/client/components/DatasetColumnMenu.tsx +++ b/src/client/components/DatasetColumnMenu.tsx @@ -157,7 +157,7 @@ export function DatasetColumnMenu({ > {rowLabel.isLabel - ? "Stop using as row label" + ? "Don't use as row label" : "Use as row label"} diff --git a/src/client/components/DatasetColumnPicker.tsx b/src/client/components/DatasetColumnPicker.tsx index a069c56..b58f743 100644 --- a/src/client/components/DatasetColumnPicker.tsx +++ b/src/client/components/DatasetColumnPicker.tsx @@ -25,7 +25,11 @@ import { } from "./dataset-columns"; import { addedTitle, pathLabel } from "./dataset-grid"; import { previewCell } from "./dataset-preview"; -import { ColumnsIcon, GrabHandleIcon } from "./trace/icons.tsx"; +import { + ChevronDownIcon, + ColumnsIcon, + GrabHandleIcon, +} from "./trace/icons.tsx"; import { useAnchoredPopover } from "./use-anchored-popover"; interface Props { @@ -238,7 +242,7 @@ export function DatasetColumnPicker({ }} > - {isLabel ? "Stop using as row label" : "Use as row label"} + {isLabel ? "Don't use as row label" : "Use as row label"} @@ -326,6 +330,7 @@ export function DatasetColumnPicker({ > Columns + {open && createPortal( diff --git a/src/client/components/DatasetView.tsx b/src/client/components/DatasetView.tsx index 1309a74..95cc6cc 100644 --- a/src/client/components/DatasetView.tsx +++ b/src/client/components/DatasetView.tsx @@ -970,17 +970,6 @@ function DatasetView({ menuOpen && createPortal(
- {onAskAgent && ( - !agent.disabledReason)} - onAsk={agent => { - setMenuOpen(false); - onAskAgent(agent); - }} - /> - )} {onNewEval && ( )} + {onAskAgent && ( + !agent.disabledReason)} + onAsk={agent => { + setMenuOpen(false); + onAskAgent(agent); + }} + /> + )} + {(onNewEval || onAskAgent) &&
}
, @@ -1130,9 +1137,6 @@ function DatasetView({ } />
- {onAskAgent && ( - - )} {onNewEval && (
- {onAskAgent && ( - - )} - {addToDatasetProps && ( - - )} + {addToDatasetProps && ( + + )} + {onAskAgent && ( + + )} {onDeleted && (