diff --git a/.changeset/20620-migrate-meta-review-pairing.md b/.changeset/20620-migrate-meta-review-pairing.md new file mode 100644 index 00000000000..2a24e68f8d3 --- /dev/null +++ b/.changeset/20620-migrate-meta-review-pairing.md @@ -0,0 +1,31 @@ +--- +'@objectstack/cli': patch +--- + +fix(cli): `os migrate meta --from N` prints the manual change that judges an applied edit beside that edit, marked review + +Clause-②: no + +Some applied mechanical edits are not the end of the job. A default flip such as +`flow-decision-mode-inclusive-explicit` writes `mode: 'inclusive'` onto a decision so +the flow keeps its old behaviour, and the manual change that judges it says the right +edit is usually none: delete the key where the branch conditions partition. That +judgment used to print hundreds of lines below the edit, among every other manual +change of the major. + +A manual change can now declare which conversions' edits it judges. In the +`Applied N mechanical change(s):` group, each run of edits by such a conversion is +followed by one line: + + ↳ review the N edits above against the manual change [protocol M] surface → replacement + +The line copies the headline the manual change prints in its own group, so its `why` +and `verify` lines are found there under the same text. Two pairs are declared today: +`flow-decision-mode-inclusive-explicit` with the decision-mode entry, and +`time-default-utc-suffix-dropped` with the time-default entry. Only declared links +pair; a manual change that merely mentions a conversion in its prose does not. + +Nothing else moves. The `N manual change(s) require your judgment:` group still lists +every manual change, byte for byte, with the same count. An edit no manual change +judges prints exactly as before. `--json`, `--out`, the exit code and the loader's +stderr are unchanged, and `--stored` is not affected. diff --git a/packages/cli/src/commands/migrate/meta.report-order.test.ts b/packages/cli/src/commands/migrate/meta.report-order.test.ts index fe4e3c10276..86a2e855510 100644 --- a/packages/cli/src/commands/migrate/meta.report-order.test.ts +++ b/packages/cli/src/commands/migrate/meta.report-order.test.ts @@ -24,23 +24,37 @@ * pins do not read positions across groups, so a reorder alone cannot turn * them red — and a dropped notice cannot hide behind a correct order. * + * ## The pairing pins (PAIR) + * + * Where a semantic entry declares in `conversionIds` that it judges a + * conversion's applied edits, ② prints that entry's headline beside those + * edits, marked review: once under each run of the conversion's edits, counting + * them. It is a copy, never a move: ③ still prints the entry, and ③'s lines and + * count stay the chain's. Declared links only, never prose: the pins enumerate + * the links off the registry and replay each linked conversion's own fixture, + * so a link the spec lane authors later is covered on the day it lands. + * * ## Why in-process, over a real chain run * * The inputs are not fabricated: the stack below is replayed through the real * `applyMetaMigrations` and parsed by the real `ObjectStackDefinitionSchema`, * exactly as the command does, and `printMigrationReport` is the function the * command's text face calls with them. Spawning the CLI would add a process - * and a config load and pin nothing more about the order. + * and a config load and pin nothing more about the order. One pin adds a link + * to a real result, and says so: the spec allows a link to a conversion an + * EARLIER step replays, and none is authored yet. */ import { stripVTControlCharacters } from 'node:util'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { ObjectStackDefinitionSchema, formatZodIssue, normalizeStackInput } from '@objectstack/spec'; +import { ALL_CONVERSIONS, ObjectStackDefinitionSchema, formatZodIssue, normalizeStackInput } from '@objectstack/spec'; import { applyMetaMigrations, + MIGRATIONS_BY_MAJOR, MIGRATION_MAJORS, MIGRATION_SUPPORT_FLOOR, type MigrationChainResult, + type MigrationTodo, } from '@objectstack/spec/migrations'; import { PROTOCOL_MAJOR } from '@objectstack/spec/kernel'; import { printMigrationReport, type MigrationReport } from './meta.js'; @@ -102,10 +116,19 @@ afterEach(() => { vi.restoreAllMocks(); }); -/** Replay the chain the way the command does, print the report, return its lines. */ -function run(stack: Record, fromMajor: number, toMajor: number): Run { +/** + * Replay the chain the way the command does, print the report, return its lines. + * `amend` edits the chain's result before it is printed — one pin uses it, to + * add a link no registry entry declares yet. + */ +function run( + stack: Record, + fromMajor: number, + toMajor: number, + amend: (result: MigrationChainResult) => MigrationChainResult = (r) => r, +): Run { const normalized = normalizeStackInput(stack, { convert: false }); - const result = applyMetaMigrations(normalized, fromMajor, toMajor); + const result = amend(applyMetaMigrations(normalized, fromMajor, toMajor)); const parsed = ObjectStackDefinitionSchema.safeParse(result.stack); const report: MigrationReport = { result, @@ -257,3 +280,216 @@ describe('no notice, edit or refusal is dropped, merged or reworded (SET)', () = expect(lines.filter((l) => l.trim() !== '').sort()).toEqual(accounted.sort()); }); }); + +/** + * Two flows, each with an edge-branched decision carrying two conditioned + * out-edges and no `mode` — the shape `flow-decision-mode-inclusive-explicit` + * rewrites, twice — and a dashboard `refreshInterval`, whose conversion no + * semantic entry names. + */ +const decisionFlow = (name: string) => ({ + name, + label: name, + type: 'autolaunched', + status: 'active', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'verdict', type: 'decision', label: 'Verdict?' }, + { id: 'refuse', type: 'end', label: 'Refuse' }, + { id: 'convert', type: 'end', label: 'Convert' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'verdict' }, + { id: 'e2', source: 'verdict', target: 'refuse', condition: "lead.status != 'confirmed'" }, + { id: 'e3', source: 'verdict', target: 'convert', condition: "lead.status == 'confirmed'" }, + ], +}); +const DECISION_STACK = { + manifest: { id: 'com.example.report-pairing', name: 'Report Pairing', version: '1.0.0', type: 'app' }, + objects: [{ name: 'rp_lead', label: 'Lead', fields: { status: { type: 'text', label: 'Status' } } }], + flows: [decisionFlow('rp_lead_verdict'), decisionFlow('rp_lead_recheck')], + dashboards: [{ name: 'rp_kpi', label: 'KPI', widgets: [], refreshInterval: 300 }], +}; + +/** One `time` default with a `Z` — a single edit by `time-default-utc-suffix-dropped`. */ +const TIME_STACK = { + manifest: { id: 'com.example.report-pairing-time', name: 'Report Pairing Time', version: '1.0.0', type: 'app' }, + objects: [{ + name: 'rp_shift', + label: 'Shift', + fields: { starts_at: { type: 'time', label: 'Starts at', defaultValue: '09:00Z' } }, + }], +}; + +const DECISION_CONVERSION = 'flow-decision-mode-inclusive-explicit'; +const DECISION_JUDGE = 'flow-decision-edge-branching-first-match'; +const TIME_CONVERSION = 'time-default-utc-suffix-dropped'; +const TIME_JUDGE = 'time-default-zone-refused'; + +const REVIEW_RE = /^ {6}↳ review /; + +/** The review line a judge prints under `edits` edits — written from the entry's data, not from the printer. */ +function reviewLine(judge: MigrationTodo, edits: number): string { + const subject = edits === 1 ? 'the edit above' : `the ${edits} edits above`; + return ` ↳ review ${subject} against the manual change [protocol ${judge.toMajor}] ${judge.surface} → ${judge.replacement}`; +} + +/** Every link the registry declares: a semantic entry, and a conversion id it names. */ +function declaredLinks(): Array<{ toMajor: number; entry: string; conversionId: string }> { + return MIGRATION_MAJORS.flatMap((m) => + MIGRATIONS_BY_MAJOR[m]!.semantic.flatMap((s) => + (s.conversionIds ?? []).map((conversionId) => ({ toMajor: m, entry: s.id, conversionId })), + ), + ); +} + +function todoOf(result: MigrationChainResult, id: string): MigrationTodo { + const todo = result.todos.find((t) => t.id === id); + expect(todo, `the chain reports the semantic entry ${id}`).toBeDefined(); + return todo!; +} + +/** The index of the last line of the first run of edits by `conversionId`, and the run's length. */ +function runOf(lines: string[], conversionId: string): { last: number; length: number } { + const isEdit = (l: string | undefined) => l !== undefined && l.startsWith(' • ') && l.endsWith(` (${conversionId})`); + const first = lines.findIndex((l) => isEdit(l)); + expect(first, `an applied edit by ${conversionId} is printed`).toBeGreaterThan(-1); + let last = first; + while (isEdit(lines[last + 1])) last += 1; + return { last, length: last - first + 1 }; +} + +/** The review lines printed directly under line `i`. */ +function reviewsUnder(lines: string[], i: number): string[] { + const out: string[] = []; + for (let j = i + 1; j < lines.length && REVIEW_RE.test(lines[j]!); j += 1) out.push(lines[j]!); + return out; +} + +describe('an applied edit a semantic entry judges prints that entry beside it, marked review (PAIR)', () => { + it('prints the judge once under the run of edits it judges, counting them', () => { + const { result, lines } = run(DECISION_STACK, MIGRATION_SUPPORT_FLOOR, TERMINUS); + const edits = result.applied.filter((a) => a.conversionId === DECISION_CONVERSION); + expect(edits, 'anti-vacuity: both decisions were rewritten').toHaveLength(2); + const { last, length } = runOf(lines, DECISION_CONVERSION); + expect(length).toBe(2); + expect(reviewsUnder(lines, last)).toEqual([reviewLine(todoOf(result, DECISION_JUDGE), 2)]); + // Beside the edit, i.e. inside ②: after its header, before ③'s. + expect(last).toBeGreaterThan(indexOf(lines, APPLIED_HEADER_RE)); + expect(last + 1).toBeLessThan(indexOf(lines, SEMANTIC_HEADER_RE)); + }); + + it('says "the edit above" under a run of one', () => { + const { result, lines } = run(TIME_STACK, MIGRATION_SUPPORT_FLOOR, TERMINUS); + const { last, length } = runOf(lines, TIME_CONVERSION); + expect(length).toBe(1); + expect(reviewsUnder(lines, last)).toEqual([reviewLine(todoOf(result, TIME_JUDGE), 1)]); + }); + + it('prints an edit no entry judges exactly as before, with no review line under it', () => { + const { result, lines } = run(DECISION_STACK, MIGRATION_SUPPORT_FLOOR, TERMINUS); + const linked = new Set(result.todos.flatMap((t) => t.conversionIds ?? [])); + const unjudged = result.applied.filter((a) => !linked.has(a.conversionId)); + expect(unjudged.length, 'anti-vacuity: the dashboard edit has no judge').toBeGreaterThan(0); + for (const a of unjudged) { + const i = lines.indexOf(` • ${a.path}: ${a.from} → ${a.to} (${a.conversionId})`); + expect(i, `unjudged edit printed: ${a.path}`).toBeGreaterThan(-1); + expect(reviewsUnder(lines, i), `no review line under ${a.path}`).toEqual([]); + } + // ②'s own lines are the chain's, byte-identical and in order, review lines aside. + expect(lines.filter((l) => l.startsWith(' • '))).toEqual(appliedLines(result)); + expect(lines.filter((l) => APPLIED_HEADER_RE.test(l))).toEqual([ + ` Applied ${result.applied.length} mechanical change(s):`, + ]); + }); + + it('prints no review line at all where no applied conversion is linked', () => { + const { result, lines } = run(FINDINGS_STACK, MIGRATION_SUPPORT_FLOOR, TERMINUS); + const linked = new Set(result.todos.flatMap((t) => t.conversionIds ?? [])); + expect(linked.size, 'anti-vacuity: the chain does carry links').toBeGreaterThan(0); + expect(result.applied.some((a) => linked.has(a.conversionId))).toBe(false); + expect(lines.filter((l) => REVIEW_RE.test(l))).toEqual([]); + }); + + it('keeps every semantic entry in ③ — the judge included — with the chain\'s count and bytes', () => { + const { result, lines } = run(DECISION_STACK, MIGRATION_SUPPORT_FLOOR, TERMINUS); + const header = indexOf(lines, SEMANTIC_HEADER_RE); + expect(lines[header]).toBe(` ${result.todos.length} manual change(s) require your judgment:`); + const expected = noticeLines(result); + expect(lines.slice(header + 1, header + 1 + expected.length)).toEqual(expected); + const entries = lines.slice(header + 1).filter((l) => /^ {4}⚠ \[protocol \d+\] /.test(l)); + expect(entries).toHaveLength(result.todos.length); + const judge = todoOf(result, DECISION_JUDGE); + expect(entries).toContain(` ⚠ [protocol ${judge.toMajor}] ${judge.surface} → ${judge.replacement}`); + }); + + it('prints nothing but the groups and the review lines the declared links call for', () => { + const { report, result, lines } = run(DECISION_STACK, MIGRATION_SUPPORT_FLOOR, TERMINUS); + // Written without the printer's run logic: a real chain replays one + // conversion at a time, so a conversion's edits are one run and its count + // is the run's length. + const runs = new Map(); + for (const [i, a] of result.applied.entries()) { + if (i > 0 && result.applied[i - 1]!.conversionId !== a.conversionId) { + expect(runs.has(a.conversionId), `${a.conversionId} edits are contiguous`).toBe(false); + } + runs.set(a.conversionId, (runs.get(a.conversionId) ?? 0) + 1); + } + const reviews = result.todos.flatMap((t) => + (t.conversionIds ?? []).filter((id) => runs.has(id)).map((id) => reviewLine(t, runs.get(id)!)), + ); + expect(reviews.length, 'anti-vacuity: the stack exercises a link').toBeGreaterThan(0); + const accounted = [ + ...lines.filter((l) => VERDICT_RE.test(l) || APPLIED_HEADER_RE.test(l) || SEMANTIC_HEADER_RE.test(l)), + ...refusalLines(report), + ...appliedLines(result), + ...reviews, + ...noticeLines(result), + ].filter((l) => l.trim() !== ''); + expect(lines.filter((l) => l.trim() !== '').sort()).toEqual(accounted.sort()); + }); + + it('pairs every link the registry declares, replayed over the linked conversion\'s own fixture', () => { + const links = declaredLinks(); + // Anti-vacuity, and the two links authored when the printer learned to pair. + expect(links).toEqual(expect.arrayContaining([ + expect.objectContaining({ entry: DECISION_JUDGE, conversionId: DECISION_CONVERSION }), + expect.objectContaining({ entry: TIME_JUDGE, conversionId: TIME_CONVERSION }), + ])); + for (const link of links) { + const conversion = ALL_CONVERSIONS.find((c) => c.id === link.conversionId); + expect(conversion, `${link.entry} names a registered conversion`).toBeDefined(); + printed = []; + const { result, lines } = run( + conversion!.fixture.before as Record, + MIGRATION_SUPPORT_FLOOR, + TERMINUS, + ); + const { last, length } = runOf(lines, link.conversionId); + expect( + reviewsUnder(lines, last), + `${link.entry} is printed beside the ${link.conversionId} edits`, + ).toContain(reviewLine(todoOf(result, link.entry), length)); + } + }); + + it('pairs across hops: a link to a conversion an earlier step replays', () => { + // No authored link crosses a hop yet, though `SemanticMigration.conversionIds` + // allows one, so this pin adds it: a protocol-18 entry made to judge a + // protocol-17 conversion. Everything else is the chain's own result. + const earlier = 'action-execute-to-target'; + const conversion = ALL_CONVERSIONS.find((c) => c.id === earlier)!; + const judgeId = DECISION_JUDGE; + const { result, lines } = run( + conversion.fixture.before as Record, + MIGRATION_SUPPORT_FLOOR, + TERMINUS, + (r) => ({ ...r, todos: r.todos.map((t) => (t.id === judgeId ? { ...t, conversionIds: [earlier] } : t)) }), + ); + const judge = todoOf(result, judgeId); + const edit = result.applied.find((a) => a.conversionId === earlier); + expect(edit?.toMajor, 'anti-vacuity: the edit comes from an earlier hop').toBeLessThan(judge.toMajor); + const { last, length } = runOf(lines, earlier); + expect(reviewsUnder(lines, last)).toEqual([reviewLine(judge, length)]); + }); +}); diff --git a/packages/cli/src/commands/migrate/meta.ts b/packages/cli/src/commands/migrate/meta.ts index d48fb06eef6..868eef7e898 100644 --- a/packages/cli/src/commands/migrate/meta.ts +++ b/packages/cli/src/commands/migrate/meta.ts @@ -13,6 +13,7 @@ import { MIGRATION_MAJORS, MIGRATION_SUPPORT_FLOOR, type MigrationChainResult, + type MigrationTodo, } from '@objectstack/spec/migrations'; import { PROTOCOL_MAJOR, PROTOCOL_VERSION } from '@objectstack/spec/kernel'; import { FILE_REFERENCE_TYPES, REFERENCE_VALUE_TYPES, STRUCTURED_JSON_TYPES } from '@objectstack/spec/data'; @@ -295,13 +296,79 @@ function printSchemaVerdict(report: MigrationReport): void { console.log(''); } +/** + * The semantic entries that judge each conversion, keyed by conversion id — + * read off the entries' declared `conversionIds` (`SemanticMigration`), in the + * order the chain reports the entries. + * + * ⛔ Declared links only. An entry's prose naming a conversion id is not a + * link: prose also names incidental analogues, so the spec lane authors a link + * only after reading both sides, and this printer pairs nothing it was not + * told to. + * + * The join runs over the whole chain result, not hop by hop: a link may name a + * conversion an EARLIER step replays than the entry's own, and both halves sit + * in the flat `applied` / `todos` arrays whichever hop produced them. + */ +function judgesByConversion(todos: readonly MigrationTodo[]): ReadonlyMap { + const judges = new Map(); + for (const todo of todos) { + for (const conversionId of todo.conversionIds ?? []) { + const list = judges.get(conversionId); + if (!list) judges.set(conversionId, [todo]); + else if (!list.includes(todo)) list.push(todo); + } + } + return judges; +} + +/** + * Group ② — every applied edit, and beside the edits a semantic entry judges, + * that entry's headline marked **review**. + * + * The edit lines are the chain's, one per edit, unchanged. A review line is + * printed once under each RUN of consecutive edits by one conversion — the + * chain replays one conversion at a time, so a conversion's edits arrive + * together — and counts the edits above it that it judges. Once per run rather + * than once per edit: a real upgrade wrote 13 edits by one judged conversion, + * and repeating a paragraph-long judgment 13 times is the noise this report is + * ordered to cut. Nothing is lost by it — every edit line still names its + * conversion, and the review line says how many of the lines above it covers. + * + * The review line is a COPY of the entry's `[protocol N] surface → replacement` + * headline, not the entry: ③ still prints that entry in full, in its place, so + * its `why` and `verify` are found there by the same headline, and ③'s count + * and lines do not change (ADR-0087 D3, "never silence"). + */ +function printAppliedEdits(result: MigrationChainResult): void { + const judges = judgesByConversion(result.todos); + console.log(chalk.bold(` Applied ${result.applied.length} mechanical change(s):`)); + let run = 0; + for (const [i, a] of result.applied.entries()) { + console.log(` • ${a.path}: ${chalk.red(a.from)} → ${chalk.green(a.to)} ${chalk.dim(`(${a.conversionId})`)}`); + run += 1; + if (result.applied[i + 1]?.conversionId === a.conversionId) continue; // the run goes on + const subject = run === 1 ? 'the edit above' : `the ${run} edits above`; + for (const judge of judges.get(a.conversionId) ?? []) { + console.log( + ` ${chalk.yellow('↳ review')} ${subject} against the manual change ` + + `[protocol ${judge.toMajor}] ${judge.surface} → ${judge.replacement}`, + ); + } + run = 0; + } + console.log(''); +} + /** * The human report of an authored-source run, in the order an upgrader acts on * it, each group under one header line that counts it (ADR-0087 D3): * * ① the VERDICT and the REFUSALS — whether the migrated stack parses and, when * it does not, every refusal left after the chain: what still blocks it; - * ② the APPLIED mechanical edits — the diff the chain has already made; + * ② the APPLIED mechanical edits — the diff the chain has already made, with + * the semantic entry that judges an edit printed beside it, marked review + * (see {@link printAppliedEdits}); * ③ the SEMANTIC notices — every semantic entry of every hop crossed. * * ## Why this order @@ -320,7 +387,8 @@ function printSchemaVerdict(report: MigrationReport): void { * silence": an entry may leave ③ only on a structured, stack-derived proof that * its surface is absent, and matching the prose of `surface` against the stack * is not one. So the groups MOVE and nothing else does: every line ② and ③ - * printed before is printed after, byte-identical and in chain order. + * printed before is printed after, byte-identical and in chain order. The one + * addition is ②'s review lines, each a copy of an entry ③ still prints. * `--json` is untouched — its keys, its values and the order of its arrays. */ export function printMigrationReport(report: MigrationReport): void { @@ -352,14 +420,8 @@ export function printMigrationReport(report: MigrationReport): void { return; } - // ② The mechanical rewrites (auto-applied). - if (result.applied.length > 0) { - console.log(chalk.bold(` Applied ${result.applied.length} mechanical change(s):`)); - for (const a of result.applied) { - console.log(` • ${a.path}: ${chalk.red(a.from)} → ${chalk.green(a.to)} ${chalk.dim(`(${a.conversionId})`)}`); - } - console.log(''); - } + // ② The mechanical rewrites (auto-applied), a judged edit's judge beside it. + if (result.applied.length > 0) printAppliedEdits(result); // Per-hop checkpoints. if (report.step) {