diff --git a/scripts/run-batch-benchmark.ts b/scripts/run-batch-benchmark.ts index c753f1605..4fdb13b0d 100644 --- a/scripts/run-batch-benchmark.ts +++ b/scripts/run-batch-benchmark.ts @@ -69,6 +69,12 @@ import { pathToFileURL } from 'node:url'; import { createHarness, rawText, isError, parseToolResponse } from '../tests/e2e/harness.js'; import type { SessionFile } from '../src/schema/session.schema.js'; import { deliveredChars } from '../src/utils/batch.js'; +import { measureFanOut, fanOutRatios, fanOutLines } from '../src/utils/fan-out.js'; +import { loadWorkflowWithDiagnostics } from '../src/loaders/workflow-loader.js'; +import { composeActivityTechnique } from '../src/loaders/technique-loader.js'; +import { flattenActivitySteps, techniqueName } from '../src/schema/activity.schema.js'; +import type { Technique } from '../src/schema/technique.schema.js'; +import { requireWorkflowsRoot } from './workflows-root.js'; /** The analysis run through the middle of the main workflow — the best measured batch candidate. */ export const DEFAULT_RUN = ['implementation-analysis', 'plan-prepare', 'assumptions-review']; @@ -161,6 +167,33 @@ async function walk( } } +/** + * Compose every step-bound operation of the walked run and measure how much of each delivery is + * content declared above it (#404 W6). Reported warn-only: a container rule is meant to be + * cross-cutting, so a low reach figure describes the design rather than faulting it. + */ +async function measureRunFanOut(workflowId: string, activities: string[]) { + const root = requireWorkflowsRoot(join(import.meta.dirname, '..', 'workflows')); + const loaded = await loadWorkflowWithDiagnostics(root, workflowId); + if (!loaded.success) throw loaded.error; + const { workflow, activitySourceWorkflow } = loaded.value; + + const composed: Technique[] = []; + for (const activityId of activities) { + const activity = workflow.activities?.find((a) => a.id === activityId); + if (!activity) continue; + const scopeWorkflowId = activitySourceWorkflow.get(activityId) ?? workflowId; + for (const step of flattenActivitySteps(activity)) { + if (step.kind !== 'technique') continue; + const ref = techniqueName(step.technique); + if (!ref) continue; + const result = await composeActivityTechnique(ref, root, scopeWorkflowId, activityId); + if (result.success) composed.push(result.value.technique); + } + } + return measureFanOut(composed); +} + /** Walk one pass `repeat` times and keep the best elapsed, so a cold FS cache does not dominate. */ export async function measure( mode: PassMetrics['mode'], @@ -184,6 +217,7 @@ async function main(): Promise { const perActivity = await measure('per-activity', { workflowId, activities, contextTokens, repeat }); const batched = await measure('batched', { workflowId, activities, contextTokens, repeat }); + const fanOut = await measureRunFanOut(workflowId, activities); const charSavingPct = perActivity.deliveredChars === 0 ? 0 @@ -222,6 +256,10 @@ async function main(): Promise { spawnSecondsInput: spawnSeconds, runDurationSavingSeconds: Number((dispatchesAvoided * spawnSeconds).toFixed(1)), }, + // Warn-only, and nothing gates on it. Container rules and inherited I/O are cross-cutting by + // design, so a reach figure describes what the corpus intends rather than faulting it; the + // figures are here so the fan-out is visible and a regression is arguable. + fanOut: { ...fanOut, ...fanOutRatios(fanOut) }, }; process.stdout.write(JSON.stringify(report, null, 2) + '\n'); @@ -236,6 +274,7 @@ async function main(): Promise { ` projected run duration: ${report.projected.runDurationSavingSeconds}s saved, from ` + `${report.projected.basis} — supply your harness's own --spawn-seconds to re-base it\n`, ); + for (const line of fanOutLines(fanOut)) process.stderr.write(`${line}\n`); if (has('gate') && charSavingPct < minSavingPct) { process.stderr.write(`GATE FAIL: batched saving ${report.measured.charSavingPct}% is below ${minSavingPct}%\n`); diff --git a/src/tools/resource-tools.ts b/src/tools/resource-tools.ts index 1db6f8f64..d0252e7e5 100644 --- a/src/tools/resource-tools.ts +++ b/src/tools/resource-tools.ts @@ -1,7 +1,7 @@ import { z } from 'zod'; import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import { normalizeRepoPath, presentPathToAgent, type ServerConfig } from '../config.js'; -import { withAuditLog } from '../logging.js'; +import { withAuditLog, logInfo } from '../logging.js'; import { loadWorkflow, loadWorkflowWithDiagnostics, getActivity } from '../loaders/workflow-loader.js'; import { readResourceStructured } from '../loaders/resource-loader.js'; @@ -45,7 +45,7 @@ import { type SessionFile, } from '../schema/session.schema.js'; import { techniqueName, flattenActivitySteps, type Step } from '../schema/activity.schema.js'; -import { buildProvenanceContext, decorateTechniqueProvenance } from '../utils/binding-provenance.js'; +import { buildProducerIndex, provenanceContextFor, decorateTechniqueProvenance } from '../utils/binding-provenance.js'; import { seedDefaults } from '../utils/variable-seed.js'; import { buildValidation, validateWorkflowVersion } from '../utils/validation.js'; import { stringifyForResponse } from '../utils/serialization.js'; @@ -686,14 +686,15 @@ export function registerResourceTools(server: McpServer, config: ServerConfig): // collapsing under reference delivery. let technique = composed.value.technique; const provenanceWarnings: string[] = []; + let resolvedTechniques = 0; if (boundStep?.id && state.currentActivity) { - const ctx = await buildProvenanceContext({ + const producerIndex = await buildProducerIndex({ workflow: wfResult.value, workflowDir: config.workflowDir, - currentActivityId: state.currentActivity, - currentStepId: boundStep.id, activitySourceWorkflow: wfDiag.value.activitySourceWorkflow, }); + resolvedTechniques = producerIndex.resolvedTechniques; + const ctx = provenanceContextFor(producerIndex, state.currentActivity, boundStep.id); if (ctx) { const binding = boundStep.kind === 'technique' && typeof boundStep.technique === 'object' ? boundStep.technique @@ -769,6 +770,11 @@ export function registerResourceTools(server: McpServer, config: ServerConfig): }); await saveSessionForTool(loaded, next); + logInfo('Technique delivery cost', { + session_index, technique: techniqueId, agentId: scope, delivery: 'unchanged', + resolved_techniques: resolvedTechniques, composed_chars: text.length, response_chars: 0, + }); + // Canonical unchanged-marker: { delivery: 'unchanged', content_hash } — // the same shape the get_activity bundle path emits (delivery.ts#unchangedMarker). // The technique id and note ride alongside as sibling context. @@ -800,6 +806,15 @@ export function registerResourceTools(server: McpServer, config: ServerConfig): }); await saveSessionForTool(loaded, next); + // What this fetch cost to build and to send. `resolved_techniques` is the distinct bound ops the + // producer scan read to decorate one step, which is the resolve work a lazy fetch pays; the two + // character figures are the composed technique and what the response carried after any shared + // block collapsed. + logInfo('Technique delivery cost', { + session_index, technique: techniqueId, agentId: scope, delivery: 'full', + resolved_techniques: resolvedTechniques, composed_chars: text.length, response_chars: body.length, + }); + return { content: [{ type: 'text' as const, text: `session_index: ${session_index}\n\n${body}` }], _meta: { session_index, validation }, @@ -891,6 +906,11 @@ export function registerResourceTools(server: McpServer, config: ServerConfig): }); await saveSessionForTool(loaded, next); + logInfo('Resource delivery cost', { + session_index, resource: resource_id, agentId: scope, delivery: 'unchanged', + resource_chars: fullText.length, response_chars: 0, + }); + const stub = stringifyForResponse({ resource_id, ...unchangedMarker(hash), @@ -909,6 +929,11 @@ export function registerResourceTools(server: McpServer, config: ServerConfig): }); await saveSessionForTool(loaded, next); + logInfo('Resource delivery cost', { + session_index, resource: resource_id, agentId: scope, delivery: 'full', + resource_chars: fullText.length, response_chars: fullText.length, + }); + return { content: [{ type: 'text' as const, text: fullText }], _meta: { session_index, validation }, diff --git a/src/tools/workflow-tools.ts b/src/tools/workflow-tools.ts index b9c131dd5..9a0a2e58b 100644 --- a/src/tools/workflow-tools.ts +++ b/src/tools/workflow-tools.ts @@ -14,8 +14,8 @@ import { resolveTechniques, formatTechniqueBundle, composeActivityTechnique, pro import { CORE_ORCHESTRATOR_TECHNIQUES, CORE_WORKER_TECHNIQUES } from '../loaders/core-ops.js'; import { readResourceRaw } from '../loaders/resource-loader.js'; import { injectResolvedStepIds, techniqueName, flattenActivitySteps, type Activity, type Step } from '../schema/activity.schema.js'; -import { buildProvenanceContext, decorateTechniqueProvenance } from '../utils/binding-provenance.js'; -import { withAuditLog, logWarn } from '../logging.js'; +import { buildProducerIndex, provenanceContextFor, decorateTechniqueProvenance } from '../utils/binding-provenance.js'; +import { withAuditLog, logInfo, logWarn } from '../logging.js'; import { applyVariableWrites } from '../utils/variable-seed.js'; import { stringifyForResponse } from '../utils/serialization.js'; import { contentHash, deliveredHash, dedupTechniqueBlocks, deliveryScope, recordDeliveries, unchangedMarker } from '../utils/delivery.js'; @@ -394,7 +394,11 @@ export function registerWorkflowTools(server: McpServer, config: ServerConfig): if (bootstrapResult.success) { lines.push('', bootstrapResult.value.content); } - return { content: [{ type: 'text' as const, text: lines.join('\n') }] }; + const text = lines.join('\n'); + // The first content of a session, and fixed: the same characters every run. It reports on the + // same channel as every other delivery so the bootstrap window is summable from the log alone. + logInfo('Bootstrap delivery cost', { delivery: 'full', response_chars: text.length }); + return { content: [{ type: 'text' as const, text }] }; })); server.tool('list_workflows', 'List available workflows (id, title, version, tags). On load failures returns `{workflows, load_errors}`. No session_index required.', {}, @@ -492,6 +496,17 @@ export function registerWorkflowTools(server: McpServer, config: ServerConfig): planning_folder_path: presentPlanningPath(loaded.folderAbsPath) ?? loaded.folderAbsPath, }; + // What the orchestrator's own delivery cost. This is the largest fixed payload of a session — + // the same operations bundle every run, read before the first decision — so it reports on the + // same channel as the worker-facing deliveries rather than being the one call that says nothing. + logInfo('Workflow delivery cost', { + session_index, workflow: workflow_id, agentId: state.agentId, + delivery: opsBlock === opsText ? 'full' : 'unchanged', + resolved_techniques: orchestratorTechniques.length, + bundle_chars: opsText.length, + response_chars: preamble.length + stringifyForResponse(summaryData).length, + }); + return { content: [{ type: 'text' as const, text: preamble + stringifyForResponse(summaryData) }], _meta: { session_index, validation }, @@ -909,6 +924,16 @@ export function registerWorkflowTools(server: McpServer, config: ServerConfig): const headroomFraction = config.bundleHeadroomFraction ?? DEFAULT_BUNDLE_HEADROOM_FRACTION; const charsPerToken = config.bundleCharsPerToken ?? DEFAULT_BUNDLE_CHARS_PER_TOKEN; const eagerBudgetChars = context_tokens * headroomFraction * charsPerToken; + // Provenance resolve work, done once for the whole delivery. The producer scan reads every + // bound op in the workflow to learn its declared outputs, and its answer does not vary with + // the step being decorated — only the step's document-order position does. One index therefore + // serves every inlined step, so a delivery resolves each unique technique once however many + // steps it carries. + let producerIndex: Awaited> | undefined; + // Running total of full-content characters committed to the eager bundle. An unchanged-reference + // marker costs effectively nothing, so it never draws down the budget; only full-content + // entries do. Held here so the delivery's cost line can report it against the budget. + let spentChars = 0; if (!optedOut && result.success && activity) { const eligible: Array = []; const collectUngated = (steps: Step[] | undefined): void => { @@ -920,10 +945,14 @@ export function registerWorkflowTools(server: McpServer, config: ServerConfig): }; collectUngated((activity as Activity).steps); - // Running total of full-content characters already committed to the eager bundle. An - // unchanged-reference marker costs effectively nothing, so it never draws down the budget; - // only full-content entries do. - let spentChars = 0; + if (eligible.length > 0) { + producerIndex = await buildProducerIndex({ + workflow: result.value, + workflowDir: config.workflowDir, + activitySourceWorkflow, + }); + } + for (const step of eligible) { const ref = techniqueName(step.technique); if (!ref) continue; @@ -936,13 +965,9 @@ export function registerWorkflowTools(server: McpServer, config: ServerConfig): const { techniqueId } = composedStep.value; let technique = composedStep.value.technique; let provenanceWarnings: string[] = []; - const ctx = await buildProvenanceContext({ - workflow: result.value, - workflowDir: config.workflowDir, - currentActivityId: activity_id, - currentStepId: step.id!, - activitySourceWorkflow, - }); + const ctx = producerIndex + ? provenanceContextFor(producerIndex, activity_id, step.id!) + : null; if (ctx) { const binding = typeof step.technique === 'object' ? step.technique : undefined; const decorated = decorateTechniqueProvenance(technique, ctx, binding, techniqueId, step.id!); @@ -1248,6 +1273,24 @@ export function registerWorkflowTools(server: McpServer, config: ServerConfig): may_continue: stand.mayContinue, }; + // What this delivery cost to build and to send, on one line. `resolved_techniques` is the + // distinct bound ops the producer scan read for the whole request and `provenance_passes` the + // steps decorated from that one scan, so the two together say whether resolve work is being + // repeated. `spent_chars` against `eager_budget_chars` is what the bundle drew down; the + // response length is what actually went over the wire, which is larger by the activity body + // and smaller than the sum of everything named where content collapsed to markers. + logInfo('Activity delivery cost', { + session_index, activity: activity_id, agentId: scope, delivery: referenceMode ? 'reference' : 'full', + resolved_techniques: producerIndex?.resolvedTechniques ?? 0, + provenance_passes: bundledSteps.length, + bundled_steps: bundledSteps.length, + bundled_steps_collapsed: bundledSteps.filter((b) => b.delivery === 'unchanged').length, + bundled_resources: bundledResourceDeliveries.length, + spent_chars: spentChars, + eager_budget_chars: Math.floor(eagerBudgetChars), + response_chars: responseText.length, + }); + return { content: [{ type: 'text' as const, text: responseText }], _meta: { diff --git a/src/utils/binding-provenance.ts b/src/utils/binding-provenance.ts index e85171eeb..ed2ed24a6 100644 --- a/src/utils/binding-provenance.ts +++ b/src/utils/binding-provenance.ts @@ -76,35 +76,52 @@ export interface ProvenanceContext { position: number; } +/** + * The producer scan's result, held for the lifetime of one request. + * + * The scan reads every bound op in the workflow to learn which outputs it declares, and neither the + * producer list nor the variable set depends on which step is being decorated — only `position` + * does. So one index serves every step of a request: a delivery that inlines several steps resolves + * each unique technique once, however many steps it carries. + */ +export interface ProducerIndex { + declaredVariables: Set; + producers: ProducerSite[]; + /** Document-order position of `activityId`/`stepId`, or -1 where the step is not in the workflow. */ + positions: Map; + /** Distinct technique refs resolved to build this index — what a delivery reports as resolve work. */ + resolvedTechniques: number; +} + /* ------------------------------- context assembly ------------------------------- */ /** * Walk the whole workflow (activities in declared order, steps flattened in document order) and * collect every producer site: technique-step outputs (the bound op's own declared outputs, or the * step-binding remap targets), checkpoint-effect `setVariable` keys, `action: set` targets, and - * loop variables. Returns null when the current step cannot be located. + * loop variables. * * Bound-op signatures are read uncomposed (`readTechnique`, memoized per activity+ref): a producer * claim belongs to the op's own file, not to root/group contract entries every sibling would then * appear to produce. Reads are best-effort — a malformed sibling op never fails the fetch, but is * logged, since it silently degrades the provenance of the technique being delivered. + * + * Build this once per request and pass it to `provenanceContextFor` per step. */ -export async function buildProvenanceContext(args: { +export async function buildProducerIndex(args: { workflow: Workflow; workflowDir: string; - currentActivityId: string; - currentStepId: string; /** * Per-activity technique-resolution scope: activity id → the workflow the activity file was * authored in. A borrowed cross-workflow activity resolves its bound ops against its source * workflow (mirroring fragment scoping); absent entries fall back to the session workflow. */ activitySourceWorkflow?: ReadonlyMap; -}): Promise { - const { workflow, workflowDir, currentActivityId, currentStepId, activitySourceWorkflow } = args; +}): Promise { + const { workflow, workflowDir, activitySourceWorkflow } = args; const declaredVariables = new Set((workflow.variables ?? []).map((v) => v.name)); const producers: ProducerSite[] = []; - let position = -1; + const positions = new Map(); let ordinal = 0; const ownOutputsCache = new Map(); @@ -137,7 +154,7 @@ export async function buildProvenanceContext(args: { for (const step of flattenActivitySteps(activity)) { const at = ordinal++; const stepId = step.id ?? (step.kind === 'technique' ? techniqueName(step.technique) : undefined) ?? '?'; - if (activity.id === currentActivityId && step.id === currentStepId) position = at; + if (step.id !== undefined) positions.set(positionKey(activity.id, step.id), at); const push = (name: string, via: ProducerSite['via'], origOutputId?: string): void => { producers.push({ name, via, origOutputId, stepId, activityId: activity.id, ordinal: at }); @@ -171,8 +188,43 @@ export async function buildProvenanceContext(args: { } } - if (position < 0) return null; - return { declaredVariables, producers, position }; + return { declaredVariables, producers, positions, resolvedTechniques: ownOutputsCache.size }; +} + +/** Positions are keyed by the pair, since one step id can occur in more than one activity. */ +function positionKey(activityId: string, stepId: string): string { + return `${activityId}|${stepId}`; +} + +/** + * The classifier's view of one step, taken from an index the request already holds. Null where the + * step is not in the workflow — a read whose position cannot be placed has no before and after, so + * there is nothing to classify. + */ +export function provenanceContextFor( + index: ProducerIndex, + currentActivityId: string, + currentStepId: string, +): ProvenanceContext | null { + const position = index.positions.get(positionKey(currentActivityId, currentStepId)); + if (position === undefined) return null; + return { declaredVariables: index.declaredVariables, producers: index.producers, position }; +} + +/** + * One step's provenance context, for a caller decorating a single step. A caller decorating several + * steps of one request builds the index once with `buildProducerIndex` and reads each step's + * position out of it, rather than scanning the corpus per step. + */ +export async function buildProvenanceContext(args: { + workflow: Workflow; + workflowDir: string; + currentActivityId: string; + currentStepId: string; + activitySourceWorkflow?: ReadonlyMap; +}): Promise { + const index = await buildProducerIndex(args); + return provenanceContextFor(index, args.currentActivityId, args.currentStepId); } /* --------------------------------- classification --------------------------------- */ diff --git a/src/utils/fan-out.ts b/src/utils/fan-out.ts new file mode 100644 index 000000000..bd0f1451b --- /dev/null +++ b/src/utils/fan-out.ts @@ -0,0 +1,131 @@ +import type { Technique } from '../schema/technique.schema.js'; + +/** + * Fan-out: how much of a delivered operation is content declared somewhere above it (#404 W6). + * + * Two things ride along with every operation inside a container. Rules declared on a root or group + * `TECHNIQUE.md` reach every operation in that container, and inherited I/O entries reach every + * operation that composes the contract. Both are cross-cutting by design, so a low share here is not + * a defect and nothing gates on these figures — they are reported so the fan-out is visible and a + * regression is arguable. + * + * Reported as warn-only figures beside `bench:batch`. A threshold would fail the corpus on its + * intended design: a container rule is *meant* to apply to operations that do not name it. + */ + +/** One measurement over a set of delivered operations. */ +export interface FanOutMetrics { + /** Composed operations measured. */ + operations: number; + /** Rule entries delivered across them, counting one per entry per operation that receives it. */ + ruleEntries: number; + /** Characters those entries account for. */ + ruleChars: number; + /** Entries whose text names the operation it arrives with. */ + ruleEntriesNamingTheirOperation: number; + /** Inherited input and output items delivered, counted per operation that receives them. */ + inheritedIoItems: number; + /** Characters those items account for, note text included. */ + inheritedIoChars: number; + /** Inherited items whose id the receiving operation's protocol templates as `{id}`. */ + inheritedIoItemsTemplated: number; +} + +const EMPTY: FanOutMetrics = { + operations: 0, + ruleEntries: 0, + ruleChars: 0, + ruleEntriesNamingTheirOperation: 0, + inheritedIoItems: 0, + inheritedIoChars: 0, + inheritedIoItemsTemplated: 0, +}; + +/** Every prose surface of an operation a rule could plausibly be about. */ +function operationText(technique: Technique): string { + const protocol = (technique.protocol ?? []) + .flatMap((block) => [block.title ?? '', ...block.steps]) + .join('\n'); + return `${technique.capability}\n${protocol}`; +} + +/** + * The names an operation answers to: its full id, and the last segment of a `group::op` or `group/op` + * path, which is how a sibling rule refers to it. + */ +function operationNames(technique: Technique): string[] { + const id = technique.id; + const tail = id.split(/::|\//).pop() ?? id; + return tail === id ? [id] : [id, tail]; +} + +/** Accumulate one composed operation into a running measurement. */ +export function measureOperation(technique: Technique, into: FanOutMetrics = { ...EMPTY }): FanOutMetrics { + const names = operationNames(technique); + const text = operationText(technique); + + let ruleEntries = 0; + let ruleChars = 0; + let naming = 0; + for (const [key, value] of Object.entries(technique.rules ?? {})) { + const entries = Array.isArray(value) ? value : [value]; + for (const entry of entries) { + ruleEntries += 1; + ruleChars += key.length + entry.length; + // A rule is about this operation when it names it, or when the operation's own prose names the + // rule's key — the two ways the corpus ties a rule to the work it governs. + if (names.some((name) => entry.includes(name)) || text.includes(key)) naming += 1; + } + } + + let ioItems = 0; + let ioChars = 0; + let templated = 0; + for (const block of [technique.inherited_inputs, technique.inherited_outputs]) { + if (!block) continue; + ioChars += block.note.length; + for (const item of block.items) { + ioItems += 1; + ioChars += item.id.length + (item.description?.length ?? 0); + if (text.includes(`{${item.id}}`)) templated += 1; + } + } + + return { + operations: into.operations + 1, + ruleEntries: into.ruleEntries + ruleEntries, + ruleChars: into.ruleChars + ruleChars, + ruleEntriesNamingTheirOperation: into.ruleEntriesNamingTheirOperation + naming, + inheritedIoItems: into.inheritedIoItems + ioItems, + inheritedIoChars: into.inheritedIoChars + ioChars, + inheritedIoItemsTemplated: into.inheritedIoItemsTemplated + templated, + }; +} + +/** Fold a set of composed operations into one measurement. */ +export function measureFanOut(techniques: readonly Technique[]): FanOutMetrics { + return techniques.reduce((acc, t) => measureOperation(t, acc), { ...EMPTY }); +} + +/** The two ratios, as percentages, with zero denominators reported as zero rather than as NaN. */ +export function fanOutRatios(m: FanOutMetrics): { ruleReachPct: number; inheritedIoReachPct: number } { + const pct = (part: number, whole: number): number => + whole === 0 ? 0 : Number(((part / whole) * 100).toFixed(1)); + return { + ruleReachPct: pct(m.ruleEntriesNamingTheirOperation, m.ruleEntries), + inheritedIoReachPct: pct(m.inheritedIoItemsTemplated, m.inheritedIoItems), + }; +} + +/** One warn-only line per ratio, for a benchmark run to print beside its measured figures. */ +export function fanOutLines(m: FanOutMetrics): string[] { + const { ruleReachPct, inheritedIoReachPct } = fanOutRatios(m); + const perOp = (chars: number): number => (m.operations === 0 ? 0 : Math.round(chars / m.operations)); + return [ + ` fan-out (warn-only, nothing gates on these): ${m.operations} operations composed`, + ` container rules: ${m.ruleChars} chars over ${m.ruleEntries} entries, ` + + `${ruleReachPct}% naming the operation they arrive with (${perOp(m.ruleChars)} chars an operation)`, + ` inherited I/O: ${m.inheritedIoChars} chars over ${m.inheritedIoItems} items, ` + + `${inheritedIoReachPct}% templated by the receiving protocol (${perOp(m.inheritedIoChars)} chars an operation)`, + ]; +} diff --git a/tests/bootstrap-budget.test.ts b/tests/bootstrap-budget.test.ts new file mode 100644 index 000000000..6959412fc --- /dev/null +++ b/tests/bootstrap-budget.test.ts @@ -0,0 +1,79 @@ +/** + * Bootstrap-time fixed content stays inside the budget the protocol states (#404 W4). + * + * Before an orchestrator makes any decision it reads a fixed block: the bootstrap text `discover` + * returns, the session-start response, and the operations bundle `get_workflow` delivers. Those are + * the same characters on every run, so their size is a property of the corpus and the server rather + * than of a session. + * + * The budget lives in the bootstrap protocol text, which is the one place an orchestrator reads it, + * and this test parses it from there. A figure restated here would be a second home free to drift + * from the one the agent is told. + */ +import { describe, it, expect } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { corpusRoot } from './corpus-root.js'; +import { createHarness, rawText, isError, parseToolResponse } from './e2e/harness.js'; + +/** Where the budget is stated: `… together under 110,000 characters`. */ +const BUDGET_RE = /together under ([\d,]+) characters/; + +function statedBudget(): number { + const path = join(corpusRoot(), 'meta', 'resources', 'bootstrap-protocol.md'); + const text = readFileSync(path, 'utf8'); + const hit = BUDGET_RE.exec(text); + expect(hit, `bootstrap-protocol.md states no fixed-content budget matching ${BUDGET_RE}`).not.toBeNull(); + return Number(hit![1]!.replace(/,/g, '')); +} + +describe('bootstrap-time fixed content', () => { + it('stays inside the budget the bootstrap protocol states', async () => { + const budget = statedBudget(); + const h = await createHarness(); + try { + const discovered = await h.client.callTool({ name: 'discover', arguments: {} }); + expect(isError(discovered)).toBe(false); + + const started = await h.client.callTool({ + name: 'start_session', + arguments: { + workflow_id: 'meta', + agent_id: 'orchestrator', + planning_folder: join(h.workspaceDir, '.engineering/artifacts/planning', 'bootstrap-budget'), + }, + }); + expect(isError(started)).toBe(false); + const sessionIndex = parseToolResponse(started).session_index as string; + + const workflow = await h.client.callTool({ name: 'get_workflow', arguments: { session_index: sessionIndex } }); + expect(isError(workflow)).toBe(false); + + const parts = { + discover: rawText(discovered).length, + startSession: rawText(started).length, + getWorkflow: rawText(workflow).length, + }; + const total = parts.discover + parts.startSession + parts.getWorkflow; + console.log(`[bootstrap-budget] ${total} of ${budget} chars — ${JSON.stringify(parts)}`); + + expect( + total, + `bootstrap-time fixed content is ${total} characters against a stated budget of ${budget}: ` + + `discover ${parts.discover}, start_session ${parts.startSession}, get_workflow ${parts.getWorkflow}. ` + + 'Either trim what the orchestrator receives before its first decision, or state a new budget ' + + 'in meta/resources/bootstrap-protocol.md with the reason it moved.', + ).toBeLessThanOrEqual(budget); + } finally { + await h.close(); + } + }); + + it('sends the orchestrator to read no definition schema before it decides', () => { + const path = join(corpusRoot(), 'meta', 'resources', 'bootstrap-protocol.md'); + const text = readFileSync(path, 'utf8'); + // A definition schema is orders of magnitude larger than the part of it an orchestrator acts on, + // so the read belongs to whichever context authors a definition. + expect(text).not.toMatch(/workflow-server:\/\/schemas/); + }); +}); diff --git a/tests/e2e/__snapshots__/corpus-sha.json b/tests/e2e/__snapshots__/corpus-sha.json index fe39d7ee9..babbca7e7 100644 --- a/tests/e2e/__snapshots__/corpus-sha.json +++ b/tests/e2e/__snapshots__/corpus-sha.json @@ -1,4 +1,4 @@ { - "corpusSha": "c0f27ef3191639532f6faab18cf2eabf004702a4", + "corpusSha": "e47942b6ad1075be9a5f0f852429d77475f47268", "note": "Corpus commit the committed walk snapshots were generated against. Update it in the same commit that bumps the workflows submodule and re-baselines the walk (npm run baseline:stamp)." } diff --git a/tests/provenance-resolve-once.test.ts b/tests/provenance-resolve-once.test.ts new file mode 100644 index 000000000..c92ee31a5 --- /dev/null +++ b/tests/provenance-resolve-once.test.ts @@ -0,0 +1,131 @@ +/** + * Resolve work is paid once per delivery, not once per step (#404 W1). + * + * The producer scan reads every bound op in the workflow to learn its declared outputs. That answer + * does not vary with the step being decorated — only the step's document-order position does — so a + * delivery that inlines several steps builds one index and reads each position out of it. This test + * counts the loader reads behind both shapes, which is the figure that regresses if the scan moves + * back inside the per-step loop. + */ +import { describe, it, expect, vi, beforeAll, afterAll } from 'vitest'; +import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; + +/** Every `readTechnique` the scan makes, in order. Reset per test. */ +const readCalls: string[] = []; + +vi.mock('../src/loaders/technique-loader.js', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + readTechnique: async (id: string, dir: string, wfId?: string) => { + readCalls.push(id); + return actual.readTechnique(id, dir, wfId); + }, + }; +}); + +const { buildProducerIndex, provenanceContextFor, buildProvenanceContext } = + await import('../src/utils/binding-provenance.js'); +type Workflow = import('../src/schema/workflow.schema.js').Workflow; + +describe('provenance resolve work per delivery', () => { + let workflowDir: string; + + beforeAll(() => { + workflowDir = mkdtempSync(join(tmpdir(), 'wf-resolve-once-')); + const tdir = join(workflowDir, 'testwf', 'techniques'); + mkdirSync(tdir, { recursive: true }); + const op = (capability: string, outputId: string): string => + `---\nmetadata:\n version: 1.0.0\n---\n\n## Capability\n\n${capability}\n\n## Outputs\n\n### ${outputId}\n\nThe value.\n\n## Protocol\n\n### 1. Go\n\n- Do it.\n`; + for (const [file, output] of [['alpha', 'alpha_out'], ['beta', 'beta_out'], ['gamma', 'gamma_out'], ['delta', 'delta_out']]) { + writeFileSync(join(tdir, `${file}.md`), op(`Do ${file}.`, output!)); + } + }); + + afterAll(() => { + try { rmSync(workflowDir, { recursive: true, force: true }); } catch { /* ignore */ } + }); + + /** Two activities, four technique steps — three of them in the activity a delivery would inline. */ + const workflow = (): Workflow => ({ + id: 'testwf', + version: '1.0.0', + title: 'Test workflow', + activities: [ + { + id: 'first', version: '1.0.0', name: 'First', required: true, + steps: [{ kind: 'technique', id: 'run-alpha', technique: 'alpha', required: true }], + }, + { + id: 'second', version: '1.0.0', name: 'Second', required: true, + steps: [ + { kind: 'technique', id: 'run-beta', technique: 'beta', required: true }, + { kind: 'technique', id: 'run-gamma', technique: 'gamma', required: true }, + { kind: 'technique', id: 'run-delta', technique: 'delta', required: true }, + ], + }, + ], + }); + + const STEPS = ['run-beta', 'run-gamma', 'run-delta']; + + it('resolves each unique technique at most once across every step of one delivery', async () => { + readCalls.length = 0; + const index = await buildProducerIndex({ workflow: workflow(), workflowDir }); + const afterScan = readCalls.length; + + // Every step of the delivered activity reads its position out of the one index. + for (const stepId of STEPS) { + expect(provenanceContextFor(index, 'second', stepId)).not.toBeNull(); + } + expect(readCalls.length).toBe(afterScan); + + // Four bound ops in the workflow, each resolved once — the scan's own memo covers the repeat + // attempts the activity-group shorthand makes for a bare ref. + expect(index.resolvedTechniques).toBe(4); + const distinct = new Set(readCalls); + expect([...distinct].filter((id) => !id.includes('::')).sort()).toEqual(['alpha', 'beta', 'delta', 'gamma']); + for (const id of distinct) { + expect(readCalls.filter((seen) => seen === id).length).toBe(1); + } + }); + + it('decorating three steps costs the same reads as decorating one', async () => { + readCalls.length = 0; + const index = await buildProducerIndex({ workflow: workflow(), workflowDir }); + for (const stepId of STEPS) provenanceContextFor(index, 'second', stepId); + const oneIndexReads = readCalls.length; + + // The single-step helper is a scan apiece — what a per-step rebuild costs a delivery. + readCalls.length = 0; + for (const stepId of STEPS) { + await buildProvenanceContext({ workflow: workflow(), workflowDir, currentActivityId: 'second', currentStepId: stepId }); + } + expect(readCalls.length).toBe(oneIndexReads * STEPS.length); + }); + + it('places every step it can and reports the rest as unlocatable', async () => { + const index = await buildProducerIndex({ workflow: workflow(), workflowDir }); + expect(provenanceContextFor(index, 'first', 'run-alpha')?.position).toBe(0); + expect(provenanceContextFor(index, 'second', 'run-beta')?.position).toBe(1); + expect(provenanceContextFor(index, 'second', 'run-delta')?.position).toBe(3); + expect(provenanceContextFor(index, 'second', 'no-such-step')).toBeNull(); + // A step id that exists in another activity is not this activity's step. + expect(provenanceContextFor(index, 'first', 'run-beta')).toBeNull(); + }); + + it('one index and a per-step scan produce identical contexts', async () => { + const index = await buildProducerIndex({ workflow: workflow(), workflowDir }); + for (const stepId of STEPS) { + const fromIndex = provenanceContextFor(index, 'second', stepId)!; + const perStep = (await buildProvenanceContext({ + workflow: workflow(), workflowDir, currentActivityId: 'second', currentStepId: stepId, + }))!; + expect(fromIndex.position).toBe(perStep.position); + expect(fromIndex.producers).toEqual(perStep.producers); + expect([...fromIndex.declaredVariables]).toEqual([...perStep.declaredVariables]); + } + }); +}); diff --git a/workflows b/workflows index c0f27ef31..e47942b6a 160000 --- a/workflows +++ b/workflows @@ -1 +1 @@ -Subproject commit c0f27ef3191639532f6faab18cf2eabf004702a4 +Subproject commit e47942b6ad1075be9a5f0f852429d77475f47268