diff --git a/packages/core/src/agent-operations/batch.ts b/packages/core/src/agent-operations/batch.ts index 212e47a143..03e5cc8aa4 100644 --- a/packages/core/src/agent-operations/batch.ts +++ b/packages/core/src/agent-operations/batch.ts @@ -5,7 +5,7 @@ import type { SceneMaterialId } from '../schema/scene-material' import { applySceneChanges, mergeSceneChanges } from './apply-changes' import { requireFurnishing } from './design-brief' import { referencedSceneMaterialIds } from './scene-materials' -import type { AgentOperation, AgentOperationOutcome, SceneChanges } from './types' +import type { AgentOperation, AgentOperationSteps, SceneChanges } from './types' type BatchInput = { calls: { tool: string; input?: Record }[] @@ -41,23 +41,19 @@ function prepareCall( return { operation, input: parsed.data } } -/** A batch one call at a time: each resume runs the next call, and the last returns the outcome. */ -export type BatchSteps = ( - ...args: Parameters> -) => Generator - /** * `run_batch`: shared tools in order, each on the scene the calls before it left, their changes * merged into one set — one undo step, one write on either surface. The user: the model spent a * round trip per step on what could be serialised (deleting balcony walls one by one, the same * build on every floor). Pre-check and operation refusals follow the same per-call policy. - * It stops after each call, so a host that can let other work run between a batch's calls does - * (`createBatchOperation` runs it through in one go). + * It stops after each call, and where a call that runs in steps (`stepped`) does, so a host that + * can let other work run meanwhile does (`AGENT_OPERATIONS.run_batch` runs it through in one go). */ export function createBatchSteps( operations: Readonly>>, contracts: readonly Contract[], -): BatchSteps { + stepped: Readonly>> = {}, +): AgentOperationSteps { return function* (nodes, { calls, onRefusal = 'stop' }, context) { let working = nodes let workingContext = context @@ -71,10 +67,10 @@ export function createBatchSteps( catalog: workingContext.catalog, }) const { operation, input } = prepareCall(call, index, operations, contracts) - const { result, changes } = operation(working, input as never, { - ...workingContext, - batchCall: index, - }) + const callContext = { ...workingContext, batchCall: index } + const { result, changes } = Object.hasOwn(stepped, call.tool) + ? yield* stepped[call.tool]!(working, input as never, callContext) + : operation(working, input as never, callContext) if (changes) { sets.push(changes) working = applySceneChanges(working, changes) @@ -143,18 +139,3 @@ export function createBatchSteps( } } } - -/** `run_batch` as one synchronous operation: every call, back to back. */ -export function createBatchOperation( - operations: Readonly>>, - contracts: readonly Contract[], -): AgentOperation { - const steps = createBatchSteps(operations, contracts) - return (nodes, input, context) => { - const batch = steps(nodes, input, context) - for (;;) { - const step = batch.next() - if (step.done) return step.value - } - } -} diff --git a/packages/core/src/agent-operations/index.ts b/packages/core/src/agent-operations/index.ts index 31fb35c880..98ce123966 100644 --- a/packages/core/src/agent-operations/index.ts +++ b/packages/core/src/agent-operations/index.ts @@ -1,11 +1,12 @@ import { AGENT_TOOL_CONTRACTS, BATCHABLE_TOOLS } from '../agent-tools' +import { inOneGo } from '../lib/steps' import { addColumn } from './add-column' import { addFence } from './add-fence' import { addLevel } from './add-level' import { addObjectInBatch } from './add-object' import { addWall } from './add-wall' import { viewAssets } from './asset-views' -import { createBatchOperation, createBatchSteps } from './batch' +import { createBatchSteps } from './batch' import { addCornerWindow } from './corner-window' import { createRoof } from './create-roof' import { createRoom } from './create-room' @@ -33,14 +34,15 @@ import { setCheckpoint } from './set-checkpoint' import { setGroundLevel } from './set-ground-level' import { getSite, placeBuilding } from './site' import { fitStair, measureStairOperation } from './stairs' -import type { AgentOperation, AgentOperations } from './types' +import type { AgentOperation, AgentOperationSteps, AgentOperations } from './types' import { createUnit, listUnits, setUnitMembers } from './units' -import { verifyScene } from './verify-scene' +import { verifyScene, verifySceneSteps } from './verify-scene' import { setWallType } from './wall-type' import './room-checks' import './unreachable-rooms-check' import './usage-checks' +export { inOneGo, type Steps } from '../lib/steps' export * from './achieved' export * from './add-column' export * from './add-fence' @@ -164,14 +166,24 @@ const BATCHED_OPERATIONS = Object.fromEntries( BATCHABLE_TOOLS.map((name) => [name, BATCH_OPERATIONS[name]]), ) as Record<(typeof BATCHABLE_TOOLS)[number], AgentOperation> +const STEPPED_OPERATIONS = { verify_scene: verifySceneSteps } +const batchSteps = createBatchSteps(BATCHED_OPERATIONS, AGENT_TOOL_CONTRACTS, STEPPED_OPERATIONS) + /** Each shared agent tool's operation, by tool name: what every surface executes. */ export const AGENT_OPERATIONS = { ...SINGLE_OPERATIONS, - run_batch: createBatchOperation(BATCHED_OPERATIONS, AGENT_TOOL_CONTRACTS), + run_batch: (nodes, input, context) => inOneGo(batchSteps(nodes, input, context)), } as const satisfies AgentOperations -/** `run_batch` one call at a time, for a host that lets other work run between a batch's calls. */ -export const RUN_BATCH_STEPS = createBatchSteps(BATCHED_OPERATIONS, AGENT_TOOL_CONTRACTS) +/** + * The operations that take seconds on a large building, in steps, by tool name: a batch after each + * of its calls, verify_scene after each of its checks. For a host that lets other work run between + * the steps; each answers what its operation in `AGENT_OPERATIONS` answers. + */ +export const AGENT_OPERATION_STEPS: Readonly>> = { + ...STEPPED_OPERATIONS, + run_batch: batchSteps, +} export { type AssetViewFacts, diff --git a/packages/core/src/agent-operations/types.ts b/packages/core/src/agent-operations/types.ts index e9d9159abe..f24e9053a2 100644 --- a/packages/core/src/agent-operations/types.ts +++ b/packages/core/src/agent-operations/types.ts @@ -1,5 +1,6 @@ import type { ZodType } from 'zod' import type { DesignBrief } from '../agent-tools/design-brief' +import type { Steps } from '../lib/steps' import type { AnyNode, Collection, @@ -115,5 +116,14 @@ export type AgentOperation = ( context: AgentContext, ) => AgentOperationOutcome +/** + * An operation that takes seconds on a large building, in steps: a batch's calls, verify_scene's + * checks. A host that can let other work run between them resumes it one step at a time; the + * operation of the same name runs it through (`inOneGo`), with the same outcome. + */ +export type AgentOperationSteps = ( + ...args: Parameters> +) => Steps + /** Operations by tool name: core's `AGENT_OPERATIONS`, or a plugin's `./agent-operations` export. */ export type AgentOperations = Readonly> diff --git a/packages/core/src/agent-operations/usage-checks.ts b/packages/core/src/agent-operations/usage-checks.ts index b72239ad92..729301de3d 100644 --- a/packages/core/src/agent-operations/usage-checks.ts +++ b/packages/core/src/agent-operations/usage-checks.ts @@ -1,5 +1,5 @@ import { isUsageType, ROOM_ENTRY_TYPE, ROOM_FOLDED_TYPES } from '../agent-tools/verify-scene-usage' -import { USAGE_TYPES, type UsageIssue, usageIssues } from '../spatial/usage-field/findings' +import { USAGE_TYPES, usageIssueSteps } from '../spatial/usage-field/findings' import { sdfChecksOn } from '../spatial/usage-field/flag' import { registerSceneCheck } from './verify-scene' @@ -24,5 +24,5 @@ registerSceneCheck({ name: 'usage', order: 60, run: (nodes, input) => - sdfChecksOn() ? (usageIssues(nodes, { types: askedTypes(input.type) }) as UsageIssue[]) : [], + sdfChecksOn() ? usageIssueSteps(nodes, { types: askedTypes(input.type) }) : [], }) diff --git a/packages/core/src/agent-operations/verify-scene-typed.test.ts b/packages/core/src/agent-operations/verify-scene-typed.test.ts index 1f4ba0ec94..042d0cb318 100644 --- a/packages/core/src/agent-operations/verify-scene-typed.test.ts +++ b/packages/core/src/agent-operations/verify-scene-typed.test.ts @@ -13,7 +13,7 @@ import { import { setSdfChecks } from '../spatial/usage-field/flag' import './index' import type { SceneNodes } from './types' -import { verifyScene } from './verify-scene' +import { verifyScene, verifySceneSteps } from './verify-scene' type Pt = [number, number] type Report = { issues: { type: string }[] } @@ -131,6 +131,16 @@ describe('verify_scene asked for one finding type, with the usage checks on', () expect(answer(report(input), input)).toEqual(answer(report({}), input)) }) + test('taken a step at a time, it stops between its checks and answers the same report', () => { + const steps = verifySceneSteps(nodes, {}, context) + let stops = 0 + let step = steps.next() + for (; !step.done; step = steps.next()) stops++ + // More stops than checks: the circulation measurement stops after each floor. + expect(stops).toBeGreaterThan(5) + expect(step.value.result).toEqual(report({})) + }) + test('the report for a type that is not a usage finding holds no usage findings', () => { const typed = report({ type: 'door_blocked' }) as Report expect(typed.issues.filter((issue) => issue.type.startsWith('usage_'))).toEqual([]) diff --git a/packages/core/src/agent-operations/verify-scene.ts b/packages/core/src/agent-operations/verify-scene.ts index cf6fb1476c..1501973d00 100644 --- a/packages/core/src/agent-operations/verify-scene.ts +++ b/packages/core/src/agent-operations/verify-scene.ts @@ -7,6 +7,7 @@ import { HIDDEN_SITE_NOTE } from '../lib/node-visibility' import { wallSupportForNodes } from '../lib/opening-floor-datum' import { detectOpenWallEnds, type OpenWallEnd } from '../lib/room-graph' import { siteBrief } from '../lib/site-placement' +import { inOneGo, type Steps } from '../lib/steps' import { nodeRegistry } from '../registry' import { type AnyNode, @@ -57,7 +58,7 @@ import { queryNodes, } from './scene-queries' import { reviewSince } from './scene-review' -import type { AgentContext, AgentOperation, SceneNodes } from './types' +import type { AgentContext, AgentOperation, AgentOperationSteps, SceneNodes } from './types' import type { RoomUnreachableIssue } from './unreachable-rooms-check' import { type WallEndShortIssue, @@ -120,12 +121,17 @@ export type VerifySceneInput = Readonly> /** * A check verify_scene runs beyond its own, registered by the module that knows it (the facade * checks, the photo's), so verify_scene imports none of them. Checks before CHECKPOINT_ORDER run - * ahead of the checkpoint's comparison, the rest after it. + * ahead of the checkpoint's comparison, the rest after it. A check that takes seconds on a large + * building answers in steps, and verify_scene stops where it does. */ export type SceneCheck = { name: string order: number - run: (nodes: SceneNodes, input: VerifySceneInput, context: AgentContext) => SceneIssue[] + run: ( + nodes: SceneNodes, + input: VerifySceneInput, + context: AgentContext, + ) => SceneIssue[] | Steps } export const CHECKPOINT_ORDER = 100 @@ -336,11 +342,15 @@ function checkSchemas(scene: SceneNodes, hostSchema: AgentContext['nodeSchema']) /** A flight of this many risers or fewer is a step up to a door, not a staircase: no going or comfort target applies. */ const SHORT_FLIGHT_STEPS = 3 -export const verifyScene: AgentOperation = ( +export const verifyScene: AgentOperation = (scene, input, context) => + inOneGo(verifySceneSteps(scene, input, context)) + +/** `verify_scene`, a step for each registered check and for each step of one that has them. */ +export const verifySceneSteps: AgentOperationSteps = function* ( scene, input, context, -) => { +) { const { errors, unknownKinds, unread, measured: nodes } = checkSchemas(scene, context.nodeSchema) const onLevel = (levelId: string) => nodesOnLevel(nodes, levelId) const ofType = (content: readonly AnyNode[], type: T) => @@ -798,8 +808,15 @@ export const verifyScene: AgentOperation = ( issues.push(...layoutIssuesFromScene(Object.values(settled))) issues.push(...ceilingObjectObservations(settled)) issues.push(...wallObjectObservations(settled)) - for (const check of checks) - if (check.order < CHECKPOINT_ORDER) issues.push(...check.run(settled, input ?? {}, context)) + const registered = function* (beforeCheckpoint: boolean): Steps { + for (const check of checks) { + if (check.order < CHECKPOINT_ORDER !== beforeCheckpoint) continue + const found = check.run(settled, input ?? {}, context) + issues.push(...(Array.isArray(found) ? found : yield* found)) + yield + } + } + yield* registered(true) // What the edits since the host's checkpoint lost, by place. const hosted = context.checkpoint ? changesSince(context.checkpoint as SceneCheckpoint, scene) @@ -810,8 +827,7 @@ export const verifyScene: AgentOperation = ( const session = context.session ?? EMPTY_SESSION const since = typeof input?.since === 'string' ? reviewSince(session, input.since, scene) : null if (since?.places) issues.push(...since.places.issues) - for (const check of checks) - if (check.order >= CHECKPOINT_ORDER) issues.push(...check.run(settled, input ?? {}, context)) + yield* registered(false) const occupiedStoryCount = levels.filter((level) => level.isOccupiedStory).length issues.splice(0, issues.length, ...withoutDuplicatePockets(issues)) diff --git a/packages/core/src/lib/steps.ts b/packages/core/src/lib/steps.ts new file mode 100644 index 0000000000..2599ac5ed6 --- /dev/null +++ b/packages/core/src/lib/steps.ts @@ -0,0 +1,13 @@ +/** + * Work in steps: each resume runs the next piece, and the last returns the result. A long + * computation written this way can be resumed one piece at a time by a caller that lets other work + * run in between; `inOneGo` runs it through. + */ +export type Steps = Generator + +export function inOneGo(steps: Steps): Result { + for (;;) { + const step = steps.next() + if (step.done) return step.value + } +} diff --git a/packages/core/src/spatial/usage-field/findings.ts b/packages/core/src/spatial/usage-field/findings.ts index 3ce0431b3a..eae2842589 100644 --- a/packages/core/src/spatial/usage-field/findings.ts +++ b/packages/core/src/spatial/usage-field/findings.ts @@ -3,6 +3,7 @@ import { type ThresholdSource, USAGE_INDICATORS, } from '../../building/code-profile' +import { inOneGo, type Steps } from '../../lib/steps' import type { AnyNode, DoorNode, ItemNode } from '../../schema' import { bedIssues } from './beds' import { blockedOpeningIssues } from './blocked' @@ -13,7 +14,7 @@ import { fixtureClearanceIssues } from './fixtures' import { m, nameOf, quote, unique } from './labels' import { type MeasureOptions, - measureUsage, + measureUsageSteps, type ObstacleRef, type Passage, type RoomMeasure, @@ -298,12 +299,12 @@ function pocketIssue(nodes: Nodes, room: RoomMeasure, from: string): UsageIssue } /** The passages, turning circles and pockets of every group of rooms, measured on one clearance field. */ -function circulationIssues( +function* circulationIssues( nodes: Nodes, shapes: UsageShapes, options: MeasureOptions, -): UsageIssue[] { - const measure = measureUsage(nodes, shapes, options) +): Steps { + const measure = yield* measureUsageSteps(nodes, shapes, options) const issues: UsageIssue[] = [] for (const group of measure.groups) { const from = group.source.kind === 'door' ? 'the entrance' : 'the stair arrival' @@ -346,11 +347,16 @@ function circulationIssues( /** * Each measurement with the finding types it gives, in the order their findings are listed. On a * building of a few hundred rooms the circulation measurement takes seconds and the others a - * fraction of one, so a question about some types takes the measurements that give them alone. + * fraction of one, so a question about some types takes the measurements that give them alone, + * and the circulation measurement is taken in steps. */ const MEASUREMENTS: { types: readonly UsageIssue['type'][] - run: (nodes: Nodes, shapes: UsageShapes, options: MeasureOptions) => UsageIssue[] + run: ( + nodes: Nodes, + shapes: UsageShapes, + options: MeasureOptions, + ) => UsageIssue[] | Steps }[] = [ { types: ['usage_pocket', 'usage_passage_narrow', 'usage_turning_narrow'], @@ -386,11 +392,22 @@ export type UsageOptions = MeasureOptions & { * not a compliance check. */ export function usageIssues(nodes: Nodes, options: UsageOptions = {}): UsageIssue[] { + return inOneGo(usageIssueSteps(nodes, options)) +} + +/** `usageIssues`, a step for each measurement and for each floor of the circulation one. */ +export function* usageIssueSteps(nodes: Nodes, options: UsageOptions = {}): Steps { const { types, ...measure } = options const asked = types ? MEASUREMENTS.filter((measurement) => measurement.types.some((type) => types.has(type))) : MEASUREMENTS if (asked.length === 0) return [] const shapes = usageShapes(nodes) - return asked.flatMap((measurement) => measurement.run(nodes, shapes, measure)) + const issues: UsageIssue[] = [] + for (const measurement of asked) { + const found = measurement.run(nodes, shapes, measure) + issues.push(...(Array.isArray(found) ? found : yield* found)) + yield + } + return issues } diff --git a/packages/core/src/spatial/usage-field/measure.ts b/packages/core/src/spatial/usage-field/measure.ts index 1097ddd465..cffd7d5b9d 100644 --- a/packages/core/src/spatial/usage-field/measure.ts +++ b/packages/core/src/spatial/usage-field/measure.ts @@ -6,6 +6,7 @@ import { floorsOf } from '../../agent-operations/circulation-spaces' import { isFloorUnderlay } from '../../agent-operations/furniture-kind' import { area, difference, union } from '../../lib/polygon-boolean' import { polygonInteriorPoint } from '../../lib/polygon-label' +import { inOneGo, type Steps } from '../../lib/steps' import type { AnyNode, DoorNode, ItemNode, WallNode } from '../../schema' import { resolveStairWalkingSurfaces } from '../../systems/stair/stair-clearance' import { cellX, cellZ, type Grid } from './field' @@ -344,13 +345,13 @@ export function prepareGroup( * shapes `usageShapes` read. The floor is a cell grid of the clearance to the nearest obstacle in a * band a body stands in; a passage is the widest path from the entrance to the room, its pinch * re-solved on a finer grid to the millimetre; a pocket is floor where a 0.60 m unit fits but - * cannot get out. + * cannot get out. One floor a step: a building of many floors takes seconds. */ -export function measureUsage( +export function* measureUsageSteps( nodes: Nodes, shapes: UsageShapes, options: MeasureOptions = {}, -): UsageMeasure { +): Steps { const cell = options.cell ?? 0.02 const floors = floorsOf(nodes).floors const exterior = exteriorDoors(nodes) @@ -571,6 +572,15 @@ export function measureUsage( } }) groups.push({ levelId: group.levelId, y: group.y, source, rooms }) + yield } return { groups, entranceDoorId: entrance } } + +export function measureUsage( + nodes: Nodes, + shapes: UsageShapes, + options: MeasureOptions = {}, +): UsageMeasure { + return inOneGo(measureUsageSteps(nodes, shapes, options)) +} diff --git a/packages/mcp/src/server.ts b/packages/mcp/src/server.ts index a701706c43..1869738634 100644 --- a/packages/mcp/src/server.ts +++ b/packages/mcp/src/server.ts @@ -70,8 +70,8 @@ export type CreatePascalMcpServerOptions = { sceneViews?: SceneViewHost /** Reads catalog items' pictures and surfaces before view_assets and paint. */ assetViews?: PlanReadingHosts['assetViews'] - /** A pause after each call of a run_batch, for a host with a scene of its own per request. */ - betweenBatchCalls?: PlanReadingHosts['betweenBatchCalls'] + /** A pause between the steps of a long call, for a host with a scene of its own per request. */ + betweenSteps?: PlanReadingHosts['betweenSteps'] /** Lines the host adds to what a client reads at connect (the person's own settings). */ instructions?: string } @@ -97,7 +97,7 @@ export function createPascalMcpServer(opts: CreatePascalMcpServerOptions): McpSe readFile: opts.readFile, sceneViews: opts.sceneViews, assetViews: opts.assetViews, - betweenBatchCalls: opts.betweenBatchCalls, + betweenSteps: opts.betweenSteps, }) registerVisionTools(server, operations) if (opts.services) registerHostedServiceTools(server, opts.services) diff --git a/packages/mcp/src/tools/author-scene.test.ts b/packages/mcp/src/tools/author-scene.test.ts index 2fe324a9bf..c6491364e7 100644 --- a/packages/mcp/src/tools/author-scene.test.ts +++ b/packages/mcp/src/tools/author-scene.test.ts @@ -260,18 +260,48 @@ describe('author_scene (MCP)', () => { readArtifact: async ({ sha256 }) => artifacts.get(sha256) ?? null, } - beforeEach(async () => { - commitAllowed = true - stored.length = 0 + /** A client on a server with a scene of its own; `betweenSteps` is the host's pause, if any. */ + const connect = async (betweenSteps?: () => Promise) => { const bridge = new SceneBridge() bridge.setScene({}, [], { materials: {} }) const operations = createSceneOperations({ bridge }) operations.loadDefault() bridge.setActiveScene({ id: 'scene_test', name: 'Test' } as never) - const server = createPascalMcpServer({ operations, geometryScripts: host }) + const server = createPascalMcpServer({ operations, geometryScripts: host, betweenSteps }) const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair() - client = new Client({ name: 'test', version: '0' }) - await Promise.all([server.connect(serverTransport), client.connect(clientTransport)]) + const connected = new Client({ name: 'test', version: '0' }) + await Promise.all([server.connect(serverTransport), connected.connect(clientTransport)]) + return connected + } + + beforeEach(async () => { + commitAllowed = true + stored.length = 0 + client = await connect() + }) + + test('on a host that pauses between steps, its closing checks pause and report the same', async () => { + let pauses = 0 + const paused = await connect(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)) + pauses++ + }) + // The servers of this file share one scene, so each writes over the last conversion. + const gate = async (on: Client) => { + const result = await on.callTool({ + name: 'author_scene', + arguments: { payload, replace: true }, + }) + const { verify, checks, complete, green, walls, rooms } = result.structuredContent as Record< + string, + unknown + > + return { verify, checks, complete, green, walls, rooms } + } + const direct = await gate(client) + expect(pauses).toBe(0) + expect(await gate(paused)).toEqual(direct) + expect(pauses).toBeGreaterThan(1) }) test('a stopped invocation never commits its compiled candidate', async () => { diff --git a/packages/mcp/src/tools/author-scene.ts b/packages/mcp/src/tools/author-scene.ts index ffac214a72..54c192548b 100644 --- a/packages/mcp/src/tools/author-scene.ts +++ b/packages/mcp/src/tools/author-scene.ts @@ -6,9 +6,10 @@ import { type AuthorScenePending, achievedChanges, authorSceneChecks, + inOneGo, type ProjectFileReader, type SceneNodes, - verifyScene, + verifySceneSteps, } from '@pascal-app/core/agent-operations' import { type AuthorScenePayload, @@ -26,7 +27,7 @@ import { compileAndStore, type GeometryScriptHost, type ObjectMakerHost } from ' import { DESTRUCTIVE_TOOL_ANNOTATIONS } from './annotations' import { refusalResult, toolError } from './errors' import { persistencePayload, publishLiveSceneSnapshot } from './live-sync' -import { toPatches } from './shared-tools' +import { pausedBetweenSteps, toPatches } from './shared-tools' type Edit = { in: 'module' | 'adapter'; find: string; replace: string } type Input = { @@ -63,7 +64,8 @@ const CODE_KEYS = new Set(['code', 'floorCode', 'wallCode']) * stored module and adapter, changed by edits, when left out), or the stored conversion again; * items naming a piece written with its module, those still being made left for the host to write * when they land; every module compiled and stored through the host, the operation applied, - * Pascal's checks run, and the gate report answered. + * Pascal's checks run, and the gate report answered. The checks take seconds on a large building: + * a host that gives `betweenSteps` is awaited between them, as in verify_scene. */ export function registerAuthorScene( server: McpServer, @@ -71,6 +73,7 @@ export function registerAuthorScene( host: GeometryScriptHost | undefined, makers?: ObjectMakerHost, readFile?: ProjectFileReader, + betweenSteps?: () => Promise, ): void { server.registerTool( authorSceneTool.name, @@ -227,9 +230,12 @@ export function registerAuthorScene( ) as unknown as typeof outcome.changes, ) if (patches.length) bridge.applyPatch(patches) - const verified = verifyScene(bridge.getNodes() as SceneNodes, undefined, { + const checking = verifySceneSteps(bridge.getNodes() as SceneNodes, undefined, { activeLevelId: null, - }).result as { + }) + const verified = ( + betweenSteps ? await pausedBetweenSteps(checking, betweenSteps) : inOneGo(checking) + ).result as { valid: boolean issues: { type?: string; message?: string; severity?: string }[] } diff --git a/packages/mcp/src/tools/index.ts b/packages/mcp/src/tools/index.ts index 2885f68c20..a72f2080d7 100644 --- a/packages/mcp/src/tools/index.ts +++ b/packages/mcp/src/tools/index.ts @@ -59,18 +59,18 @@ export function registerTools( readFile, sceneViews, assetViews, - betweenBatchCalls, + betweenSteps, }: ToolHosts = {}, ): void { registerGetScene(server, operations) registerSharedTools(server, operations, catalog, { assetViews, - betweenBatchCalls, + betweenSteps, buildObject: (input) => compileObjectOnHost(geometryScripts, operations, input), }) registerAddColumn(server, operations, geometryScripts) registerAddObject(server, operations, geometryScripts, objectMakers) - registerAuthorScene(server, operations, geometryScripts, objectMakers, readFile) + registerAuthorScene(server, operations, geometryScripts, objectMakers, readFile, betweenSteps) registerGetSource(server, operations, geometryScripts) registerMeasure(server, operations) registerViewScene(server, operations, sceneViews) diff --git a/packages/mcp/src/tools/run-batch-pause.test.ts b/packages/mcp/src/tools/paused-steps.test.ts similarity index 72% rename from packages/mcp/src/tools/run-batch-pause.test.ts rename to packages/mcp/src/tools/paused-steps.test.ts index 06d7907a64..f30e53e49d 100644 --- a/packages/mcp/src/tools/run-batch-pause.test.ts +++ b/packages/mcp/src/tools/paused-steps.test.ts @@ -26,10 +26,10 @@ async function served(pause?: (bridge: SceneBridge) => Promise) { bridge.setScene({ [building.id]: building, [level.id]: level }, [building.id] as never) const server = createPascalMcpServer({ bridge, - ...(pause && { betweenBatchCalls: () => pause(bridge) }), + ...(pause && { betweenSteps: () => pause(bridge) }), }) const [serverTransport, clientTransport] = InMemoryTransport.createLinkedPair() - const client = new Client({ name: 'run-batch-pause-test', version: '0.0.0' }) + const client = new Client({ name: 'paused-steps-test', version: '0.0.0' }) await Promise.all([server.connect(serverTransport), client.connect(clientTransport)]) const walls = () => Object.values(bridge.getNodes()).filter((node) => node.type === 'wall').length const batch = async (calls: unknown[], onRefusal?: 'stop' | 'skip') => @@ -37,7 +37,13 @@ async function served(pause?: (bridge: SceneBridge) => Promise) { name: 'run_batch', arguments: { calls, ...(onRefusal && { onRefusal }) }, })) as Result - return { batch, bridge, walls } + const verify = async () => + ( + (await client.callTool({ name: 'verify_scene', arguments: {} })) as { + structuredContent: unknown + } + ).structuredContent + return { batch, bridge, verify, walls } } /** A result without the ids its calls minted. */ @@ -54,7 +60,7 @@ const shape = (result: Result) => ({ })), }) -describe("run_batch on a host that pauses between a batch's calls", () => { +describe('on a host that pauses between the steps of a long call', () => { test('pauses after each call, with the scene as the batch found it, then applies the batch whole', async () => { const seen: number[] = [] const host = await served(async () => { @@ -70,7 +76,26 @@ describe("run_batch on a host that pauses between a batch's calls", () => { expect(result.structuredContent).toMatchObject({ status: 'applied', applied: 4, refused: 0 }) }) - test('answers as a batch run in one go does: in order, stopped by a refusal, or past it', async () => { + test('verify_scene pauses between its checks, alone and as a call of a batch, with the same answer', async () => { + let pauses = 0 + const paused = await served(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)) + pauses++ + }) + const direct = await served() + expect(await paused.verify()).toEqual(await direct.verify()) + const checks = pauses + expect(checks).toBeGreaterThan(1) + + pauses = 0 + const calls = [wall(0), { tool: 'verify_scene' }] + const [a, b] = [await paused.batch(calls), await direct.batch(calls)] + // The batch's own two pauses, and the check's inside its call. + expect(pauses).toBe(checks + calls.length) + expect(shape(a)).toEqual(shape(b)) + }) + + test('a batch answers as one run in one go does: in order, stopped by a refusal, or past it', async () => { const batches: [unknown[], ('stop' | 'skip')?][] = [ [[wall(0), { tool: 'get_walls' }, wall(2)]], [[wall(0), missing, wall(2)]], diff --git a/packages/mcp/src/tools/shared-tools.ts b/packages/mcp/src/tools/shared-tools.ts index ca31329fc2..c5f8318fa7 100644 --- a/packages/mcp/src/tools/shared-tools.ts +++ b/packages/mcp/src/tools/shared-tools.ts @@ -1,6 +1,7 @@ import type { McpServer } from '@modelcontextprotocol/server' import { type AddObjectInput, + AGENT_OPERATION_STEPS, AGENT_OPERATIONS, type AgentContext, type AgentHostRuntime, @@ -13,9 +14,9 @@ import { createAssetViewLoader, createFetchAssetViewIO, furnishingRefusal, - RUN_BATCH_STEPS, readCatalog, type SceneChanges, + type Steps, startOfSession, writeCollections, } from '@pascal-app/core/agent-operations' @@ -121,12 +122,13 @@ export type PlanReadingHosts = { /** Compiles one add_object module for a batch before the synchronous operations run. */ buildObject?: (input: Omit) => Promise /** - * Awaited after each call of a run_batch, so the host's timers and I/O run during a long one. - * Only for a host whose scene no other call can reach meanwhile (a bridge of its own for each - * request): the batch is planned on the scene as its first call found it and applied after its - * last. Without it a batch runs uninterrupted. + * Awaited between the steps of a call that takes seconds on a large building (after each call of + * a run_batch, after each check of a verify_scene, author_scene's closing one included), so the + * host's timers and I/O run meanwhile. Only for a host whose scene no other call can reach then + * (a bridge of its own for each request): a batch is planned on the scene as its first call + * found it and applied after its last. Without it these calls run uninterrupted. */ - betweenBatchCalls?: () => Promise + betweenSteps?: () => Promise } const defaultAssetViews = createAssetViewLoader(createFetchAssetViewIO()) @@ -719,14 +721,15 @@ function modelResult( return output } -/** A batch one call at a time, each in the scene's scope, the host's pause after each. */ -async function runBatchInSteps( - bridge: SceneOperations, - batch: ReturnType, +/** Work one step at a time, the host's pause after each; `scope` is where each step runs. */ +export async function pausedBetweenSteps( + steps: Steps, pause: () => Promise, -) { + scope: (step: () => IteratorResult) => IteratorResult = (step) => + step(), +): Promise { for (;;) { - const step = inScene(bridge, () => batch.next()) + const step = scope(() => steps.next()) if (step.done) return step.value await pause() } @@ -827,12 +830,16 @@ export function registerOperationTool( pictures: needs.pictures, }) try { + const steps = + hosts.betweenSteps && Object.hasOwn(AGENT_OPERATION_STEPS, tool.contract.name) + ? AGENT_OPERATION_STEPS[tool.contract.name] + : undefined outcome = - tool.contract.name === runBatchTool.name && hosts.betweenBatchCalls - ? await runBatchInSteps( - bridge, - RUN_BATCH_STEPS(nodes, input as never, context), - hosts.betweenBatchCalls, + steps && hosts.betweenSteps + ? await pausedBetweenSteps( + steps(nodes, input as never, context), + hosts.betweenSteps, + (step) => inScene(bridge, step), ) : inScene(bridge, () => tool.operation(nodes, input as never, context)) } catch (error) { diff --git a/wiki/architecture/agent-surfaces.md b/wiki/architecture/agent-surfaces.md index 6b9e617ce7..88209079c6 100644 --- a/wiki/architecture/agent-surfaces.md +++ b/wiki/architecture/agent-surfaces.md @@ -117,7 +117,7 @@ Views are planned on the site: a building's position and yaw move and turn the b **Objects in a batch.** `run_batch` runs `add_object` calls. A compile is async and the operations are sync, so the host builds every permitted add_object module first (`buildBatchObjects`, three compiles at a time, with the same compile its lone `add_object` uses) and hands the results to the batch in `AgentContext.batchObjects`; `addObjectInBatch` places each, with the receipt a lone call gives (`achieved` per object). `add_object` is in `INDEPENDENT_BATCH_TOOLS`: a module that fails (`script_failed`), a missing reason, an invalid input or a refused placement is that call's own report, whatever `onRefusal` says, and the other objects are applied. Every other tool's refusal follows `onRefusal`: `stop` (default) applies nothing, `skip` reports the refusal and applies the other calls. Unsupported tools, invalid inputs and brief refusals follow this same policy. A host that built nothing answers `object_not_built` (the chat builds through `executeRunBatch`, the MCP through `registerOperationTool`'s `buildObject` host). -**A pause between a batch's calls.** A batch is one synchronous operation: its calls run back to back, and its changes are applied in the same stretch, so nothing else on the server runs meanwhile and no other call can reach the scene between planning and applying. A long batch (several whole-scene checks on a large building) therefore holds the event loop for its whole length, and a host's timers do not fire. A host that builds a scene of its own for each request gives `betweenBatchCalls` (`createPascalMcpServer`), and the MCP then runs the batch one call at a time (`RUN_BATCH_STEPS`, each step in the scene's scope) and awaits the host after each: same order, same refusal policy, same result, one history step. A host that serves concurrent calls on one bridge leaves it out and keeps the uninterrupted batch. +**A pause between the steps of a long call.** An operation is synchronous: a batch's calls run back to back and its changes are applied in the same stretch, and `verify_scene` runs every check in one go. Nothing else on the server runs meanwhile, so no other call can reach the scene between planning and applying, and a host's timers do not fire: several whole-scene checks on a large building hold the event loop for their whole length, and so does `author_scene`, which ends with those checks. The operations that take seconds on a large building are therefore also written in steps (`AGENT_OPERATION_STEPS`, `Steps` in `core/src/lib/steps.ts`): `run_batch` stops after each of its calls, and `verify_scene` after each of its checks and, in the circulation measurement, after each floor (a batch stops inside a `verify_scene` call too). `AGENT_OPERATIONS` runs the same steps through in one go (`inOneGo`), so the answer is the same either way. A host that builds a scene of its own for each request gives `betweenSteps` (`createPascalMcpServer`), and the MCP then resumes these operations one step at a time (each step in the scene's scope) and awaits the host after each, in `run_batch`, `verify_scene` and `author_scene`'s closing checks: same order, same refusal policy, same result, one history step. A host that serves concurrent calls on one bridge leaves it out and keeps the uninterrupted call. A registered check (`registerSceneCheck`) that takes seconds answers in steps too, by returning a generator. **Inside a floor.** `interior: { levelId }` looks into one floor. The plan checks the level (`interior_level_invalid`, also for a level holding only roofs and ceilings; `interior_hidden` for a hidden level or ancestor, a hidden site excepted, since it hides only its ground; `interior_transform_unsupported` for a tipped building) and that a target is on it and in the picture (`interior_target_elsewhere`, also for its roofs and ceilings), frames the floor from 55° above unless a target, an eye or a camera is given, and hands the host only the level id. The editor that renders it works out the rest from its own scene with `sceneViewInterior`: for that one render the other floors and this floor's roofs and ceilings are hidden, everything on the floor (walls, openings, furniture) and its shadows are cut at 0.65 of the storey height, its walls are drawn whole whatever the user's wall mode, and the user's solo and isolation are lifted on it. Everything is put back right after the render, so the live view does not change.