From c0ce124f8c1f44a684c19cfdcdced27f4b888ce3 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Sat, 18 Jul 2026 10:28:36 -0400 Subject: [PATCH] refactor(tui): adopt reactive first index and read-only keyed surface - quark: Collection.first(name) is now a stable Readable; Keyed.ReadOnly names the read surface; useSlot takes plain constant keys; Layout.collectionOf() replaces the content plan double cast - BackgroundToolHint reads a backgroundRunning first index instead of hand-tracking structure plus tool slots - toolDisplay moves to util/tool-display (content.ts needs it for the index) --- packages/quark/bench/collection.ts | 195 ++++++++++++++++++ packages/quark/src/keyed.ts | 10 +- packages/quark/src/layout.ts | 83 +++++--- packages/quark/src/reactivity.ts | 14 +- packages/quark/src/solid.ts | 9 +- packages/quark/test/layout.test.ts | 50 +++-- packages/quark/test/solid.test.ts | 17 +- packages/tui/src/routes/session/content.ts | 17 +- packages/tui/src/routes/session/index.tsx | 50 +---- .../tui/src/routes/session/permission.tsx | 2 +- packages/tui/src/util/tool-display.ts | 23 +++ .../tui/inline-tool-wrap-snapshot.test.tsx | 2 +- 12 files changed, 374 insertions(+), 98 deletions(-) create mode 100644 packages/quark/bench/collection.ts diff --git a/packages/quark/bench/collection.ts b/packages/quark/bench/collection.ts new file mode 100644 index 0000000000..ce6c14819a --- /dev/null +++ b/packages/quark/bench/collection.ts @@ -0,0 +1,195 @@ +import { Layout } from "../src/layout" +import { createHarness, type Workload } from "./harness" + +type Job = { + readonly id: number + readonly labels: readonly number[] + readonly status: "running" | "retrying" + readonly value: number +} + +const size = 1_000 +const iterations = 200_000 +const initial = Array.from({ length: size }, (_, id): Job => ({ + id, + labels: [id], + status: id === 750 ? "retrying" : "running", + value: 0, +})) +const JobLayout = Layout.struct({ + id: Layout.key(Layout.number), + labels: Layout.array(Layout.number), + status: Layout.string, + value: Layout.number, +}) +const Jobs = Layout.collection(JobLayout, ({ members, first }) => ({ + labels: members(["labels"], (job) => job.labels), + retry: first(["status"], (job) => job.status === "retrying"), +})) +const LabeledJobs = Layout.collection(JobLayout, ({ members }) => ({ + labels: members(["labels"], (job) => job.labels), +})) +const JobPlan = Layout.compile(JobLayout) +const bench = createHarness({ warmup: 10_000, width: 38 }) + +function manualValueUpdate(): Workload { + const jobs = JobPlan.make(initial) + const labels = new Set(initial.flatMap((job) => job.labels)) + const retry = jobs.get(750)! + return { + run(index) { + const id = index % size + jobs.modify(id, (job) => ({ ...job, value: index + 1 })) + }, + consume: () => Number(labels.has(500)) + retry().id + jobs.get(iterations % size)!().value, + } +} + +function indexedValueUpdate(): Workload { + const jobs = Jobs.make(initial) + return { + run(index) { + const id = index % size + jobs.modify(id, (job) => ({ ...job, value: index + 1 })) + }, + consume: () => + Number(jobs.hasMember("labels", 500)) + jobs.first("retry")()!.id + jobs.get(iterations % size)!().value, + } +} + +function manualMemberAppend(): Workload { + const jobs = JobPlan.make(initial) + const labels = new Map(initial.flatMap((job) => job.labels.map((label) => [label, 1]))) + const members = new Map(initial.map((job) => [job.id, new Set(job.labels)])) + return { + run(index) { + const id = index % size + const label = size + index + jobs.modify(id, (job) => { + const next = [...job.labels.slice(-7), label] + const previous = members.get(id)! + const current = new Set(next) + previous.forEach((member) => { + if (current.has(member)) return + const count = labels.get(member)! + if (count === 1) labels.delete(member) + if (count > 1) labels.set(member, count - 1) + }) + current.forEach((member) => { + if (!previous.has(member)) labels.set(member, (labels.get(member) ?? 0) + 1) + }) + members.set(id, current) + return { ...job, labels: next } + }) + }, + consume: () => Number(labels.has(size + iterations - 1)) + jobs.get(iterations % size)!().labels.length, + } +} + +function indexedMemberAppend(): Workload { + const jobs = LabeledJobs.make(initial) + return { + run(index) { + const id = index % size + const label = size + index + jobs.modify(id, (job) => ({ + ...job, + labels: [...job.labels.slice(-7), label], + })) + }, + consume: () => + Number(jobs.hasMember("labels", size + iterations - 1)) + jobs.get(iterations % size)!().labels.length, + } +} + +function manualGrowingAppend(): Workload { + const jobs = JobPlan.make([{ id: 0, labels: [], status: "running", value: 0 }]) + const labels = new Set() + let label = 0 + return { + run() { + const next = label++ + jobs.modify(0, (job) => ({ ...job, labels: [...job.labels, next] })) + labels.add(next) + }, + consume: () => Number(labels.has(label - 1)) + jobs.get(0)!().labels.length, + } +} + +function indexedGrowingAppend(): Workload { + const jobs = LabeledJobs.make([{ id: 0, labels: [], status: "running", value: 0 }]) + let label = 0 + return { + run() { + const next = label++ + jobs.modify(0, (job) => ({ ...job, labels: [...job.labels, next] }), { + members: { labels: { add: [next] } }, + }) + }, + consume: () => Number(jobs.hasMember("labels", label - 1)) + jobs.get(0)!().labels.length, + } +} + +function automaticGrowingAppend(): Workload { + const jobs = LabeledJobs.make([{ id: 0, labels: [], status: "running", value: 0 }]) + let label = 0 + return { + run() { + const next = label++ + jobs.modify(0, (job) => ({ ...job, labels: [...job.labels, next] })) + }, + consume: () => Number(jobs.hasMember("labels", label - 1)) + jobs.get(0)!().labels.length, + } +} + +function collectionSet(indexed: boolean, changed: boolean): Workload { + const jobs = indexed ? Jobs.make(initial) : JobPlan.make(initial) + return { + run(index) { + const id = index % size + const current = jobs.values() + const job = current[id] + jobs.set(current.with(id, { ...job, value: changed ? job.value + 1 : job.value })) + }, + consume: () => jobs.values()[0].value + jobs.values().length, + } +} + +console.log(`Compiled collection benchmark (${size} items, ${bench.samples} samples)\n`) + +const value = bench.compare(iterations, [ + { name: "Handwritten value update", make: manualValueUpdate }, + { name: "Compiled indexed value update", make: indexedValueUpdate }, +]) +const member = bench.compare(iterations, [ + { name: "Handwritten member append", make: manualMemberAppend }, + { name: "Compiled indexed member append", make: indexedMemberAppend }, +]) +const growing = bench.compare(2_000, [ + { name: "Handwritten growing append", make: manualGrowingAppend }, + { name: "Automatic indexed growing append", make: automaticGrowingAppend }, + { name: "Indexed delta growing append", make: indexedGrowingAppend }, +]) +const equivalentSet = bench.compare(2_000, [ + { name: "Bare keyed equivalent set", make: () => collectionSet(false, false) }, + { name: "Compiled indexed equivalent set", make: () => collectionSet(true, false) }, +]) +const changedSet = bench.compare(2_000, [ + { name: "Bare keyed changed set", make: () => collectionSet(false, true) }, + { name: "Compiled indexed changed set", make: () => collectionSet(true, true) }, +]) + +console.log("\nRatios to handwritten (lower is faster)") +console.log(`Value update: ${value.ratio(1, 0).toFixed(3)}x`) +console.log(`Member append: ${member.ratio(1, 0).toFixed(3)}x`) +console.log(`Automatic growing append: ${growing.ratio(1, 0).toFixed(3)}x`) +console.log(`Delta growing append: ${growing.ratio(2, 0).toFixed(3)}x`) +console.log(`Equivalent collection set: ${equivalentSet.ratio(1, 0).toFixed(3)}x`) +console.log(`Changed collection set: ${changedSet.ratio(1, 0).toFixed(3)}x`) +console.log(`METRIC collection_value_ratio=${value.ratio(1, 0).toFixed(6)}`) +console.log(`METRIC collection_member_ratio=${member.ratio(1, 0).toFixed(6)}`) +console.log(`METRIC collection_automatic_growing_ratio=${growing.ratio(1, 0).toFixed(6)}`) +console.log(`METRIC collection_delta_growing_ratio=${growing.ratio(2, 0).toFixed(6)}`) +console.log(`METRIC collection_equivalent_set_ratio=${equivalentSet.ratio(1, 0).toFixed(6)}`) +console.log(`METRIC collection_changed_set_ratio=${changedSet.ratio(1, 0).toFixed(6)}`) +bench.finish() diff --git a/packages/quark/src/keyed.ts b/packages/quark/src/keyed.ts index 575aa196bb..c876968cff 100644 --- a/packages/quark/src/keyed.ts +++ b/packages/quark/src/keyed.ts @@ -9,19 +9,23 @@ export namespace Keyed { equivalenceSuppressions: number } - export interface Keyed { + /** Read surface of a keyed collection; hand this to consumers that must not mutate. */ + export interface ReadOnly { readonly slots: Readable[]> readonly values: Readable has(key: Key): boolean get(key: Key): Readable | undefined + before(key: Key): Readable | undefined + after(key: Key): Readable | undefined + } + + export interface Keyed extends ReadOnly { set(values: readonly A[]): boolean update(value: A): boolean modify(key: Key, f: (value: A) => A): boolean insert(value: A, position?: Position): Readable remove(key: Key): boolean move(key: Key, position?: Position): boolean - before(key: Key): Readable | undefined - after(key: Key): Readable | undefined } export function make(options: { diff --git a/packages/quark/src/layout.ts b/packages/quark/src/layout.ts index 662006f09e..c50bb52240 100644 --- a/packages/quark/src/layout.ts +++ b/packages/quark/src/layout.ts @@ -1,5 +1,5 @@ import { Keyed } from "./keyed" -import { Transaction, type Readable } from "./reactivity" +import { Computed, State, Transaction, type Readable } from "./reactivity" export namespace Layout { export interface Field { @@ -41,10 +41,8 @@ export namespace Layout { readonly fields: StructFields } - export interface Union< - Tag extends PropertyKey, - Variants extends Readonly>>, - > extends Field> { + export interface Union>>> + extends Field> { readonly type: "union" readonly tag: Tag readonly variants: Variants @@ -135,7 +133,8 @@ export namespace Layout { export interface Collection> extends Keyed.Keyed { modify(key: Key, f: (value: A) => A, changes?: { readonly members?: MemberChanges }): boolean hasMember>(name: Name, member: Member): boolean - first>(name: Name): Readable | undefined + /** Reactive first match of the named index; publishes when the first match changes identity or value. */ + first>(name: Name): Readable } export interface CollectionPlan> extends Plan { @@ -223,7 +222,8 @@ export namespace Layout { >(layout: KeyedUnion, options?: CompileOptions): Plan, A> export function compile(input: unknown, options: CompileOptions = {}): unknown { const layout = input as - Struct | KeyedUnion>>> + | Struct + | KeyedUnion>>> if (layout.type === "keyed-union") { const model = unionDiffModel(layout.key.name, layout.tag, layout.variants) return makePlan( @@ -276,11 +276,17 @@ export namespace Layout { export function collection(input: unknown, define: unknown, options?: CompileOptions): unknown { const plan = compile(input as never, options) as Plan const definitions = (define as (index: IndexBuilder) => Indexes)({ - members: ((fieldsOrExtract: readonly PropertyKey[] | ((value: unknown) => Iterable), extract?: (value: unknown) => Iterable) => + members: (( + fieldsOrExtract: readonly PropertyKey[] | ((value: unknown) => Iterable), + extract?: (value: unknown) => Iterable, + ) => typeof fieldsOrExtract === "function" ? { type: "members", extract: fieldsOrExtract } : { type: "members", fields: fieldsOrExtract, extract: extract! }) as IndexBuilder["members"], - first: ((fieldsOrMatches: readonly PropertyKey[] | ((value: unknown) => boolean), matches?: (value: unknown) => boolean) => + first: (( + fieldsOrMatches: readonly PropertyKey[] | ((value: unknown) => boolean), + matches?: (value: unknown) => boolean, + ) => typeof fieldsOrMatches === "function" ? { type: "first", matches: fieldsOrMatches } : { type: "first", fields: fieldsOrMatches, matches: matches! }) as IndexBuilder["first"], @@ -293,6 +299,28 @@ export namespace Layout { } } + /** + * Explicit-target collection compiler for keyed-union layouts whose derived + * value type intentionally under-describes the real one (e.g. reference + * fields typed `unknown`, optional fields omitted). The layout's key name, + * key type, and tag values are checked against `Target`; field-level shapes + * are trusted at this single boundary. + */ + export function collectionOf() { + return function < + const Name extends keyof Target & PropertyKey, + const Tag extends keyof Target & PropertyKey, + const Variants extends Readonly, Field>>, + const Definitions extends Indexes, + >( + layout: KeyedUnion, + define: (index: IndexBuilder) => Definitions, + options?: CompileOptions, + ): CollectionPlan { + return collection(layout as never, define as never, options) as CollectionPlan + } + } + function makePlan( key: PropertyKey, getKey: (value: A) => Key, @@ -358,7 +386,7 @@ export namespace Layout { afterPlacement?(slot: Readable): void afterRebuild?(): void member?(candidate: unknown): boolean - firstSlot?(): Readable | undefined + readonly first?: Readable } function membersEntry( @@ -421,29 +449,34 @@ export namespace Layout { matches: (value: A) => boolean, ): Entry { const matching = new Set() - let slot: Readable | undefined + // The current first-matching slot is reactive state so `first` readers + // observe identity changes; the flattening computed below also tracks + // the slot itself, so value changes of the first match publish too. + const slot = State.make | undefined>(undefined) + const first = Computed.make(() => slot()?.()) function findFirst() { - slot = values.slots().find((candidate) => matching.has(plan.keyOf(candidate()))) + slot.set(values.slots().find((candidate) => matching.has(plan.keyOf(candidate())))) } function afterPlacement(candidate: Readable) { if (!matching.has(plan.keyOf(candidate()))) return - if (!slot) { - slot = candidate + const current = slot() + if (!current) { + slot.set(candidate) return } - if (slot === candidate) { + if (current === candidate) { findFirst() return } // Single pass: whichever of the two slots appears first wins. - for (const current of values.slots()) { - if (current === candidate) { - slot = candidate + for (const item of values.slots()) { + if (item === candidate) { + slot.set(candidate) return } - if (current === slot) return + if (item === current) return } } @@ -457,20 +490,20 @@ export namespace Layout { if (matched) matching.add(key) if (!matched) matching.delete(key) if (mode === "add") return - if (slot === valueSlot && !matched) { + if (slot() === valueSlot && !matched) { findFirst() return } - if (slot !== valueSlot && !previous && matched) afterPlacement(valueSlot) + if (slot() !== valueSlot && !previous && matched) afterPlacement(valueSlot) } }, remove(key, removedSlot) { matching.delete(key) - if (removedSlot && slot === removedSlot) findFirst() + if (removedSlot && slot() === removedSlot) findFirst() }, afterPlacement, afterRebuild: findFirst, - firstSlot: () => slot, + first, } } @@ -628,7 +661,9 @@ export namespace Layout { return byName.get(normalizeName(name))?.member?.(member) ?? false }, first(name) { - return byName.get(normalizeName(name))?.firstSlot?.() + const entry = byName.get(normalizeName(name)) + if (!entry?.first) throw new Error(`Unknown first index: ${String(name)}`) + return entry.first }, } collection.set(initial) diff --git a/packages/quark/src/reactivity.ts b/packages/quark/src/reactivity.ts index 2ffdd3dc74..c0801e3d41 100644 --- a/packages/quark/src/reactivity.ts +++ b/packages/quark/src/reactivity.ts @@ -1,3 +1,11 @@ +/** + * A callable reactive value. + * + * `subscribe`, `set`, and `update` are shared this-based methods, not + * per-instance closures: invoke them as methods (`readable.subscribe(f)`) or + * bind before detaching (`readable.subscribe.bind(readable)`). A detached + * bare reference loses its node and throws. + */ export interface Readable { (): A subscribe(listener: (value: A) => void): () => void @@ -188,8 +196,7 @@ function readComputed(node: ComputedNode): A { } if ( flags & Flags.Dirty || - (flags & Flags.Pending && - (checkDirty(node.deps!, node) || ((node.flags = flags & ~Flags.Pending), false))) || + (flags & Flags.Pending && (checkDirty(node.deps!, node) || ((node.flags = flags & ~Flags.Pending), false))) || !flags ) { if (updateComputed(node) && node.subs !== undefined) { @@ -255,8 +262,7 @@ function runWatcher(watcher: WatcherNode): void { const flags = watcher.flags watcher.flags = flags & ~(Flags.Queued | Flags.Dirty | Flags.Pending) if (!(flags & Flags.Watching)) return - const dirty = - !!(flags & Flags.Dirty) || (!!(flags & Flags.Pending) && checkDirty(watcher.deps!, watcher)) + const dirty = !!(flags & Flags.Dirty) || (!!(flags & Flags.Pending) && checkDirty(watcher.deps!, watcher)) // Revalidation runs user code in computed evaluations, which may dispose // this watcher; a disposed watcher must not deliver. if (!dirty || !(watcher.flags & Flags.Watching)) return diff --git a/packages/quark/src/solid.ts b/packages/quark/src/solid.ts index 9acd6bcfff..47bebfef92 100644 --- a/packages/quark/src/solid.ts +++ b/packages/quark/src/solid.ts @@ -10,12 +10,17 @@ export function useValue(readable: Readable): Accessor { * Reactive accessor for one keyed slot. Structural changes re-resolve the * slot, while memo equality prevents an unchanged slot from propagating to the * consumer. Value changes flow through the slot itself. + * + * The key is usually constant and may be passed plainly; pass an accessor when + * the key itself is reactive. (A key that is itself a function value is + * indistinguishable from an accessor and must be wrapped.) */ -export function useSlot(keyed: Pick, "slots" | "get">, key: () => Key): Accessor { +export function useSlot(keyed: Keyed.ReadOnly, key: Key | (() => Key)): Accessor { + const resolve = typeof key === "function" ? (key as () => Key) : () => key const structure = useValue(keyed.slots) const slot = createMemo(() => { structure() - return keyed.get(key()) + return keyed.get(resolve()) }) const value = createMemo(() => { const current = slot() diff --git a/packages/quark/test/layout.test.ts b/packages/quark/test/layout.test.ts index 91e5f66ad6..2377eec496 100644 --- a/packages/quark/test/layout.test.ts +++ b/packages/quark/test/layout.test.ts @@ -224,19 +224,40 @@ describe("Layout", () => { expect(jobs.hasMember("labels", "billing")).toBe(true) expect(jobs.hasMember("labels", "missing")).toBe(false) - expect(jobs.first("nextRetry")?.().id).toBe("two") + expect(jobs.first("nextRetry")()?.id).toBe("two") expect(jobs.before("two")?.().id).toBe("one") expect(jobs.after("two")?.().id).toBe("three") jobs.modify("one", (job) => ({ ...job, labels: [], status: "retrying" })) expect(jobs.hasMember("labels", "urgent")).toBe(false) expect(jobs.hasMember("labels", "billing")).toBe(true) - expect(jobs.first("nextRetry")?.().id).toBe("one") + expect(jobs.first("nextRetry")()?.id).toBe("one") jobs.remove("two") expect(jobs.hasMember("labels", "billing")).toBe(false) jobs.move("three", { before: "one" }) - expect(jobs.first("nextRetry")?.().id).toBe("three") + expect(jobs.first("nextRetry")()?.id).toBe("three") + }) + + it("publishes first-index changes to subscribers", () => { + const jobs = Jobs.make([ + { id: "one", labels: [], status: "running" }, + { id: "two", labels: [], status: "retrying" }, + ]) + const seen: (string | undefined)[] = [] + const dispose = jobs.first("nextRetry").subscribe((job) => seen.push(job?.id)) + + // Identity change: a match earlier in slot order becomes the first. + jobs.modify("one", (job) => ({ ...job, status: "retrying" })) + // Value change of the current first match publishes through the slot. + jobs.modify("one", (job) => ({ ...job, labels: ["late"] })) + // The first match stops matching; the next one takes over. + jobs.modify("one", (job) => ({ ...job, status: "done" })) + // No match left. + jobs.remove("two") + + expect(seen).toEqual(["one", "one", "two", undefined]) + dispose() }) it("keeps indexes synchronized across inserts, updates, and replacement", () => { @@ -244,14 +265,14 @@ describe("Layout", () => { jobs.insert({ id: "three", labels: ["shared"], status: "retrying" }) jobs.insert({ id: "two", labels: ["shared"], status: "retrying" }, { before: "three" }) - expect(jobs.first("nextRetry")?.().id).toBe("two") + expect(jobs.first("nextRetry")()?.id).toBe("two") jobs.update({ id: "two", labels: [], status: "done" }) - expect(jobs.first("nextRetry")?.().id).toBe("three") + expect(jobs.first("nextRetry")()?.id).toBe("three") expect(jobs.hasMember("labels", "shared")).toBe(true) jobs.remove("three") - expect(jobs.first("nextRetry")).toBeUndefined() + expect(jobs.first("nextRetry")()).toBeUndefined() expect(jobs.hasMember("labels", "shared")).toBe(false) jobs.set([ @@ -260,7 +281,7 @@ describe("Layout", () => { ]) expect(jobs.hasMember("labels", "replacement")).toBe(true) expect(jobs.hasMember("labels", "one")).toBe(false) - expect(jobs.first("nextRetry")?.().id).toBe("four") + expect(jobs.first("nextRetry")()?.id).toBe("four") }) it("skips index projection when the change is disjoint from its declared fields", () => { @@ -284,7 +305,7 @@ describe("Layout", () => { jobs.update({ id: "one", labels, status: "retrying" }) expect(extractions).toBe(baseline.extractions) expect(matchChecks).toBe(baseline.matchChecks + 1) - expect(jobs.first("nextRetry")?.().id).toBe("one") + expect(jobs.first("nextRetry")()?.id).toBe("one") // Labels-only change: the first-match check reads only `status`, so it skips. jobs.update({ id: "one", labels: ["urgent"], status: "retrying" }) @@ -340,14 +361,11 @@ describe("Layout", () => { const jobs = CountedJobs.make([one, two]) comparisons = 0 - jobs.set([ - { ...one }, - { ...two, value: "after" }, - ]) + jobs.set([{ ...one }, { ...two, value: "after" }]) expect(comparisons).toBe(4) expect(jobs.get("one")?.()).toBe(one) - expect(jobs.first("changed")?.().id).toBe("two") + expect(jobs.first("changed")()?.id).toBe("two") comparisons = 0 jobs.update({ ...jobs.get("one")!(), status: "busy" }) @@ -356,7 +374,7 @@ describe("Layout", () => { comparisons = 0 jobs.modify("two", (job) => ({ ...job, value: "done" })) expect(comparisons).toBe(1) - expect(jobs.first("changed")).toBeUndefined() + expect(jobs.first("changed")()).toBeUndefined() }) it("refreshes indexes that read a changed union discriminant", () => { @@ -379,7 +397,7 @@ describe("Layout", () => { expect(rows.get(1)?.().type).toBe("ready") expect(rows.hasMember("types", "waiting")).toBe(false) expect(rows.hasMember("types", "ready")).toBe(true) - expect(rows.first("firstReady")?.().type).toBe("ready") + expect(rows.first("firstReady")()?.type).toBe("ready") }) it("tracks lazy member iterables while they are consumed", () => { @@ -470,7 +488,7 @@ describe("Layout", () => { expect(jobs.values()).toEqual([{ id: "one", labels: ["safe"], status: "running" }]) expect(jobs.hasMember("labels", "safe")).toBe(true) expect(jobs.hasMember("labels", "boom")).toBe(false) - expect(jobs.first("nextRetry")).toBeUndefined() + expect(jobs.first("nextRetry")()).toBeUndefined() expect(() => jobs.insert({ id: "two", labels: ["boom"], status: "retrying" }, { before: "missing" })).toThrow( "Keyed value does not exist: missing", diff --git a/packages/quark/test/solid.test.ts b/packages/quark/test/solid.test.ts index 43ebaab0d0..06a87ae85b 100644 --- a/packages/quark/test/solid.test.ts +++ b/packages/quark/test/solid.test.ts @@ -33,7 +33,7 @@ describe("Solid adapter", () => { createRoot((dispose) => { const current = useValue(state) - createComputed(() => value = current()) + createComputed(() => (value = current())) state.set(second) dispose() }) @@ -41,6 +41,21 @@ describe("Solid adapter", () => { expect(value).toBe(second) }) + it("accepts a plain constant key", () => { + const keyed = Keyed.make<{ readonly id: number; readonly value: string }, number>({ key: (value) => value.id }) + keyed.set([{ id: 1, value: "one" }]) + const values: Array = [] + + createRoot((dispose) => { + const value = useSlot(keyed, 1) + createComputed(() => values.push(value()?.value)) + keyed.modify(1, (item) => ({ ...item, value: "ONE" })) + dispose() + }) + + expect(values).toEqual(["one", "ONE"]) + }) + it("switches keyed slots and releases obsolete subscriptions", () => { const keyed = Keyed.make<{ readonly id: number; readonly value: string }, number>({ key: (value) => value.id }) keyed.set([ diff --git a/packages/tui/src/routes/session/content.ts b/packages/tui/src/routes/session/content.ts index eaff9c6560..8182bc9e78 100644 --- a/packages/tui/src/routes/session/content.ts +++ b/packages/tui/src/routes/session/content.ts @@ -1,5 +1,6 @@ import type { SessionMessageAssistant } from "@opencode-ai/client" import { Keyed, Layout } from "@opencode-ai/quark" +import { toolDisplay } from "../../util/tool-display" /** * Stable per-part reactive slots for assistant message content. @@ -12,9 +13,9 @@ import { Keyed, Layout } from "@opencode-ai/quark" export namespace SessionContent { type ContentPart = SessionMessageAssistant["content"][number] export type Part = ContentPart & { readonly partID: string } - export type Parts = Keyed.Keyed + export type Parts = ReturnType /** Read surface for view components; mutation stays with the data layer. */ - export type PartsView = Pick + export type PartsView = Keyed.ReadOnly & Pick // Streamed sub-objects are replaced immutably on change, so reference // equality is the correct (and cheapest) field comparator for them. @@ -38,8 +39,16 @@ export namespace SessionContent { }, }) // The layout describes the reactive fields of the client content shapes; - // the plan is typed against those shapes at this single boundary. - const PartPlan = Layout.compile(PartLayout) as unknown as Layout.Plan + // collectionOf types the plan against those shapes at this single boundary. + const PartPlan = Layout.collectionOf()(PartLayout, ({ first }) => ({ + // First running shell/subagent tool; drives the background-work hint + // without subscribing views to whole-collection values. + backgroundRunning: first(["type", "state"], (part) => { + if (part.type !== "tool" || part.state.status !== "running") return false + const display = toolDisplay(part.name) + return display === "shell" || display === "subagent" + }), + })) export function textID(ordinal: number) { return `text:${ordinal}` diff --git a/packages/tui/src/routes/session/index.tsx b/packages/tui/src/routes/session/index.tsx index a791600905..105cafdf34 100644 --- a/packages/tui/src/routes/session/index.tsx +++ b/packages/tui/src/routes/session/index.tsx @@ -41,7 +41,7 @@ import type { import { useLocal } from "../../context/local" import { Locale } from "../../util/locale" import { FilePath } from "../../ui/file-path" -import { webSearchProviderLabel } from "../../util/tool-display" +import { toolDisplay, webSearchProviderLabel } from "../../util/tool-display" import { useRenderer, useTerminalDimensions, type JSX } from "@opentui/solid" import { useClient } from "../../context/client" import { useEditorContext } from "../../context/editor" @@ -1107,24 +1107,13 @@ function BackgroundToolHint(props: { (message): message is SessionMessageAssistant => message.type === "assistant" && !message.time.completed, ), ) - // Track the part structure (publishes only on part insert/remove) and the - // individual tool slots; text and reasoning deltas never reach this memo. - const toolSlots = createMemo(() => { + // The collection maintains the first-match index; text and reasoning deltas + // never publish it, so this memo re-runs only on background-tool changes. + const visible = createMemo(() => { const message = current() - if (!message) return [] - const parts = props.parts(message.id) - return useValue(parts.slots)() - .filter((slot) => slot().type === "tool") - .map((slot) => useValue(slot)) + if (!message) return false + return useValue(props.parts(message.id).first("backgroundRunning"))() !== undefined }) - const visible = createMemo(() => - toolSlots().some((tool) => { - const part = tool() - if (part.type !== "tool" || part.state.status !== "running") return false - const display = toolDisplay(part.name) - return display === "shell" || display === "subagent" - }), - ) return ( {(value) => ( @@ -1169,7 +1158,7 @@ function SessionMessageView(props: { message: SessionMessageInfo }) { // instead of rebuilding the whole accessor list; value changes flow through // the individual slots. function usePartSlots(parts: (messageID: string) => SessionContent.PartsView, refs: () => readonly PartRef[]) { - return mapArray(refs, (ref) => ({ ref, part: useSlot(parts(ref.messageID), () => ref.partID) })) + return mapArray(refs, (ref) => ({ ref, part: useSlot(parts(ref.messageID), ref.partID) })) } function SessionPartView(props: { @@ -1285,7 +1274,7 @@ function SessionReasoningGroupView(props: { {(ref) => { - const slot = useSlot(props.parts(ref.messageID), () => ref.partID) + const slot = useSlot(props.parts(ref.messageID), ref.partID) const part = createMemo(() => { const item = slot() return item?.type === "reasoning" ? item : undefined @@ -2942,29 +2931,6 @@ function numberValue(value: unknown) { return typeof value === "number" && Number.isFinite(value) ? value : undefined } -const toolDisplays = new Set([ - "shell", - "glob", - "read", - "grep", - "webfetch", - "websearch", - "write", - "edit", - "subagent", - "execute", - "patch", - "question", - "skill", -]) - -export function toolDisplay(tool: string) { - // Legacy transcripts recorded the shell tool as "bash" and the subagent tool as "task"; render - // them with the renamed views. - const normalized = tool === "bash" ? "shell" : tool === "task" ? "subagent" : tool === "apply_patch" ? "patch" : tool - return toolDisplays.has(normalized) ? normalized : "generic" -} - function recordValue(value: unknown): Record | undefined { if (typeof value !== "object" || value === null || Array.isArray(value)) return return value as Record diff --git a/packages/tui/src/routes/session/permission.tsx b/packages/tui/src/routes/session/permission.tsx index 9f596a017e..295caa30ed 100644 --- a/packages/tui/src/routes/session/permission.tsx +++ b/packages/tui/src/routes/session/permission.tsx @@ -24,7 +24,7 @@ export function usePermissionInput(request: PermissionV2Request): () => Record ({}) - const part = useSlot(data.session.message.parts(request.sessionID, tool.messageID), () => tool.callID) + const part = useSlot(data.session.message.parts(request.sessionID, tool.messageID), tool.callID) return createMemo(() => { const item = part() if (item?.type === "tool" && item.state.status !== "streaming") return item.state.input diff --git a/packages/tui/src/util/tool-display.ts b/packages/tui/src/util/tool-display.ts index c9d92434e2..eed92c5c8e 100644 --- a/packages/tui/src/util/tool-display.ts +++ b/packages/tui/src/util/tool-display.ts @@ -1,3 +1,26 @@ +const toolDisplays = new Set([ + "shell", + "glob", + "read", + "grep", + "webfetch", + "websearch", + "write", + "edit", + "subagent", + "execute", + "patch", + "question", + "skill", +]) + +export function toolDisplay(tool: string) { + // Legacy transcripts recorded the shell tool as "bash" and the subagent tool as "task"; render + // them with the renamed views. + const normalized = tool === "bash" ? "shell" : tool === "task" ? "subagent" : tool === "apply_patch" ? "patch" : tool + return toolDisplays.has(normalized) ? normalized : "generic" +} + export function webSearchProviderLabel(provider: unknown) { if (provider === "parallel") return "Parallel Web Search" if (provider === "exa") return "Exa Web Search" diff --git a/packages/tui/test/cli/tui/inline-tool-wrap-snapshot.test.tsx b/packages/tui/test/cli/tui/inline-tool-wrap-snapshot.test.tsx index 1f44f92600..4b77df974f 100644 --- a/packages/tui/test/cli/tui/inline-tool-wrap-snapshot.test.tsx +++ b/packages/tui/test/cli/tui/inline-tool-wrap-snapshot.test.tsx @@ -9,8 +9,8 @@ import { parseDiagnostics, parseQuestionAnswers, parseQuestions, - toolDisplay, } from "../../../src/routes/session" +import { toolDisplay } from "../../../src/util/tool-display" let testSetup: Awaited> | undefined