diff --git a/AGENTS.md b/AGENTS.md index 20711e7..1c23b79 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -46,8 +46,8 @@ docs/ ontology.md, motion.md, build-plan.md, agents.md, seed-data.md deploy/ README.md (deployment), Caddyfile example, autodeploy units ``` -~45,000 lines including tests. 547 unit tests across five packages -(core 78, prime 24, api 268, piggy 172, cli 5), plus E2E suites under +~45,000 lines including tests. 564 unit tests across five packages +(core 78, prime 24, api 276, piggy 181, cli 5), plus E2E suites under `apps/api/e2e` and `apps/piggy/e2e` that need a database — and, for one Piggy case, a key. Node 22+. diff --git a/apps/api/src/routes/motion.ts b/apps/api/src/routes/motion.ts index a00250d..7bcba6f 100644 --- a/apps/api/src/routes/motion.ts +++ b/apps/api/src/routes/motion.ts @@ -42,7 +42,7 @@ import type { MotionTemplate, QualificationScore, } from '@pig/db'; -import { demandDeals, engagementArtifacts, engagements, motionTemplates } from '@pig/db'; +import { demandDeals, engagementArtifacts, engagements, motionTemplates, users } from '@pig/db'; import { eq } from 'drizzle-orm'; import { Hono } from 'hono'; import { z } from 'zod'; @@ -62,6 +62,7 @@ import { loadEngagement, loadTemplateForWrite, lockNewestVersion, + lockTemplateForWrite, motionSlug, MotionService, newestVisibleInLineage, @@ -247,6 +248,27 @@ async function checkedTemplateId( await loadTemplateForWrite(tx, viewerOf(principal), id); } +/** + * An owner id somebody sent, checked before it is stored. + * + * `owner_user_id` is a foreign key with nothing in front of it, so an id for a + * user who has since been removed leaves as a 500 from the constraint rather + * than the 404 every other reference here answers with. Only ever the id the + * client actually supplied: an engagement created without one defaults to the + * caller, and re-reading a user the request has just authenticated would be a + * query bought with nothing — the same decision `calendar.ts` makes, and one a + * test there pins. It also leaves an explicit `ownerUserId: null` — unassigning + * — as the no-op it is. + */ +async function checkedOwnerUserId( + tx: MotionTransaction, + id: string | null | undefined, +): Promise { + if (!id) return; + const [owner] = await tx.select({ id: users.id }).from(users).where(eq(users.id, id)).limit(1); + if (!owner) throw MutationError.notFound('User'); +} + /** The same gate as `authorizePublish`, applied once the body says `shared`. */ function requirePublishFor(principal: Principal, visibility: string | undefined): void { if (visibility === 'shared') requireAnyTeamCapability(principal, 'motion:publish'); @@ -365,7 +387,11 @@ export function motionTemplateUpdateDefinition(): MutationDefinition< invalidMessage: 'Invalid motion template change.', async mutate({ input, params, principal, tx, now }) { const viewer = viewerOf(principal); - const existing = await loadTemplateForWrite( + // Locked, so the `usage_count` that §7a is about to be judged on is the + // one belonging to the row this transaction goes on to write. Read + // without the lock it is a count that a concurrent instantiation can + // move between the check and the UPDATE. + const existing = await lockTemplateForWrite( tx, viewer, requiredId(params, 'Motion template'), @@ -581,13 +607,22 @@ export function motionEngagementCreateDefinition(): MutationDefinition< permission: authorizeWrite, invalidMessage: 'Invalid engagement.', async mutate({ input, principal, tx, now }) { + // Locked, because the existence check below is only worth making if it + // cannot be raced. Two simultaneous opens of one deal both saw nothing, + // both inserted, and the loser got `23505` on + // `engagements_demand_deal_key` — not a `MutationError`, so a 500 with + // no way to tell that an engagement now exists. Concurrent opens queue + // on this row instead, and the loser's check sees the committed + // engagement and answers with its id, as the comment below promises. const [deal] = await tx .select() .from(demandDeals) .where(eq(demandDeals.id, input.demandDealId)) - .limit(1); + .limit(1) + .for('update'); if (!deal) throw MutationError.notFound('Demand deal'); await checkedTemplateId(tx, principal, input.playbookTemplateId); + await checkedOwnerUserId(tx, input.ownerUserId); // Checked rather than left to the unique constraint, so the caller gets // the id of the engagement that already exists instead of a 500. @@ -643,6 +678,7 @@ export function motionEngagementUpdateDefinition(): MutationDefinition< async mutate({ input, params, principal, tx, now }) { const { engagement, deal } = await loadEngagement(tx, requiredId(params, 'Engagement')); await checkedTemplateId(tx, principal, input.playbookTemplateId); + await checkedOwnerUserId(tx, input.ownerUserId); const [updated] = await tx .update(engagements) diff --git a/apps/api/src/services/motion.ts b/apps/api/src/services/motion.ts index 8f60d30..b469644 100644 --- a/apps/api/src/services/motion.ts +++ b/apps/api/src/services/motion.ts @@ -692,22 +692,28 @@ export class MotionService { ), ) .groupBy(engagementArtifacts.engagementId), - // Newest first, then the first sighting of each engagement wins. A - // `DISTINCT ON` would be tidier but this stays one bounded query and the - // history panel needs the same ordering anyway. + // `DISTINCT ON`, because folding a `scored_at desc` scan in memory drops + // engagements rather than truncating a list. A hundred engagements + // carrying three scores each can spend the whole 200-row budget on the + // sixty most recently scored, and every other row then renders "never + // scored" beside an exact artefact count, with nothing anywhere saying + // so. Postgres requires the ORDER BY to lead with the distinct + // expression, which is why `engagement_id` comes first; that is also the + // leading column of `qualification_scores_engagement_idx`, so this reads + // straight off the index. The result is bounded by `ids.length` and the + // limit is left as a cap rather than a budget. this.db - .select() + .selectDistinctOn([qualificationScores.engagementId]) .from(qualificationScores) .where(inArray(qualificationScores.engagementId, ids)) - .orderBy(desc(qualificationScores.scoredAt)) + .orderBy(qualificationScores.engagementId, desc(qualificationScores.scoredAt)) .limit(SCORE_LIMIT), ]); const countById = new Map(counts.map((row) => [row.engagementId, row.total])); - const latest = new Map(); - for (const score of scores) { - if (!latest.has(score.engagementId)) latest.set(score.engagementId, score); - } + const latest = new Map( + scores.map((score) => [score.engagementId, score]), + ); return rows.map(({ engagement, dealName, stage, accountId, accountName }) => ({ id: engagement.id, @@ -759,11 +765,52 @@ export async function loadTemplateForWrite( viewer: MotionViewer, id: string, ): Promise { - const [row] = await tx + return templateForWrite(tx, viewer, id, false); +} + +/** + * The same read, holding the row until this transaction commits. + * + * `usage_count` is the whole of §7a, and under READ COMMITTED reading it + * without the lock is a decision taken about a row somebody else is already + * changing. Two requests against a template at zero: an instantiation copies + * the body and increments the count, while a PATCH that read zero a moment + * earlier blocks on the row lock and then applies anyway — leaving an artefact + * whose `template_id` names a template that no longer contains what it copied, + * which is the provenance failure the rule exists to prevent. + * + * Both sides therefore take this lock, and locking only the edit does not + * work: the copy has to hold the row until its increment commits, or it is + * still reading a body that is about to change. Whichever transaction locks + * first serialises the other, and a blocked locking read re-fetches the + * committed tuple — so the edit honestly sees `usage_count = 1` and answers + * `409 template_in_use`, or the instantiation honestly copies the edited body. + * + * `no key update` rather than `update`, because the row is a foreign-key + * target — `engagement_artifacts.template_id` — and nothing here is a reason + * to block an insert that merely references it. + */ +export async function lockTemplateForWrite( + tx: MotionTransaction, + viewer: MotionViewer, + id: string, +): Promise { + return templateForWrite(tx, viewer, id, true); +} + +/** One predicate for both, so the lock cannot drift away from the read filter. */ +async function templateForWrite( + tx: MotionTransaction, + viewer: MotionViewer, + id: string, + lock: boolean, +): Promise { + const query = tx .select() .from(motionTemplates) .where(and(eq(motionTemplates.id, id), visibleTemplates(viewer))) .limit(1); + const [row] = lock ? await query.for('no key update') : await query; if (!row) throw MutationError.notFound('Motion template'); return row; } @@ -905,8 +952,11 @@ export async function instantiateArtifact( input: InstantiateInput, now: Date, ): Promise<{ artifact: EngagementArtifact; template: MotionTemplate | null }> { + // Locked, not merely read: the body copied below has to be the body the + // `usage_count` increment at the end of this function is counting, or a + // concurrent edit rewrites the template between the two. const template = input.templateId - ? await loadTemplateForWrite(tx, viewer, input.templateId) + ? await lockTemplateForWrite(tx, viewer, input.templateId) : null; // Every read path hides an archived template, so instantiating one can only // happen by holding an id from before it was archived — and it would leave diff --git a/apps/api/test/motion.test.ts b/apps/api/test/motion.test.ts index 4eb3c5c..c35963c 100644 --- a/apps/api/test/motion.test.ts +++ b/apps/api/test/motion.test.ts @@ -17,6 +17,11 @@ * queries in call order and records what ran; it is deliberately not a * database, because a fake that pretends to run SQL is a fake that will one day * assert a broken query works. + * + * That fake is also why the last suite goes through `createApp` instead. A + * definition driven directly, or a route mounted by this file's own `mounted`, + * passes whether or not `app.ts` ever calls `createMotionRoutes` — the state + * `read-guards.ts` and `learn.ts` were both in while their tests were green. */ import { strict as assert } from 'node:assert'; import { describe, it } from 'node:test'; @@ -24,8 +29,12 @@ import { getTableName, isSQLWrapper, isTable, type SQL } from 'drizzle-orm'; import { PgDialect } from 'drizzle-orm/pg-core'; import { motionScoreBasisPoints } from '@pig/core'; import type { Database, MotionTemplate } from '@pig/db'; +import { teamMemberships, users } from '@pig/db'; import { Hono } from 'hono'; +import { createApp } from '../src/app'; import { AuthError, type Principal } from '../src/lib/auth'; +import type { AuthProvider } from '../src/lib/auth-provider'; +import { loadConfig } from '../src/lib/config'; import { executeMutation, MutationError, type ApiEnv } from '../src/lib/mutation'; import { createMotionRoutes, @@ -134,7 +143,13 @@ function database(script: unknown[][]): { db: Database; log: Recorded } { // can follow it, as it does on every query in this feature that allocates // a version number. The chain is a thenable, so `await` still ends it. limit: () => selectChain, - for: () => selectChain, + // Recorded, not merely tolerated: two of the rules here are held shut by a + // row lock, and a lock that quietly stops being taken changes nothing a + // behavioural assertion can see. + for: (strength: string) => { + log.events.push(`for:${strength}`); + return selectChain; + }, then: (resolve: (rows: unknown[]) => unknown) => resolve(next()), }; @@ -281,6 +296,26 @@ describe('a used template is never edited in place', () => { assert.equal(log.updated[0]?.values.title, 'POC plan, tightened'); }); + it('locks the row before it trusts usage_count, so a concurrent instantiation cannot be missed', async () => { + const { db, log } = database([[template({ usageCount: 0 })]]); + + await executeMutation( + db, + member, + async () => ({ body: '# Tightened' }), + motionTemplateUpdateDefinition(), + { id: TEMPLATE_ID }, + ); + + // Read without the lock, `usage_count` is a number another transaction is + // already moving: an instantiation copies the body and increments the + // count while this PATCH, having seen zero, waits on the row and then + // rewrites the body anyway — leaving an artefact whose `template_id` names + // a template that no longer contains what it copied. `no key update` + // rather than `update` because the row is a foreign-key target. + assert.deepEqual(log.events.slice(0, 3), ['transaction', 'select', 'for:no key update']); + }); + it('refuses an edit to somebody else\'s template even when it is shared — publishing is not donating', async () => { const { db, log } = database([[template({ visibility: 'shared', ownerUserId: OTHER })]]); @@ -661,6 +696,30 @@ describe('instantiating a template', () => { // back into the library. assert.equal(log.inserted[0]?.row.body, template().body); }); + + it('holds the template row while it copies the body, not only while it counts the use', async () => { + const { db, log } = database([ + [{ engagement, deal }], + [template({ visibility: 'shared', ownerUserId: OTHER })], + ]); + + await executeMutation( + db, + member, + async () => ({ templateId: TEMPLATE_ID }), + motionArtifactCreateDefinition(), + { id: ENGAGEMENT_ID }, + ); + + // Locking only the PATCH does not close the race: the copy has to hold the + // row until its own increment commits, or an edit lands between the read + // that copied the body and the count that was supposed to have shut the + // template to edits. + assert.deepEqual( + log.events.slice(0, 4), + ['transaction', 'select', 'select', 'for:no key update'], + ); + }); }); describe('one engagement per deal', () => { @@ -682,6 +741,27 @@ describe('one engagement per deal', () => { ); assert.deepEqual(log.inserted, []); }); + + it('locks the deal before it looks, so two simultaneous opens queue instead of both inserting', async () => { + const { db, log } = database([[deal], []]); + + await executeMutation( + db, + member, + async () => ({ demandDealId: DEAL_ID }), + motionEngagementCreateDefinition(), + ); + + // The race is not expressible against this fake — it runs no SQL and has + // no concurrency — so what is pinned is the lock that removes it. Without + // it both requests' checks above see nothing, both insert, and the loser + // gets `23505` on `engagements_demand_deal_key`, which is not a + // `MutationError` and so leaves as `500 Internal error` with no way to + // tell that an engagement now exists. The lock is taken on the deal + // because the row the loser must wait behind is the engagement that does + // not exist yet. + assert.deepEqual(log.events.slice(0, 3), ['transaction', 'select', 'for:update']); + }); }); describe('qualification scores', () => { @@ -784,3 +864,86 @@ describe('an id that cannot name a row is a row that does not exist', () => { assert.deepEqual(log.events, ['transaction'], 'refused on shape, without a query'); }); }); + +// --------------------------------------------------- the mount, not the mock + +const SUBJECT = 'motion-auth-subject'; + +/** + * The whole app, with the cheapest database that can carry an authenticated + * request through it — the fixture `http-auth.test.ts` uses, which answers by + * table identity rather than in call order because the queries `loadPrincipal` + * and the overview make are an implementation detail. + */ +function createdApp(): ReturnType { + const user = { + id: OWNER, + email: 'seller@example.com', + name: 'Seller', + authSubject: SUBJECT, + deactivatedAt: null, + isPlatformAdmin: false, + }; + + function chain(rows: unknown[]): Record { + const self: Record = { + leftJoin: () => self, + innerJoin: () => self, + where: () => self, + orderBy: () => self, + groupBy: () => self, + limit: () => self, + then: (resolve: (value: unknown[]) => unknown) => resolve(rows), + }; + return self; + } + + const db = { + select: () => ({ + from: (table: unknown) => { + if (table === users) return chain([user]); + if (table === teamMemberships) return chain([{ team: 'demand', role: 'member' }]); + return chain([]); + }, + }), + update: () => ({ set: () => ({ where: async () => undefined }) }), + transaction: async (work: (tx: unknown) => Promise) => work({}), + } as unknown as Database; + + const provider: AuthProvider = { + name: 'test', + async verifyAccessToken(token: string) { + if (token !== 'good-token') throw new Error('bad token'); + return { subject: SUBJECT, email: user.email }; + }, + }; + + // A real `loadConfig`, for the reason `http-auth.test.ts` gives: a hand-built + // Config object would let this pass under one the server refuses to start on. + const config = loadConfig({ + NODE_ENV: 'test', + DATABASE_URL: 'postgres://pig:pig@localhost:5432/pig-not-connected', + PIG_PUBLIC_URL: 'http://localhost:8920', + PIG_ADMIN_EMAILS: '', + } as NodeJS.ProcessEnv); + + return createApp(config, db, provider); +} + +describe('the routes are mounted in app.ts, not only in this file', () => { + it('answers a member holding book:read on GET /api/motion', async () => { + // AGENTS.md §5, verbatim: every other test here mounts the factory itself, + // so all twelve routes could be dropped from `app.ts` and this file would + // stay green. The request is authenticated deliberately — `app.use('/api/*')` + // authenticates ahead of every feature route, so an anonymous GET answers + // 401 whether or not anything is mounted behind it, and the same assertion + // would pass against a feature that had been deleted outright. + const response = await createdApp().request('/api/motion', { + headers: { authorization: 'Bearer good-token' }, + }); + + assert.notEqual(response.status, 404, 'createMotionRoutes is not mounted in createApp'); + assert.notEqual(response.status, 401, 'the request never reached the motion handler'); + assert.notEqual(response.status, 403, 'the read guard answered, so the mount is untested'); + }); +}); diff --git a/apps/piggy/src/page-tools.ts b/apps/piggy/src/page-tools.ts index 54a4de7..eddd8e3 100644 --- a/apps/piggy/src/page-tools.ts +++ b/apps/piggy/src/page-tools.ts @@ -62,6 +62,7 @@ import { CalendarService } from '@pig/api/src/services/calendar'; import { and, count, + countDistinct, desc, eq, gte, @@ -210,6 +211,16 @@ const SUPPLY_DEALS_LABEL = 'supply deal(s) on the book'; const ACCOUNTS_LABEL = 'account(s) on the book'; const CONTACTS_LABEL = 'contact(s) in the CRM'; +/** + * The denominators the motion tools are drawn from. + * + * The library one says `shared` and says `lineage` because both are real + * restrictions on the figure: private drafts are outside it by design, and a + * lineage is one piece of practice however many versions it has carried. + */ +const MOTION_LIBRARY_LABEL = 'shared template lineage(s) in the motion library'; +const ENGAGEMENTS_LABEL = 'engagement(s) on the book'; + /** One whole-table count, for use as a denominator. */ function rowCount(rows: readonly { value: number }[]): number { return rows[0]?.value ?? 0; @@ -1086,7 +1097,12 @@ function countBy(rows: readonly Row[], key: (row: Row) => string): Record { - const [libraryRead, engagementRead, promotions] = await Promise.all([ + // One predicate for the exemplars and for the count below them, so the list + // and the figure cannot come to describe different sets. It is also the + // predicate the /motion tile counts: `motionLibraryWhere({})` already excludes + // archived rows, and promotion always writes `visibility: 'shared'`. + const promoted = and(motionLibraryWhere({}), isNotNull(motionTemplates.originArtifactId))!; + const [libraryRead, engagementRead, promotions, promotionTotal] = await Promise.all([ db .select({ slug: motionTemplates.slug, @@ -1114,13 +1130,21 @@ async function readMotionSummary(db: Database): Promise { createdAt: motionTemplates.createdAt, }) .from(motionTemplates) - .where(and(motionLibraryWhere({}), isNotNull(motionTemplates.originArtifactId))) + .where(promoted) .orderBy(desc(motionTemplates.createdAt)) .limit(EXEMPLARS), + // Counted in SQL rather than read off the list above it, which is capped at + // EXEMPLARS. Promotions is the one figure whose job is to show the loop + // closing, so it is the one figure that must not stop moving: read off the + // list it would say 8 the moment the loop started working, for ever, beside + // a tile counting 12 exactly. Versions rather than lineages, like the tile — + // a second promotion into one lineage is a second time the loop closed. + db.select({ value: count() }).from(motionTemplates).where(promoted), ]); const { rows: templateRows, truncated: libraryTruncated } = bounded(libraryRead); const { rows: openEngagements, truncated: engagementsTruncated } = bounded(engagementRead); const truncated = libraryTruncated || engagementsTruncated; + const promotionCount = rowCount(promotionTotal); const lineages = newestPerSlug(templateRows); const engagementsByStage = countBy(openEngagements, (row) => row.stage); @@ -1133,21 +1157,55 @@ async function readMotionSummary(db: Database): Promise { `${atLeast(lineages.length, libraryTruncated)} shared template(s) across ` + `${Object.keys(countBy(lineages, (row) => row.kind)).length} of ${MOTION_KINDS.length} ` + `kind(s), and ${atLeast(openEngagements.length, engagementsTruncated)} open engagement(s). ` + - (uncovered.length === 0 - ? 'Every live demand stage has at least one shared template.' - : `No shared template covers ${uncovered.join(', ')}.`) + - ` ${promotions.length} artifact(s) promoted back into the library recently.` + + (libraryTruncated + ? 'Which stages the library misses cannot be told from this scan, because it hit its ' + + 'row cap. ' + : uncovered.length === 0 + ? 'Every live demand stage has at least one shared template. ' + : `No shared template covers ${uncovered.join(', ')}. `) + + `${promotionCount} artifact(s) promoted back into the library in all` + + (promotions.length ? `, newest ${promotions.length} listed.` : '.') + (truncated ? ` ${TRUNCATION_NOTE}` : ''), + /** Unfiltered within the shared library, so this read IS its own denominator. */ + scope: resultScope({ + covers: 'are shared and not archived', + matched: lineages.length, + total: lineages.length, + totalLabel: MOTION_LIBRARY_LABEL, + listed: 0, + truncated: libraryTruncated, + }), truncated, sharedTemplates: lineages.length, openEngagements: openEngagements.length, - uncoveredStages: uncovered, + /** + * Null rather than a list when the scan was capped. "No shared template + * covers procurement" is a definite negative, and a definite negative drawn + * from part of the library is a claim the read cannot support — the stage + * may well be covered by a lineage beyond the cap. An exact answer under + * truncation needs a per-slug newest-version aggregate in SQL, which is a + * larger change than this figure is worth. + */ + uncoveredStages: libraryTruncated ? null : uncovered, stages: DEMAND_OPEN_STAGES.map((stage) => ({ stage, openEngagements: engagementsByStage[stage] ?? 0, sharedTemplates: lineages.filter((template) => template.stage === stage).length, })), libraryByKind: countBy(lineages, (row) => row.kind), + promotions: promotionCount, + /** + * The exact figure sits beside the list on purpose. A model handed eight + * rows and no total reads the length of the list as the count, which is the + * mistake `ResultScope` exists for. + */ + recentPromotionsScope: resultScope({ + covers: 'were promoted back into the library from an engagement artefact', + matched: promotionCount, + total: promotionCount, + totalLabel: 'promoted template version(s) in the motion library', + listed: promotions.length, + }), // The loop made visible: what the last few engagements gave back. recentPromotions: promotions.map((row) => ({ title: row.title, @@ -1159,8 +1217,8 @@ async function readMotionSummary(db: Database): Promise { } async function readMotionLibrary(db: Database, filter: MotionLibraryFilter): Promise { - const { rows, truncated } = bounded( - await db + const [libraryRead, libraryTotal] = await Promise.all([ + db .select({ slug: motionTemplates.slug, kind: motionTemplates.kind, @@ -1174,25 +1232,60 @@ async function readMotionLibrary(db: Database, filter: MotionLibraryFilter): Pro .where(motionLibraryWhere(filter)) .orderBy(desc(motionTemplates.updatedAt)) .limit(SCAN_LIMIT + 1), - ); + /** + * The denominator, and both halves of it are decisions. + * + * `motionLibraryWhere({})` rather than a bare `count()` over the table: a + * total that included private rows would publish the existence and the size + * of colleagues' drafts through the back door, which is the one thing the + * library rule exists to withhold — a denominator leaks as readily as a + * list. And `countDistinct(slug)` rather than `count()`, because the slug is + * the identity of a lineage: counting versions reports a library several + * times the size of the one anybody can choose from, which is the same + * mistake `newestPerSlug` exists to avoid on the matched side. Counted in + * SQL, so the total stays exact when the read beside it hits its cap. + */ + db + .select({ value: countDistinct(motionTemplates.slug) }) + .from(motionTemplates) + .where(motionLibraryWhere({})), + ]); + const { rows, truncated } = bounded(libraryRead); const lineages = newestPerSlug(rows as LineageRow[]); + const sharedTemplates = rowCount(libraryTotal); const described = [ filter.kind ? `kind ${filter.kind}` : null, filter.stage ? `stage ${filter.stage}` : null, filter.query ? `"${filter.query}"` : null, ].filter((part): part is string => part !== null); - const scope = described.length ? ` matching ${described.join(', ')}` : ''; + const matching = described.length ? ` matching ${described.join(', ')}` : ''; return { headline: (lineages.length === 0 - ? `No shared template${scope}. Private drafts are not searched, so a template may ` + - 'exist and not be visible here.' - : `${atLeast(lineages.length, truncated)} shared template(s)${scope}, newest version ` + - 'of each.') + (truncated ? ` ${TRUNCATION_NOTE}` : ''), + ? `None of the ${sharedTemplates} ${MOTION_LIBRARY_LABEL}${matching}. Private drafts are ` + + 'not searched, so a template may exist and not be visible here.' + : `${atLeast(lineages.length, truncated)} of ${sharedTemplates} ` + + `${MOTION_LIBRARY_LABEL}${matching}, newest version of each.`) + + (truncated ? ` ${TRUNCATION_NOTE}` : ''), + scope: resultScope({ + covers: described.length ? `match ${described.join(', ')}` : 'are shared and not archived', + matched: lineages.length, + total: sharedTemplates, + totalLabel: MOTION_LIBRARY_LABEL, + listed: Math.min(lineages.length, EXEMPLARS), + filters: { + ...(filter.kind ? { kind: filter.kind } : {}), + ...(filter.stage ? { stage: filter.stage } : {}), + ...(filter.query ? { query: filter.query } : {}), + }, + truncated, + }), truncated, count: lineages.length, + /** The denominator as a bare field: this is how big the shared library is. */ + sharedTemplates, byKind: countBy(lineages, (row) => row.kind), // Most recently updated first: the practice people are actually amending. templates: lineages.slice(0, EXEMPLARS).map((template) => ({ @@ -1220,8 +1313,8 @@ async function readMotionLibrary(db: Database, filter: MotionLibraryFilter): Pro */ async function readEngagements(db: Database, query: string | null): Promise { const fragment = query ? likeFragment(query) : null; - const { rows, truncated } = bounded( - await db + const [engagementRead, engagementTotal] = await Promise.all([ + db .select({ id: engagements.id, status: engagements.status, @@ -1241,7 +1334,13 @@ async function readEngagements(db: Database, query: string | null): Promise row.id); @@ -1280,13 +1379,25 @@ async function readEngagements(db: Database, query: string | null): Promise row.status === 'open').length} open.`) + + : `${atLeast(rows.length, truncated)} of ${totalEngagements} ${ENGAGEMENTS_LABEL}` + + `${query ? ` match "${query}"` : ''}, of which ` + + `${rows.filter((row) => row.status === 'open').length} open.`) + (truncated ? ` ${TRUNCATION_NOTE}` : ''), + scope: resultScope({ + covers: query ? `match "${query}"` : 'are on the book', + matched: rows.length, + total: totalEngagements, + totalLabel: ENGAGEMENTS_LABEL, + listed: exemplars.length, + filters: query ? { query } : {}, + truncated, + }), truncated, count: rows.length, + /** The denominator as a bare field: engagements exist that this did not match. */ + totalEngagements, byStatus: countBy(rows, (row) => row.status), byStage: countBy(rows, (row) => row.stage), engagements: exemplars.map((row) => { diff --git a/apps/piggy/test/motion-tools.test.ts b/apps/piggy/test/motion-tools.test.ts index 4c85e20..a1525d8 100644 --- a/apps/piggy/test/motion-tools.test.ts +++ b/apps/piggy/test/motion-tools.test.ts @@ -16,16 +16,25 @@ * * The unit suite runs in CI BEFORE the migration step, against a database with * no tables, so nothing here may execute a query. The WHERE clause is rendered - * with `PgDialect` rather than run. + * with `PgDialect` rather than run, and the tools themselves are executed + * against a stub handle — the second half of this file, which pins the figures + * they report and the denominators those figures are drawn from. */ import assert from 'node:assert/strict'; import test from 'node:test'; import { MOTION_KINDS } from '@pig/core'; -import type { Database } from '@pig/db'; +import { + engagementArtifacts, + engagements, + motionTemplates, + qualificationScores, + type Database, +} from '@pig/db'; +import type { SQL } from 'drizzle-orm'; import { PgDialect } from 'drizzle-orm/pg-core'; import { zodToJsonSchema } from 'zod-to-json-schema'; import { assertPigToolBoundary } from '../src/chat'; -import { createPagePigTools, motionLibraryWhere } from '../src/page-tools'; +import { createPagePigTools, motionLibraryWhere, type ResultScope } from '../src/page-tools'; /** Schema and SQL-shape checks only: no query is executed. */ const db = {} as Database; @@ -209,3 +218,331 @@ test('an omitted motion filter arrives as the null the emitted schema asks for', ); } }); + +// --------------------------------------------------------------------------- +// The figures the tools report +// --------------------------------------------------------------------------- + +/** + * A stub handle, because the unit suite still may not execute a query. + * + * Drizzle's builder is a promise you can keep calling methods on, so this is + * the same: every chaining method returns itself and `then` resolves the rows. + * What is under test here is the shaping — which figure reaches the headline + * and what denominator sits beside it — not the SQL, which + * `e2e/page-tools.test.ts` covers against a real book. + * + * The where clauses are not evaluated: a stub that reimplemented them would be + * testing itself, so a filtered read is fixtured as the rows it returned. They + * ARE rendered, for the one assertion that has to see them — a denominator + * taken over the whole table rather than the shared library would publish the + * size of everybody's private drafts, and it would look exactly like this from + * the outside. + */ +interface StubMotion { + /** What the library read returned. Shared, non-archived, versions included. */ + templates: readonly Record[]; + /** `count(distinct slug)`: lineages, not rows. Fixtured, not derived. */ + lineageTotal: number; + /** The promotion exemplars, capped by the tool, and their exact total. */ + promotionRows?: readonly Record[]; + promotionTotal?: number; + /** The open engagements the summary counts by stage. */ + openEngagements?: readonly Record[]; + /** What the engagement search matched, against every engagement on the book. */ + engagementRows?: readonly Record[]; + engagementTotal?: number; + artifacts?: readonly Record[]; + scores?: readonly Record[]; +} + +function stubBook(book: StubMotion): { handle: Database; denominators: string[] } { + const denominators: string[] = []; + + const rowsFor = ( + table: unknown, + projection: Record, + where: string, + ): readonly unknown[] => { + const counting = Object.hasOwn(projection, 'value'); + if (table === motionTemplates) { + if (counting) { + denominators.push(where); + // Both counts are taken over `motion_templates`; only the promotion one + // asks for an origin artefact, which is what tells them apart here. + return where.includes('origin_artifact_id') + ? [{ value: book.promotionTotal ?? 0 }] + : [{ value: book.lineageTotal }]; + } + // The promotion exemplars are the only template read carrying a date. + return Object.hasOwn(projection, 'createdAt') ? (book.promotionRows ?? []) : book.templates; + } + if (table === engagements) { + if (counting) { + denominators.push(where); + return [{ value: book.engagementTotal ?? 0 }]; + } + // The summary asks for a stage per open engagement; the search asks for + // the row. + return Object.hasOwn(projection, 'id') + ? (book.engagementRows ?? []) + : (book.openEngagements ?? []); + } + if (table === engagementArtifacts) return book.artifacts ?? []; + if (table === qualificationScores) return book.scores ?? []; + throw new Error('the stub was asked for a table this suite does not fixture'); + }; + + const select = (projection: Record) => ({ + from: (table: unknown) => { + let where = ''; + const builder: Record = { + where: (clause: SQL | undefined) => { + if (clause) where = dialect.sqlToQuery(clause).sql; + return builder; + }, + }; + for (const method of ['limit', 'orderBy', 'groupBy', 'innerJoin', 'leftJoin']) { + builder[method] = () => builder; + } + builder.then = (resolve: (value: readonly unknown[]) => unknown) => + resolve(rowsFor(table, projection, where)); + return builder; + }, + }); + return { handle: { select } as unknown as Database, denominators }; +} + +type Reading = Record & { headline?: string; scope?: ResultScope }; + +async function read( + route: '/motion' | '/motion/library' | '/motion/engagements', + handle: Database, + input: Record = {}, +): Promise { + const [only] = createPagePigTools(handle, route); + assert.ok(only, `no tool for ${route}`); + return (await only.execute(input)) as Reading; +} + +function template(slug: string, stage: string, version = 1) { + return { + slug, + kind: 'playbook', + stage, + version, + title: `Template ${slug} v${version}`, + summary: 'A shared template.', + usageCount: 3, + }; +} + +/** + * Twelve lineages in thirteen rows, covering six of the eight open stages. + * + * The thirteenth row is a second version of the first lineage, and it is there + * because the two figures differ: a library reported by rows says 13 when the + * number of pieces of practice anybody can choose from is 12. + */ +const COVERED_STAGES = ['qualification', 'legal', 'scoping', 'proposal', 'poc', 'expansion']; +const SHARED_ROWS = [ + ...Array.from({ length: 12 }, (_, i) => + template(`starter-${i}`, COVERED_STAGES[i % COVERED_STAGES.length]!), + ), + template('starter-0', 'qualification', 2), +]; +const LINEAGES = 12; + +/** Twelve promotions, of which the tool may show eight. The gap is the point. */ +const PROMOTION_TOTAL = 12; +const PROMOTION_ROWS = Array.from({ length: 8 }, (_, i) => ({ + title: `Promoted ${i}`, + kind: 'case_study', + version: 2, + createdAt: new Date(Date.UTC(2026, 0, i + 1)), +})); + +const LIBRARY = stubBook({ + templates: SHARED_ROWS, + lineageTotal: LINEAGES, + promotionRows: PROMOTION_ROWS, + promotionTotal: PROMOTION_TOTAL, + openEngagements: Array.from({ length: 5 }, () => ({ stage: 'poc', dealName: 'DEMO — Halcyon' })), +}); + +test('the promotion figure is the exact total, not the length of the list beside it', async () => { + const reading = await read('/motion', LIBRARY.handle); + + // The /motion tile counts promotions in SQL for exactly this reason. Piggy + // reading the length of its own capped list would peg the answer at 8 the + // moment the loop started working, and disagree with the tile on screen. + assert.equal(reading.promotions, PROMOTION_TOTAL); + assert.match(String(reading.headline), /12 artifact\(s\) promoted back into the library/); + assert.doesNotMatch(String(reading.headline), /8 artifact\(s\) promoted/); + + const scope = reading.recentPromotionsScope as ResultScope; + assert.equal(scope.total, PROMOTION_TOTAL); + assert.equal(scope.listed, PROMOTION_ROWS.length); + assert.equal((reading.recentPromotions as unknown[]).length, PROMOTION_ROWS.length); +}); + +test('the motion summary counts lineages, not versions', async () => { + const reading = await read('/motion', LIBRARY.handle); + const scope = reading.scope; + assert.ok(scope); + + // 13 rows, 12 lineages. A denominator of 13 would be a library four times the + // size of the one anybody can choose from, in miniature. + assert.equal(scope.total, LINEAGES); + assert.equal(reading.sharedTemplates, LINEAGES); + assert.notEqual(scope.total, SHARED_ROWS.length); + assert.match(scope.totalLabel, /shared template lineage\(s\)/); +}); + +test('no motion denominator is taken over the whole template table', async () => { + await read('/motion', LIBRARY.handle); + await read('/motion/library', LIBRARY.handle, {}); + assert.ok(LIBRARY.denominators.length >= 2); + for (const where of LIBRARY.denominators) { + // A total that counted private rows would publish the existence and the + // size of colleagues' drafts through a figure nobody thinks of as a read. + assert.match(where, /"visibility" = \$\d+/, where); + assert.match(where, /"archived_at" is null/, where); + } +}); + +test('the stages the library misses are named when the whole library was read', async () => { + const reading = await read('/motion', LIBRARY.handle); + // Six stages are covered by the fixture, so the two that are not are a piece + // of work somebody can act on — which is why this is a list and not a count. + assert.deepEqual(reading.uncoveredStages, ['procurement', 'deployment']); + assert.match(String(reading.headline), /No shared template covers procurement, deployment\./); +}); + +test('stage coverage is not asserted from a capped scan', async () => { + // SCAN_LIMIT is 500 and the read asks for one more, so 501 rows is a library + // that certainly continues past the cap. + const capped = stubBook({ + templates: Array.from({ length: 501 }, (_, i) => template(`over-${i}`, 'qualification')), + lineageTotal: 501, + promotionRows: PROMOTION_ROWS, + promotionTotal: PROMOTION_TOTAL, + }); + const reading = await read('/motion', capped.handle); + + // "No shared template covers deployment" is a definite negative, and the rows + // that would refute it are precisely the ones the cap dropped. Null says the + // question was not answered; an empty list would say every stage is covered + // and a full list would say seven are not, and both are inventions. + assert.equal(reading.uncoveredStages, null); + assert.doesNotMatch(String(reading.headline), /No shared template covers/); + assert.doesNotMatch(String(reading.headline), /Every live demand stage/); + assert.match(String(reading.headline), /row cap/); +}); + +test('a filtered library search is counted against the whole shared library', async () => { + // The stub evaluates no where clause, so what a kind filter matched is + // fixtured: three lineages, out of a library that still holds twelve. + const filtered = stubBook({ + templates: [ + template('proposal-blocks', 'proposal'), + template('proposal-terms', 'procurement'), + template('proposal-exec', 'proposal'), + ], + lineageTotal: LINEAGES, + }); + const reading = await read('/motion/library', filtered.handle, { kind: 'proposal' }); + const scope = reading.scope; + assert.ok(scope); + + assert.equal(scope.matched, 3); + assert.equal(scope.total, LINEAGES); + assert.equal(reading.count, 3); + assert.equal(reading.sharedTemplates, LINEAGES); + // The denominator has to reach the headline, because the headline is the + // field a small model quotes: "3 shared templates" alone is the size of a + // filter presented as the size of the library. + assert.match(String(reading.headline), /3 of 12 shared template lineage\(s\)/); + assert.match(scope.summary, /the total is 12/); + assert.equal(scope.filters.kind, 'proposal'); +}); + +test('an unfiltered library search says so rather than hedging an exact figure', async () => { + const reading = await read('/motion/library', LIBRARY.handle, { + kind: null, + stage: null, + query: null, + }); + const scope = reading.scope; + assert.ok(scope); + + // Nothing was filtered out, so `matched` IS the total. Reporting the nulls a + // schema-abiding model sends as filters would teach it to distrust a figure + // that is exact. + assert.equal(scope.matched, scope.total); + assert.deepEqual(scope.filters, {}); + assert.match(scope.summary, /All 12 shared template lineage\(s\) in the motion library/); +}); + +test('an engagement search is counted against every engagement on the book', async () => { + const book = stubBook({ + templates: [], + lineageTotal: LINEAGES, + engagementTotal: 9, + engagementRows: [ + { + id: 'engagement-1', + status: 'open', + summary: 'Mid POC.', + openedAt: new Date(Date.UTC(2026, 1, 1)), + stage: 'poc', + dealName: 'DEMO — Halcyon Research', + accountName: 'DEMO — Halcyon Research', + }, + { + id: 'engagement-2', + status: 'closed', + summary: 'Won.', + openedAt: new Date(Date.UTC(2025, 10, 1)), + stage: 'expansion', + dealName: 'DEMO — Halcyon Expansion', + accountName: 'DEMO — Halcyon Research', + }, + ], + artifacts: [{ engagementId: 'engagement-1', status: 'final' }], + scores: [], + }); + const reading = await read('/motion/engagements', book.handle, { query: 'Halcyon' }); + const scope = reading.scope; + assert.ok(scope); + + assert.equal(scope.matched, 2); + assert.equal(scope.total, 9); + assert.equal(reading.totalEngagements, 9); + assert.match(String(reading.headline), /2 of 9 engagement\(s\) on the book match "Halcyon"/); + assert.equal(scope.filters.query, 'Halcyon'); +}); + +test('every motion result carries a complete scope, and no scope outruns its own total', async () => { + const readings = [ + await read('/motion', LIBRARY.handle), + await read('/motion/library', LIBRARY.handle, {}), + await read('/motion/engagements', stubBook({ templates: [], lineageTotal: 0 }).handle), + ]; + + for (const reading of readings) { + const found = Object.entries(reading) + .filter(([key]) => key === 'scope' || key.endsWith('Scope')) + .map(([, value]) => value as ResultScope); + assert.ok(found.length > 0, `a motion result carries no scope at all: ${reading.headline}`); + for (const scope of found) { + assert.ok(scope.summary.length > 0); + assert.ok(scope.totalLabel.length > 0); + // The denominator has to reach the sentence, because the sentence is what + // gets quoted. + assert.match(scope.summary, new RegExp(`\\b${scope.total}\\b`)); + assert.ok(scope.matched <= scope.total, 'matched exceeds its own denominator'); + assert.ok(scope.listed <= scope.matched, 'more rows listed than matched'); + } + } +}); diff --git a/apps/web/src/components/AppSidebar.tsx b/apps/web/src/components/AppSidebar.tsx index f5002ea..01d15e0 100644 --- a/apps/web/src/components/AppSidebar.tsx +++ b/apps/web/src/components/AppSidebar.tsx @@ -9,9 +9,9 @@ */ import { Fragment } from 'react'; import { X } from 'lucide-react'; -import { Link, useMatch, useResolvedPath } from 'react-router-dom'; +import { Link, useLocation } from 'react-router-dom'; import { useIdentity } from '@/lib/identity'; -import { NAV_GROUP_HEADING, NAV_GROUPS, visibleNav, type NavItem } from '@/lib/nav'; +import { activeNavItem, NAV_GROUP_HEADING, NAV_GROUPS, visibleNav, type NavItem } from '@/lib/nav'; import { AccountSwitcher } from './AccountSwitcher'; import { Button, Label } from './ui'; import { @@ -33,6 +33,16 @@ export function AppSidebar() { const identity = useIdentity(); const items = visibleNav(identity); const { isMobile, setOpenMobile } = useSidebar(); + const { pathname } = useLocation(); + /* + * One winner for the whole rail, decided here rather than by each row asking + * the router about itself. Motion is the first group whose destinations nest + * — `/motion` is a prefix of `/motion/library` — and a per-row match lit both + * of those at once, so the sidebar and the header disagreed about which page + * you were on. `activeNavItem` is the same longest-match helper the header + * titles with, which is what keeps them from ever disagreeing again. + */ + const current = activeNavItem(items, pathname); return ( @@ -75,7 +85,7 @@ export function AppSidebar() { {groupItems.map((item) => ( - + ))} @@ -103,14 +113,11 @@ export function AppSidebar() { ); } -function NavItemRow({ item }: { item: NavItem }) { +// `asChild` renders the row *as* the link rather than wrapping one, so there is +// a single focusable element per row. Active state arrives as a prop because it +// is a question about the whole table — see the caller. +function NavItemRow({ item, isActive }: { item: NavItem; isActive: boolean }) { const { setOpenMobile, isMobile } = useSidebar(); - // `asChild` renders the row *as* the link rather than wrapping one, so there - // is a single focusable element per row. Active state is asked of the router - // instead of compared against a pathname, so `/demand/abc` still lights - // Demand and `/` does not light everything. - const resolved = useResolvedPath(item.to); - const isActive = useMatch({ path: resolved.pathname, end: item.to === '/' }) !== null; return ( diff --git a/apps/web/src/components/motion/FieldsView.tsx b/apps/web/src/components/motion/FieldsView.tsx index a666af8..48c8575 100644 --- a/apps/web/src/components/motion/FieldsView.tsx +++ b/apps/web/src/components/motion/FieldsView.tsx @@ -38,10 +38,10 @@ export function FieldsView({ const record = asRecord(fields); if (!record) return null; - const body = renderKind(kind, record); - if (!body) return null; - - return
{body}
; + // Emptiness is decided inside each kind's renderer, not here: `renderKind` + // hands back an element, and an element is truthy however little it draws. + // A guard at this level could only ever read as one and never fire. + return
{renderKind(kind, record)}
; } function renderKind(kind: MotionKind, fields: Record): ReactNode { @@ -71,7 +71,11 @@ function renderKind(kind: MotionKind, fields: Record): ReactNod function DiscoveryFields({ fields }: { fields: Record }) { const sections = recordList(fields.sections); - if (sections.length === 0) return null; + const decisions = recordList(fields.decisions); + const blockingSet = recordList(fields.blockingSet); + if (sections.length === 0 && decisions.length === 0 && blockingSet.length === 0) return null; + + const blockingCount = decisions.filter((decision) => decision.blocking === true).length; return ( <> @@ -101,6 +105,84 @@ function DiscoveryFields({ fields }: { fields: Record }) { ); })} + + {decisions.length === 0 ? null : ( +
0 ? `${blockingCount} of ${decisions.length} blocking` : undefined} + > +
+ {decisions.map((decision, index) => { + const question = text(decision.question); + if (!question) return null; + const area = text(decision.area); + const id = text(decision.id); + const options = recordList(decision.options); + return ( +
+
+ {area ? {area} : null} + {decision.blocking === true ? Blocking : null} + {/* The blocking set below names decisions by id, so the id + is content here rather than a React key. */} + {id ? {id} : null} +
+

{question}

+ + {options.length === 0 ? null : ( +
    + {options.map((option, optionIndex) => { + const choice = text(option.option); + if (!choice) return null; + const verdict = text(option.verdict); + const escalatesTo = text(option.escalatesTo); + return ( +
  • +
    + {verdict ? {verdict} : null} + {/* Who answers follows the option chosen, not the + topic: a lead who settles a weights question + off the cuff has priced nothing. */} + {escalatesTo ? ( + + escalates to {escalatesTo} + + ) : null} +
    +

    {choice}

    + +
    • + ); + })} +
+ )} +
+ ); + })} +
+
+ )} + + {blockingSet.length === 0 ? null : ( +
}> +
    + {blockingSet.map((entry, index) => { + const item = text(entry.item); + if (!item) return null; + return ( +
  • +

    {item}

    + + +
    • + ); + })} +
+
+ )} ); } @@ -147,7 +229,14 @@ function QualificationFields({ fields }: { fields: Record }) { )} {bands.length === 0 ? null : ( -
+ // Not "what each score means": these bands are the framework author's + // own calibration on their own scale, and `MOTION_BANDS` is the + // product's — four bands in basis points, shown as a badge on the same + // page. The shipped scorecard's five bands genuinely disagree with it + // at 7500 (Strategic, against "do not start compute"), so a heading + // that read as the product's verdict put two opposite instructions + // about one number on one screen. +
    {bands.map((band, index) => { const label = text(band.label); @@ -308,7 +397,30 @@ function PricingFields({ fields }: { fields: Record }) { const inputs = recordList(fields.inputs); const packages = recordList(fields.packages); const tradeables = recordList(fields.tradeables); - if (inputs.length === 0 && packages.length === 0 && tradeables.length === 0) return null; + const budgetSources = recordList(fields.budgetSources); + const forecast = asRecord(fields.computeForecast); + const questionnaire = recordList(fields.questionnaireMap); + const justification = asRecord(fields.soleSourceJustification); + const steps = recordList(fields.steps); + const championHomework = recordList(fields.championHomework); + if ( + inputs.length === 0 && + packages.length === 0 && + tradeables.length === 0 && + budgetSources.length === 0 && + !forecast && + questionnaire.length === 0 && + !justification && + steps.length === 0 && + championHomework.length === 0 + ) { + return null; + } + + // Serial is the number the champion's own calendar produces if nobody runs + // anything beside anything else, and it is what makes the `parallelWith` + // column below worth reading. + const serialDays = steps.reduce((total, step) => total + (number(step.typicalDays) ?? 0), 0); return ( <> @@ -356,6 +468,130 @@ function PricingFields({ fields }: { fields: Record }) { />
)} + + {budgetSources.length === 0 ? null : ( +
+
+ {budgetSources.map((entry, index) => { + const source = text(entry.source); + if (!source) return null; + const days = number(entry.typicalDays); + return ( +
+
+ {source} + {entry.fastest === true ? Fastest : null} + {days === null ? null : ( + ~{days} days + )} +
+ + {/* Speed and durability are different questions and the + fastest source is routinely the least durable one, so + neither is shown without the other. */} + + + +
+ ); + })} +
+
+ )} + + {forecast ? ( +
+ + [ + text(line.line), + text(line.low), + text(line.expected), + text(line.high), + ])} + /> + + {/* Cost is charged against the full commitment, not the hours that + sold — AGENTS.md §4 — so the rule travels with the forecast that + tempts a reader to model it the other way. */} + +
+ ) : null} + + {questionnaire.length === 0 ? null : ( +
+ [text(entry.topic), text(entry.source)])} + /> +
+ )} + + {justification ? ( +
+ + {recordList(justification.paragraphs).map((paragraph, index) => { + const heading = text(paragraph.heading); + if (!heading) return null; + const draft = text(paragraph.draft); + return ( +
+

{heading}

+ {draft ? ( + // The champion pastes this into their own requisition, so + // the whitespace the author wrote is part of the paragraph. +

+ {draft} +

+ ) : null} +
+ ); + })} + +
+ ) : null} + + {steps.length === 0 ? null : ( +
0 ? `${serialDays} days end to end, serially` : undefined} + > +
    + {steps.map((entry, index) => { + const step = text(entry.step); + if (!step) return null; + const days = number(entry.typicalDays); + return ( +
  1. +
    + {step} + {days === null ? null : ( + ~{days} days + )} +
    + + + {/* Procurement never says no, it goes quiet, and the stated + reason for the silence is almost never the real one — so + the symptom and the move are the part that gets read. */} + + +
    2. + ); + })} +
+
+ )} + + {championHomework.length === 0 ? null : ( +
+ [text(entry.task), text(entry.why)])} + /> +
+ )} ); } @@ -473,7 +709,21 @@ function PlaybookFields({ fields }: { fields: Record }) { const stages = recordList(fields.stages); const research = asRecord(fields.researchInterface); const promotion = recordList(fields.promotion); - if (stages.length === 0 && !research && promotion.length === 0) return null; + const checks = recordList(fields.checks); + const fallbacks = recordList(fields.fallbacks); + const firstThirtyDays = recordList(fields.firstThirtyDays); + if ( + stages.length === 0 && + !research && + promotion.length === 0 && + checks.length === 0 && + fallbacks.length === 0 && + firstThirtyDays.length === 0 + ) { + return null; + } + + const gateCount = checks.filter((check) => check.gate === true).length; return ( <> @@ -504,6 +754,79 @@ function PlaybookFields({ fields }: { fields: Record }) {
) : null} + {checks.length === 0 ? null : ( +
0 ? `${gateCount} of ${checks.length} are gates` : undefined} + > +
    + {checks.map((entry, index) => { + const check = text(entry.check); + if (!check) return null; + const area = text(entry.area); + const id = text(entry.id); + return ( +
  • +
    + {area ? {area} : null} + {/* A gate stops the traffic ramp rather than being work + somebody catches up on, which is the same reason a + POC's kill gate is the one property that earns colour. */} + {entry.gate === true ? Gate : null} + {/* The fallbacks and the first thirty days name checks by + id, so the id is content rather than a React key. */} + {id ? {id} : null} +
    +

    {check}

    + + +
    • + ); + })} +
+
+ )} + + {fallbacks.length === 0 ? null : ( +
}> +
    + {fallbacks.map((entry, index) => { + const failure = text(entry.failure); + if (!failure) return null; + return ( +
  • +

    {failure}

    + {/* A response nobody can trigger is not a fallback, so how + the failure is detected is shown before what to do. */} + + +
    • + ); + })} +
+
+ )} + + {firstThirtyDays.length === 0 ? null : ( +
+
    + {firstThirtyDays.map((entry, index) => { + const when = text(entry.when); + if (!when) return null; + return ( +
  1. +

    {when}

    + + +
    2. + ); + })} +
+
+ )} + {promotion.length === 0 ? null : (
{ diff --git a/apps/web/src/pages/MotionEngagements.tsx b/apps/web/src/pages/MotionEngagements.tsx index b182e08..f5f8cf7 100644 --- a/apps/web/src/pages/MotionEngagements.tsx +++ b/apps/web/src/pages/MotionEngagements.tsx @@ -394,6 +394,7 @@ function OpenEngagementSheet({ const taken = new Set((allEngagements.data?.engagements ?? []).map((row) => row.demandDealId)); const available = (deals.data?.deals ?? []).filter((row) => !taken.has(row.deal.id)); + const failed = deals.isError || allEngagements.isError; const create = useMutation({ mutationFn: () => @@ -445,7 +446,39 @@ function OpenEngagementSheet({ - {!deals.isLoading && !allEngagements.isLoading && available.length === 0 ? ( + {/* + A failed read is not an empty book. Without this the sheet told + people "every demand deal already has an engagement" when the deal + query had simply errored. The engagement query's failure has to be + said too rather than tolerated: `taken` is then empty, so the list + is every deal including the ones already running, and choosing one + of those can only answer 409. + */} + {failed ? ( + <> +

+ {(deals.error ?? allEngagements.error)?.message ?? + 'The deal list could not be loaded.'} +

+

+ {deals.isError + ? 'No deal can be offered until this loads.' + : 'Deals that already have an engagement cannot be filtered out, so a choice here may be refused.'} +

+ + + ) : !deals.isLoading && !allEngagements.isLoading && available.length === 0 ? (

Every demand deal already has an engagement. Open one from the deal in Pipeline once there is a new deal to run. diff --git a/apps/web/src/pages/MotionTemplate.tsx b/apps/web/src/pages/MotionTemplate.tsx index ef67642..285131c 100644 --- a/apps/web/src/pages/MotionTemplate.tsx +++ b/apps/web/src/pages/MotionTemplate.tsx @@ -403,8 +403,36 @@ export function MotionTemplate() { + {/* + Pending and failed are answered before empty. "No open engagement to + instantiate into" is a claim about the book, and it was being made + while the query was still in flight and again when it had failed — + so a reader was told to go and open an engagement they already have. + `isPending` is a safe reading of "still loading" here only because + both halves of this query's `enabled` are already established: + `mayWrite` by the arm above, `id` by the detail data this branch is + rendered from. + */} {!mayWrite ? (

{WRITE_DENIED}

+ ) : engagements.isPending ? ( + + ) : engagements.isError ? ( + <> +

+ {engagements.error instanceof Error + ? engagements.error.message + : 'The open engagements could not be loaded.'} +

+ + ) : openEngagements.length === 0 ? (

No open engagement to instantiate into.{' '} diff --git a/docs/agents.md b/docs/agents.md index 2af36fc..d93d70e 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -83,6 +83,7 @@ and is redacted if an upstream error happens to echo it. | `pig_inventory_search` | What could we buy to cover demand we cannot serve? | | `pig_search` | Find an account | | `pig_get_account` | Everything about one account | +| `pig_motion_library` | What practice have we already written for this stage? | | `pig_log_activity` | Record a call, meeting or note | `pig_capacity_match` is the one worth learning. Ask it in plain language: @@ -96,10 +97,10 @@ break-even. ## Why the surface is small -Nine tools, each doing one thing. A sprawling tool list measurably degrades +Ten tools, each doing one thing. A sprawling tool list measurably degrades model performance, and anything genuinely niche is reachable through `pig_search` or the HTTP API. If you need something that is not here, it is -probably better added as a service method than as a tenth tool. +probably better added as a service method than as an eleventh tool. ## What it cannot do diff --git a/packages/db/migrations/0015_motion.sql b/packages/db/migrations/0015_motion.sql index 7431a05..89365e0 100644 --- a/packages/db/migrations/0015_motion.sql +++ b/packages/db/migrations/0015_motion.sql @@ -98,6 +98,7 @@ ALTER TABLE "engagement_artifacts" ADD CONSTRAINT "engagement_artifacts_promoted ALTER TABLE "motion_templates" ADD CONSTRAINT "motion_templates_origin_artifact_fk" FOREIGN KEY ("origin_artifact_id") REFERENCES "public"."engagement_artifacts"("id") ON DELETE set null ON UPDATE no action;--> statement-breakpoint ALTER TABLE "qualification_scores" ADD CONSTRAINT "qualification_scores_engagement_id_engagements_id_fk" FOREIGN KEY ("engagement_id") REFERENCES "public"."engagements"("id") ON DELETE cascade ON UPDATE no action;--> statement-breakpoint ALTER TABLE "qualification_scores" ADD CONSTRAINT "qualification_scores_framework_template_fk" FOREIGN KEY ("framework_template_id") REFERENCES "public"."motion_templates"("id") ON DELETE set null ON UPDATE no action;--> statement-breakpoint +ALTER TABLE "qualification_scores" ADD CONSTRAINT "qualification_scores_scored_by_user_id_users_id_fk" FOREIGN KEY ("scored_by_user_id") REFERENCES "public"."users"("id") ON DELETE set null ON UPDATE no action;--> statement-breakpoint CREATE INDEX "motion_templates_kind_stage_idx" ON "motion_templates" USING btree ("kind","stage");--> statement-breakpoint CREATE INDEX "motion_templates_visibility_kind_idx" ON "motion_templates" USING btree ("visibility","kind");--> statement-breakpoint CREATE INDEX "motion_templates_owner_idx" ON "motion_templates" USING btree ("owner_user_id");--> statement-breakpoint diff --git a/packages/db/src/seed/demo/clear.ts b/packages/db/src/seed/demo/clear.ts index 2c3a477..0c66be5 100644 --- a/packages/db/src/seed/demo/clear.ts +++ b/packages/db/src/seed/demo/clear.ts @@ -7,7 +7,7 @@ * situation this command is for — someone demoing on top of their own data — * and it must leave that data untouched. */ -import { eq, inArray, like, sql } from 'drizzle-orm'; +import { eq, inArray, like, or, sql } from 'drizzle-orm'; import { unstampDemoActivity } from './activities'; import { accounts, @@ -96,11 +96,23 @@ export async function clear(context: DemoContext): Promise { * does. The starter library itself is NOT touched: it is authored product * content from the base seed, carries no prefix, and survives `--clear` the * same way the PIG-hosted learn rows do. + * + * Found by parentage as well as by prefix, because the cascade off the demo + * deals takes a wider set than the prefix does: an engagement opened on a + * demo deal through the app carries whatever summary its author typed, or + * none. Every one of those the prefix scan missed would be deleted anyway a + * few lines below, silently, with its artefacts' borrowed `usage_count` + * never given back — and a starter template left above zero can never be + * edited again. */ const demoEngagements = await db .select({ id: engagements.id }) .from(engagements) - .where(like(engagements.summary, `${prefix}%`)); + .where( + demandDealIds.length > 0 + ? or(like(engagements.summary, `${prefix}%`), inArray(engagements.demandDealId, demandDealIds)) + : like(engagements.summary, `${prefix}%`), + ); const engagementIds = demoEngagements.map((engagement) => engagement.id); if (engagementIds.length > 0) { /* diff --git a/packages/db/src/seed/motion/trainability-qualification.json b/packages/db/src/seed/motion/trainability-qualification.json index 029925d..57f4657 100644 --- a/packages/db/src/seed/motion/trainability-qualification.json +++ b/packages/db/src/seed/motion/trainability-qualification.json @@ -4,7 +4,7 @@ "title": "Trainability and Deal Qualification Scorecard", "summary": "A weighted, anchored scorecard for deciding whether a customer's problem is a post-training problem Prime Intellect can win and whether the deal behind it is real — run it at the end of technical discovery, before anything goes to legal.", "stage": "qualification", - "body": "*Playbook stage: qualification, at its close.* Score this from the notes of **First Technical Discovery**, before the deal enters legal. The verifier floor used below is set once, in the **Strategic Deployment Playbook**.\n\n## What this scores\n\nThis is not BANT. BANT tells you whether someone can buy something; it says nothing about whether the thing they want to buy can be built. In post-training, most lost deals are not lost on price or to a competitor. They are lost because the customer's problem was never a trainable problem, and nobody said so in week two.\n\nThis framework scores two independent questions and refuses to average them into one comfortable number:\n\n1. **Technical fit.** Can this workflow be turned into an environment with a verifier, trained against a measured baseline, and shown to improve? (57 points)\n2. **Commercial reality.** Is there a person who owns the metric, money that already exists, and a legal path that closes before compute is committed? (43 points)\n\nRun it at the end of technical discovery, before you write a scoping document. Re-run it after environment design, because the verifier score in particular almost always moves, and usually down.\n\n## How to score\n\nEach dimension is scored 0-4 against its anchors. Anchors describe **observable evidence**, not your confidence. If you cannot point to the artefact the anchor names — the file, the dashboard, the name, the contract clause — you score the level below. \"They said they have it\" is a 1, not a 3.\n\nWeighted score = Σ(weight × score) ÷ 4, giving a 0-100 result. Weights sum to 100 and scores top out at 4, so straight 4s are exactly 100.\n\nTwo derived numbers matter as much as the total:\n\n- **Group subtotals**, reported separately and never only as a blend. Technical is out of 57, commercial out of 43.\n- **Weighted point loss** per dimension: weight × (4 − score). This is what the conditional band acts on, and it is not the same thing as a low weighted score. A perfect 4 on executive sponsor contributes 16 weighted points and loses none; a 3 on verifier quality contributes 39 and loses 13. The dimension to fix is the one losing the most points, not the one contributing the fewest. Point loss across all twelve dimensions always sums to 400 minus the weighted total, which is a useful arithmetic check.\n\n## Score bands and what to do\n\n| Band | Label | Action |\n|---|---|---|\n| 78-100 | Build | Move to scoping. Commit named research support, write the environment and verifier design into the proposal, and put a dated POC gate with the metric owner's agreed threshold on it. |\n| 62-77 | Qualified, conditional | Advance, but name the two dimensions with the largest weighted point loss — max weight × (4 − score), breaking ties toward the technical group — as written POC entry conditions with owners and dates. Do not start compute until both are cleared. |\n| 46-61 | Scope down | Do not sell the workflow as described. Find the narrowest sub-task that would itself score above 62, and sell that as a paid evaluation or a single environment build with a fixed price and a fixed end. |\n| 30-45 | Defer | No POC. Offer evaluations or on-demand compute if genuinely useful, name the artefact that must exist before re-scoring, set a re-qualification date, and stop talking about post-training until then. |\n| 0-29 | Decline | Decline in the next meeting, verbally, naming the missing artefact rather than the customer. Recommend the cheaper thing that would actually work now, and leave an explicit trigger that brings them back. Log the reason in PIG as closed_lost with the disqualifying dimension recorded. |\n\n### On declining\n\nDeclining well is the highest-leverage thing in this document, because a bad POC consumes delivery capacity that cannot be recovered — assume {{FDE-weeks}} per POC, a figure to fill in from your own delivery data rather than one this document can supply — and it produces a reference customer who tells people it did not work. Decline in the room, not in an email chain that dies.\n\nSay roughly this: *\"I don't think this is a post-training problem yet, and I'd rather tell you that than sell you six weeks that end in a shrug. Right now nobody can say what a correct output looks like without {{named senior person}} reading it. Until that judgement is written down as a check something else can run, training has nothing to optimise against. Here's what I'd do instead: {{prompting / retrieval / workflow change}}. When you have {{the specific missing artefact}}, come back and we will move fast.\"*\n\nThree things that does: it names the missing artefact rather than blaming them, it gives them a cheaper thing that actually works, and it leaves a trigger condition that brings them back. Never decline on \"we're not a fit\". That is a door closing with no handle on it.\n\n## The artefact that unblocks each dimension\n\nEvery band above tells you to name an artefact. Naming it is the whole job, so here is the mapping rather than the instruction.\n\n| Dimension | Artefact the customer must produce |\n|---|---|\n| Task definability | A written spec plus ten worked examples, with a second reviewer's independent verdict recorded on each |\n| Verifier availability and quality | At least 200 human-labelled items on a held-out set, stratified so the minority outcome is at least 30 of them, and a measured judge-versus-human agreement number on them |\n| Traces, volume, and rights to use them | Counsel's written confirmation that the traces may be used for model training, plus a counted volume produced by a query rather than from memory |\n| Measured baseline | A held-out set drawn from a time window after the training traces, a scoring script the customer can rerun, and a recorded frontier-model or human number |\n| Headroom over prompting | A categorised failure set from a serious prompted baseline |\n| Environment constructibility | One containerised run of the workflow against seeded state with a scripted reset |\n| Named metric owner | That person's numeric success threshold, stated by them, in writing |\n| Budget line and its source | The budget code, and the named spend it displaces |\n| Executive sponsor | One logged instance of the executive removing a specific blocker |\n| Security and legal path | A comparable AI vendor's completed review, plus deployment topology and weights ownership agreed in writing |\n| External forcing function | The external date, and the quantified cost of missing it stated by the sponsor |\n| Expansion surface | Two named adjacent workflows with named owners |\n\n## Hard disqualifiers\n\nA weighted average is a machine for laundering one fatal problem into an acceptable number. Take a deal scoring 3 on everything, then move budget source, executive sponsor and forcing function to 4 and verifier quality to 0. It scores 70.5: a comfortable Qualified, conditional. It will still fail, because you cannot train against a reward you cannot compute. Averages assume dimensions substitute for each other. Several of these do not; they are gates.\n\nThe four disqualifiers below override the score entirely, and each caps the deal at a named band rather than at one generic outcome:\n\n- **No computable verifier and no path to one**, and **incompatible weights-ownership or deployment expectation**, cap at **Decline (0-29)**. Neither softens with time or effort inside this deal, and routing them to Defer would leave the customer waiting on something that is not coming. Use the decline script.\n- **No rights to train on the data**, and **nobody states the number**, cap at **Defer (30-45)**, with the unblocking artefact from the table above named explicitly and a re-qualification date set. A motivated customer can produce both.\n\nRecord which one fired on the demand record in PIG. The pattern across a quarter of disqualified deals is the most useful thing the field sends back to the people designing environments.\n\n## Worked example\n\nAn anonymised deal, scored across all twelve dimensions at the end of technical discovery.\n\n| Dimension | Weight | Score | Weighted | Point loss |\n|---|---|---|---|---|\n| Task definability | 10 | 3 | 30 | 10 |\n| Verifier availability and quality | 13 | 2 | 26 | 26 |\n| Traces, volume, and rights to use them | 8 | 3 | 24 | 8 |\n| Measured baseline | 8 | 2 | 16 | 16 |\n| Headroom over prompting and workflow design | 10 | 3 | 30 | 10 |\n| Environment constructibility | 8 | 3 | 24 | 8 |\n| Named metric owner | 9 | 3 | 27 | 9 |\n| Budget line and its source | 9 | 4 | 36 | 0 |\n| Executive sponsor | 4 | 4 | 16 | 0 |\n| Security and legal path | 8 | 3 | 24 | 8 |\n| External forcing function | 8 | 4 | 32 | 0 |\n| Expansion surface | 5 | 3 | 15 | 5 |\n| **Total** | **100** | | **300** | **100** |\n\nWeighted total 300, so the score is 300 ÷ 4 = **75**. The subtotals happen to be equal in raw weighted points, 150 each, which is 37.5 of a possible 57 on technical and 37.5 of 43 on commercial — a deal that is commercially stronger than it is trainable, which the blended 75 does not tell you. Point loss sums to 100, which is 400 − 300, so the arithmetic checks.\n\n75 lands in **Qualified, conditional (62-77)**. The two POC entry conditions are the two largest point losses: **verifier quality** (26) and **measured baseline** (16). Note what the discarded rule would have done: the two *lowest weighted scores* are expansion surface (15) and executive sponsor (16), both of which are fine and neither of which can sink the POC. That is the whole reason the band acts on loss.\n\nNow the override. At the environment design workshop the verifier score is re-taken and drops to 1: the rubric exists but the judge has never been measured, and the sponsor is asked directly, on the record, to fund a 200-item labelling pass and declines. The arithmetic barely moves — 287 ÷ 4 = 71.75, still Qualified, conditional — but disqualifier 1 has fired, and it caps at Decline. The deal does not advance, and it gets the decline conversation rather than a re-qualification date, because there is no artefact anyone has agreed to produce.\n\n## Reading the two groups separately\n\nAlways report the two subtotals, never only the blend.\n\n- **High technical, low commercial.** A real training problem inside an organisation that cannot buy it. It burns delivery months on beautiful environments nobody funds. Correct move: an inexpensive paid evaluation that puts a number in front of an executive, so the buyer gets created rather than assumed.\n- **Low technical, high commercial.** Money, sponsor, urgency, and a task that is not trainable. This is the dangerous one, because everything social in the deal pushes you forward. Sell inference or reserved compute if that is genuinely useful, and keep post-training out of the contract. Selling a training programme into an unverifiable task is how you convert a well-funded champion into a detractor.\n- **Both mid.** Usually one workflow doing the work of three. Split it. The narrow version of a sprawling request is often a 3 or 4 across the technical group.\n\n## What the scoring conversation actually sounds like\n\nThe anchors are written so that you can score them from a discovery call if you ask forcing questions. Some that work:\n\n- \"Show me the last ten outputs of this workflow and tell me which ones were wrong.\" (Scores task definability and verifier quality in one move. If they argue about three of the ten, that argument *is* the verifier problem.)\n- \"Who gets asked about this number in their performance review?\" (Metric owner. If the answer is a committee, it is a 1.)\n- \"What is the budget code, and what was it going to be spent on if we don't do this?\" (Budget source. Displacement budget is real; net-new budget in an unapproved plan is a 1.)\n- \"How many of these did you run last month, and where is that logged?\" (Trace availability. A number from memory is a 1; a query against a table is a 3.)\n- \"If we do nothing, what happens on {{date}}?\" (Forcing function. If the honest answer is \"nothing\", it is a 0, whatever the enthusiasm level.)\n- \"What is the latency budget and the per-item cost ceiling for this in production?\" (Not scored — see the weight rationale — but a task that must answer in 300ms for a fraction of a cent changes the whole programme, and you want that on the table before scoping.)\n- \"Can you send me the DPA you signed with your last inference vendor?\" (Security path. An existing executed agreement with a comparable vendor is worth more than any assurance about process.)\n\n## Where the score belongs in the pipeline\n\nScore at **qualification**, before legal. This is deliberate and it matters in this pipeline: legal sits second, so an unqualified deal does not merely waste your time, it consumes counsel's time on an MSA and DPA for an engagement that should never have started. The score is the gate that protects the second stage.\n\nAttach the completed scorecard to the demand record in PIG. On any deal that later reaches closed_lost, the delta between the qualification score and the outcome is the only honest feedback loop this motion has. Deals that scored above 78 and still lost are telling you a dimension is missing or mis-weighted. Deals that scored below 46, were pushed through anyway, and won are telling you the same thing in the other direction, and are rarer than the optimists in the room believe.\n\n## Weight rationale\n\nVerifier quality carries the single largest weight (13) because it is the closest thing in this business to a binary. Everything downstream — environment, reward, eval, the improvement claim in the case study — is built on it, and no amount of budget compensates for its absence. Task definability (10) and headroom over prompting (10) come next: the first determines whether an environment can exist, the second whether anyone should pay for one rather than spending an afternoon on a better prompt. On the commercial side, metric owner (9) and budget source (9) outweigh executive sponsor (4) on purpose. Sponsors change jobs; a named person whose review depends on the number does not stop caring about it, and they are the one who will defend the result internally when it lands short of the deck.\n\nTwo things a reader may expect to find scored here are deliberately not dimensions. **Latency and cost envelope** is a constraint, not a measure of fit: it does not make a task more or less trainable, it decides what model size and serving topology the trained result must fit into, and a task can be perfectly trainable and still uneconomic at 300ms. Constraints belong in scoping, where they change the design, rather than in a score, where a hard limit would be averaged away — which is the same failure the disqualifiers exist to prevent. Ask the question in discovery, record the answer in the scoping document, and if the envelope is genuinely unmeetable, that is a decline on feasibility rather than a low score. **Long-horizon versus single-shot** is not scored separately because it does not vary independently of the dimensions that are: a long-horizon task shows up as a hard verifier (credit assignment over a trajectory), a harder environment to construct (state, resets, episode boundaries) and larger headroom over prompting. Scoring it again would triple-count the same fact. It appears where it is observable, in the top anchors of headroom and environment constructibility.", + "body": "*Playbook stage: qualification, at its close.* Score this from the notes of **First Technical Discovery**, before the deal enters legal. The verifier floor used below is set once, in the **Strategic Deployment Playbook**.\n\n## What this scores\n\nThis is not BANT. BANT tells you whether someone can buy something; it says nothing about whether the thing they want to buy can be built. In post-training, most lost deals are not lost on price or to a competitor. They are lost because the customer's problem was never a trainable problem, and nobody said so in week two.\n\nThis framework scores two independent questions and refuses to average them into one comfortable number:\n\n1. **Technical fit.** Can this workflow be turned into an environment with a verifier, trained against a measured baseline, and shown to improve? (57 points)\n2. **Commercial reality.** Is there a person who owns the metric, money that already exists, and a legal path that closes before compute is committed? (43 points)\n\nRun it at the end of technical discovery, before you write a scoping document. Re-run it after environment design, because the verifier score in particular almost always moves, and usually down.\n\n## How to score\n\nEach dimension is scored 0-4 against its anchors. Anchors describe **observable evidence**, not your confidence. If you cannot point to the artefact the anchor names — the file, the dashboard, the name, the contract clause — you score the level below. \"They said they have it\" is a 1, not a 3.\n\nWeighted score = Σ(weight × score) ÷ 4, giving a 0-100 result. Weights sum to 100 and scores top out at 4, so straight 4s are exactly 100.\n\nTwo derived numbers matter as much as the total:\n\n- **Group subtotals**, reported separately and never only as a blend. Technical is out of 57, commercial out of 43.\n- **Weighted point loss** per dimension: weight × (4 − score). This is what the conditional band acts on, and it is not the same thing as a low weighted score. A perfect 4 on executive sponsor contributes 16 weighted points and loses none; a 3 on verifier quality contributes 39 and loses 13. The dimension to fix is the one losing the most points, not the one contributing the fewest. Point loss across all twelve dimensions always sums to 400 minus the weighted total, which is a useful arithmetic check.\n\n## Score bands and what to do\n\nThe badge PIG puts beside a saved score is its own coarser four-band reading of the same number, product-wide and unaware of this framework; the bands below are this framework's own calibration, and they are what every action in this document is written against.\n\n| Band | Label | Action |\n|---|---|---|\n| 78-100 | Build | Move to scoping. Commit named research support, write the environment and verifier design into the proposal, and put a dated POC gate with the metric owner's agreed threshold on it. |\n| 62-77 | Qualified, conditional | Advance, but name the two dimensions with the largest weighted point loss — max weight × (4 − score), breaking ties toward the technical group — as written POC entry conditions with owners and dates. Do not start compute until both are cleared. |\n| 46-61 | Scope down | Do not sell the workflow as described. Find the narrowest sub-task that would itself score above 62, and sell that as a paid evaluation or a single environment build with a fixed price and a fixed end. |\n| 30-45 | Defer | No POC. Offer evaluations or on-demand compute if genuinely useful, name the artefact that must exist before re-scoring, set a re-qualification date, and stop talking about post-training until then. |\n| 0-29 | Decline | Decline in the next meeting, verbally, naming the missing artefact rather than the customer. Recommend the cheaper thing that would actually work now, and leave an explicit trigger that brings them back. Log the reason in PIG as closed_lost with the disqualifying dimension recorded. |\n\n### On declining\n\nDeclining well is the highest-leverage thing in this document, because a bad POC consumes delivery capacity that cannot be recovered — assume {{FDE-weeks}} per POC, a figure to fill in from your own delivery data rather than one this document can supply — and it produces a reference customer who tells people it did not work. Decline in the room, not in an email chain that dies.\n\nSay roughly this: *\"I don't think this is a post-training problem yet, and I'd rather tell you that than sell you six weeks that end in a shrug. Right now nobody can say what a correct output looks like without {{named senior person}} reading it. Until that judgement is written down as a check something else can run, training has nothing to optimise against. Here's what I'd do instead: {{prompting / retrieval / workflow change}}. When you have {{the specific missing artefact}}, come back and we will move fast.\"*\n\nThree things that does: it names the missing artefact rather than blaming them, it gives them a cheaper thing that actually works, and it leaves a trigger condition that brings them back. Never decline on \"we're not a fit\". That is a door closing with no handle on it.\n\n## The artefact that unblocks each dimension\n\nEvery band above tells you to name an artefact. Naming it is the whole job, so here is the mapping rather than the instruction.\n\n| Dimension | Artefact the customer must produce |\n|---|---|\n| Task definability | A written spec plus ten worked examples, with a second reviewer's independent verdict recorded on each |\n| Verifier availability and quality | At least 200 human-labelled items on a held-out set, stratified so the minority outcome is at least 30 of them, and a measured judge-versus-human agreement number on them |\n| Traces, volume, and rights to use them | Counsel's written confirmation that the traces may be used for model training, plus a counted volume produced by a query rather than from memory |\n| Measured baseline | A held-out set drawn from a time window after the training traces, a scoring script the customer can rerun, and a recorded frontier-model or human number |\n| Headroom over prompting | A categorised failure set from a serious prompted baseline |\n| Environment constructibility | One containerised run of the workflow against seeded state with a scripted reset |\n| Named metric owner | That person's numeric success threshold, stated by them, in writing |\n| Budget line and its source | The budget code, and the named spend it displaces |\n| Executive sponsor | One logged instance of the executive removing a specific blocker |\n| Security and legal path | A comparable AI vendor's completed review, plus deployment topology and weights ownership agreed in writing |\n| External forcing function | The external date, and the quantified cost of missing it stated by the sponsor |\n| Expansion surface | Two named adjacent workflows with named owners |\n\n## Hard disqualifiers\n\nA weighted average is a machine for laundering one fatal problem into an acceptable number. Take a deal scoring 3 on everything, then move budget source, executive sponsor and forcing function to 4 and verifier quality to 0. It scores 70.5: a comfortable Qualified, conditional. It will still fail, because you cannot train against a reward you cannot compute. Averages assume dimensions substitute for each other. Several of these do not; they are gates.\n\nThe four disqualifiers below override the score entirely, and each caps the deal at a named band rather than at one generic outcome:\n\n- **No computable verifier and no path to one**, and **incompatible weights-ownership or deployment expectation**, cap at **Decline (0-29)**. Neither softens with time or effort inside this deal, and routing them to Defer would leave the customer waiting on something that is not coming. Use the decline script.\n- **No rights to train on the data**, and **nobody states the number**, cap at **Defer (30-45)**, with the unblocking artefact from the table above named explicitly and a re-qualification date set. A motivated customer can produce both.\n\nRecord which one fired on the demand record in PIG. The pattern across a quarter of disqualified deals is the most useful thing the field sends back to the people designing environments.\n\n## Worked example\n\nAn anonymised deal, scored across all twelve dimensions at the end of technical discovery.\n\n| Dimension | Weight | Score | Weighted | Point loss |\n|---|---|---|---|---|\n| Task definability | 10 | 3 | 30 | 10 |\n| Verifier availability and quality | 13 | 2 | 26 | 26 |\n| Traces, volume, and rights to use them | 8 | 3 | 24 | 8 |\n| Measured baseline | 8 | 2 | 16 | 16 |\n| Headroom over prompting and workflow design | 10 | 3 | 30 | 10 |\n| Environment constructibility | 8 | 3 | 24 | 8 |\n| Named metric owner | 9 | 3 | 27 | 9 |\n| Budget line and its source | 9 | 4 | 36 | 0 |\n| Executive sponsor | 4 | 4 | 16 | 0 |\n| Security and legal path | 8 | 3 | 24 | 8 |\n| External forcing function | 8 | 4 | 32 | 0 |\n| Expansion surface | 5 | 3 | 15 | 5 |\n| **Total** | **100** | | **300** | **100** |\n\nWeighted total 300, so the score is 300 ÷ 4 = **75**. The subtotals happen to be equal in raw weighted points, 150 each, which is 37.5 of a possible 57 on technical and 37.5 of 43 on commercial — a deal that is commercially stronger than it is trainable, which the blended 75 does not tell you. Point loss sums to 100, which is 400 − 300, so the arithmetic checks.\n\n75 lands in **Qualified, conditional (62-77)**. The two POC entry conditions are the two largest point losses: **verifier quality** (26) and **measured baseline** (16). Note what the discarded rule would have done: the two *lowest weighted scores* are expansion surface (15) and executive sponsor (16), both of which are fine and neither of which can sink the POC. That is the whole reason the band acts on loss.\n\nNow the override. At the environment design workshop the verifier score is re-taken and drops to 1: the rubric exists but the judge has never been measured, and the sponsor is asked directly, on the record, to fund a 200-item labelling pass and declines. The arithmetic barely moves — 287 ÷ 4 = 71.75, still Qualified, conditional — but disqualifier 1 has fired, and it caps at Decline. The deal does not advance, and it gets the decline conversation rather than a re-qualification date, because there is no artefact anyone has agreed to produce.\n\n## Reading the two groups separately\n\nAlways report the two subtotals, never only the blend.\n\n- **High technical, low commercial.** A real training problem inside an organisation that cannot buy it. It burns delivery months on beautiful environments nobody funds. Correct move: an inexpensive paid evaluation that puts a number in front of an executive, so the buyer gets created rather than assumed.\n- **Low technical, high commercial.** Money, sponsor, urgency, and a task that is not trainable. This is the dangerous one, because everything social in the deal pushes you forward. Sell inference or reserved compute if that is genuinely useful, and keep post-training out of the contract. Selling a training programme into an unverifiable task is how you convert a well-funded champion into a detractor.\n- **Both mid.** Usually one workflow doing the work of three. Split it. The narrow version of a sprawling request is often a 3 or 4 across the technical group.\n\n## What the scoring conversation actually sounds like\n\nThe anchors are written so that you can score them from a discovery call if you ask forcing questions. Some that work:\n\n- \"Show me the last ten outputs of this workflow and tell me which ones were wrong.\" (Scores task definability and verifier quality in one move. If they argue about three of the ten, that argument *is* the verifier problem.)\n- \"Who gets asked about this number in their performance review?\" (Metric owner. If the answer is a committee, it is a 1.)\n- \"What is the budget code, and what was it going to be spent on if we don't do this?\" (Budget source. Displacement budget is real; net-new budget in an unapproved plan is a 1.)\n- \"How many of these did you run last month, and where is that logged?\" (Trace availability. A number from memory is a 1; a query against a table is a 3.)\n- \"If we do nothing, what happens on {{date}}?\" (Forcing function. If the honest answer is \"nothing\", it is a 0, whatever the enthusiasm level.)\n- \"What is the latency budget and the per-item cost ceiling for this in production?\" (Not scored — see the weight rationale — but a task that must answer in 300ms for a fraction of a cent changes the whole programme, and you want that on the table before scoping.)\n- \"Can you send me the DPA you signed with your last inference vendor?\" (Security path. An existing executed agreement with a comparable vendor is worth more than any assurance about process.)\n\n## Where the score belongs in the pipeline\n\nScore at **qualification**, before legal. This is deliberate and it matters in this pipeline: legal sits second, so an unqualified deal does not merely waste your time, it consumes counsel's time on an MSA and DPA for an engagement that should never have started. The score is the gate that protects the second stage.\n\nAttach the completed scorecard to the demand record in PIG. On any deal that later reaches closed_lost, the delta between the qualification score and the outcome is the only honest feedback loop this motion has. Deals that scored above 78 and still lost are telling you a dimension is missing or mis-weighted. Deals that scored below 46, were pushed through anyway, and won are telling you the same thing in the other direction, and are rarer than the optimists in the room believe.\n\n## Weight rationale\n\nVerifier quality carries the single largest weight (13) because it is the closest thing in this business to a binary. Everything downstream — environment, reward, eval, the improvement claim in the case study — is built on it, and no amount of budget compensates for its absence. Task definability (10) and headroom over prompting (10) come next: the first determines whether an environment can exist, the second whether anyone should pay for one rather than spending an afternoon on a better prompt. On the commercial side, metric owner (9) and budget source (9) outweigh executive sponsor (4) on purpose. Sponsors change jobs; a named person whose review depends on the number does not stop caring about it, and they are the one who will defend the result internally when it lands short of the deck.\n\nTwo things a reader may expect to find scored here are deliberately not dimensions. **Latency and cost envelope** is a constraint, not a measure of fit: it does not make a task more or less trainable, it decides what model size and serving topology the trained result must fit into, and a task can be perfectly trainable and still uneconomic at 300ms. Constraints belong in scoping, where they change the design, rather than in a score, where a hard limit would be averaged away — which is the same failure the disqualifiers exist to prevent. Ask the question in discovery, record the answer in the scoping document, and if the envelope is genuinely unmeetable, that is a decline on feasibility rather than a low score. **Long-horizon versus single-shot** is not scored separately because it does not vary independently of the dimensions that are: a long-horizon task shows up as a hard verifier (credit assignment over a trajectory), a harder environment to construct (state, resets, episode boundaries) and larger headroom over prompting. Scoring it again would triple-count the same fact. It appears where it is observable, in the top anchors of headroom and environment constructibility.", "fields": { "dimensions": [ { diff --git a/scripts/motion-fields-check.mjs b/scripts/motion-fields-check.mjs new file mode 100644 index 0000000..c6857e3 --- /dev/null +++ b/scripts/motion-fields-check.mjs @@ -0,0 +1,85 @@ +/* + * Does every seeded template's `fields` actually reach the screen? + * + * node scripts/motion-fields-check.mjs # PIG_WEB_URL=http://127.0.0.1:8975 + * + * Three of the twelve starter templates shipped with `fields` shapes no renderer + * in `FieldsView` read — `decisions`, `blockingSet`, `checks`, `steps` and the + * rest — so about forty authored records rendered as no DOM at all, in the + * library and again on the engagement that instantiated them. Nothing failed: + * each renderer returned null for a key set it did not recognise, and a template + * page that is header-plus-body looks like a template that was written that way. + * + * So the check is not "does the page load". It samples the actual authored + * strings out of the seed JSON and asserts they are present in the rendered + * text, which is the only claim that distinguishes a rendered field from a + * dropped one. + */ +import { chromium } from 'playwright'; +import { readdirSync, readFileSync } from 'node:fs'; +import { join } from 'node:path'; + +const BASE = process.env.PIG_WEB_URL ?? 'http://127.0.0.1:8975'; +const SEEDS = 'packages/db/src/seed/motion'; + +/** Every string of real prose in a fields tree, longest first. */ +function strings(node, out = []) { + if (typeof node === 'string') out.push(node); + else if (Array.isArray(node)) node.forEach((n) => strings(n, out)); + else if (node && typeof node === 'object') Object.values(node).forEach((n) => strings(n, out)); + return out; +} + +const templates = readdirSync(SEEDS) + .filter((f) => f.endsWith('.json')) + .map((f) => JSON.parse(readFileSync(join(SEEDS, f), 'utf8'))) + .filter((t) => t.fields && Object.keys(t.fields).length > 0); + +const live = await fetch(`${BASE}/api/motion/templates?all=1`).then((r) => r.json()); +const bySlug = new Map(live.templates.map((t) => [t.slug, t])); + +const browser = await chromium.launch({ channel: 'chrome' }); +const ctx = await browser.newContext({ viewport: { width: 1440, height: 900 } }); +let failures = 0; + +for (const template of templates) { + const row = bySlug.get(template.slug); + if (!row) { + console.log(`SKIP ${template.slug}: not in the seeded library`); + continue; + } + + // Long strings only: a short one ("Owner", "Yes") can appear in the chrome by + // coincidence and would make this pass on a page that dropped every field. + const sample = strings(template.fields) + .filter((s) => s.length > 45) + .sort((a, b) => b.length - a.length) + .slice(0, 6); + if (sample.length === 0) { + console.log(`SKIP ${template.slug}: no prose long enough to sample`); + continue; + } + + const page = await ctx.newPage(); + await page.goto(`${BASE}/motion/library/${row.id}`, { waitUntil: 'networkidle' }); + await page.waitForTimeout(900); + const text = await page.evaluate(() => document.body.innerText); + // The markdown body is rendered too, so a string that also appears there + // would not prove the FIELDS reached the screen. Those are excluded. + const fieldsOnly = sample.filter((s) => !template.body.includes(s)); + const checking = fieldsOnly.length ? fieldsOnly : sample; + const missing = checking.filter((s) => !text.includes(s)); + + if (missing.length) { + failures++; + console.log(`FAIL ${template.slug} (${template.kind}): ${missing.length}/${checking.length} authored strings absent`); + console.log(` e.g. "${missing[0].slice(0, 90)}…"`); + } else { + console.log(`ok ${template.slug} (${template.kind}): ${checking.length} authored strings on screen`); + } + await page.close(); +} + +await browser.close(); +console.log(failures === 0 ? '\nevery seeded fields tree reaches the screen' : `\n${failures} template(s) render authored content nowhere`); +process.exit(failures === 0 ? 0 : 1);