Skip to content

Commit 476c373

Browse files
claude[bot]claude
andauthored
tooling(pm): give dispatch-gates --ran an explained bucket for the value-bearing class (#15165)
`runReconciliation` classified a recorded command that is not in the derived runnable set into three outcomes, two of which are "explained, not a mistake": a CI-measured-only family and a pending-changeset family. The value-bearing class added for #15083 — an invocation whose argv takes a value from the workflow, which `commandsFor` subtracts from the union for the same reason it subtracts the CI-measured one — had no such bucket, so a dev who recorded one landed in `extra` under a caption that reads "named by nothing this run derived": the one sentence that is false about it, because this run derived it and then classified it out. Third bucket, built from `notRunnableCommandSet` off the same rows the CI-measured set is built from, checked beside it (the two subtractions `commandsFor` makes, in that order) and before the `extra` fallback, and rendered on its own labelled line naming the reason. Diagnostic only: `recon.ok` still reads `unrun.length`, `--commands` is untouched, and no derivation key moves. Measured at the merge base 4283b72: a record of the 15 derived commands for a changeset path plus the live `node scripts/check-empty-changeset.mjs --base "$MERGE_BASE"` invocation reported Outside this card's derivation (1) — recorded, and named by nothing this run derived. - node scripts/check-empty-changeset.mjs --base "$MERGE_BASE" and the same record on this tree reports the labelled bucket instead, with the remainder heading gone and the verdict unmoved (15 derived, 15 run, exit 0). Self-test: seven cases, three of them a real CLI run whose fixture is the tool's own `--json` answer on this tree — a parameter defaulting to empty is exactly the shape that keeps unit cases green while the live mode still dumps the class in the remainder. Claude-Session: https://claude.ai/code/session_019RfFHiRCSs3JXLK4cwcfox Co-authored-by: Claude <noreply@anthropic.com>
1 parent 3bd90d4 commit 476c373

1 file changed

Lines changed: 149 additions & 7 deletions

File tree

scripts/pm/dispatch-gates.mjs

Lines changed: 149 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9730,9 +9730,20 @@ export function parseRunRecord(text) {
97309730
* heading) and which deduplicates the two spellings of one family into one
97319731
* command before this function ever sees them. What the tool CAN still meet in
97329732
* a record it classifies itself — a CI-measured-only family, which `commandsFor`
9733-
* subtracts by design, and a pending-changeset family, derived against a path
9734-
* that did not exist at derivation time. Both are matched byte-exactly against
9735-
* sets this same derivation produced.
9733+
* subtracts by design; a VALUE-BEARING family, which `commandsFor` subtracts for
9734+
* the same reason one step later because its argv takes a value from the
9735+
* workflow (#15083); and a pending-changeset family, derived against a path
9736+
* that did not exist at derivation time. All three are matched byte-exactly
9737+
* against sets this same derivation produced.
9738+
*
9739+
* ⭐ The third bucket is not a convenience: this file's own rule is that an
9740+
* omission is disclosed WHERE the omission happens, and a class `commandsFor`
9741+
* deliberately withholds is a class the runner cannot be expected to have
9742+
* derived. Left in the remainder it reads as a command "named by nothing this
9743+
* run derived" — the one sentence that is false about it, because this run
9744+
* derived it and then classified it out. The likeliest recorder is a dev who
9745+
* writes the bare spelling of one of these scripts out of habit, and what they
9746+
* are owed is the reason, not a shrug (#15115).
97369747
*
97379748
* What is left for the runner to explain is therefore only what the tool
97389749
* genuinely cannot know: a gate that refused with its own prerequisite. That is
@@ -9751,6 +9762,12 @@ export function parseRunRecord(text) {
97519762
export function runReconciliation({
97529763
derived = [],
97539764
ciOnlyCommands = new Set(),
9765+
// Beside `ciOnlyCommands` and not after `pendingCommands`, because the file
9766+
// already groups them: these are the TWO subtractions `commandsFor` makes
9767+
// from the runnable union, in that order, and the pending families are a
9768+
// different fact (a path that did not exist at derivation time). Defaulting
9769+
// to empty keeps every existing caller's verdict byte-identical.
9770+
notRunnableCommands = new Set(),
97549771
pendingCommands = new Set(),
97559772
record = [],
97569773
} = {}) {
@@ -9792,6 +9809,7 @@ export function runReconciliation({
97929809
}
97939810

97949811
const explainedCiOnly = [];
9812+
const explainedNotRunnable = [];
97959813
const explainedPending = [];
97969814
const extra = [];
97979815
const nearMiss = [];
@@ -9804,6 +9822,14 @@ export function runReconciliation({
98049822
explainedCiOnly.push(entry.command);
98059823
continue;
98069824
}
9825+
// The order is the precedence, and it is the one `commandsFor` already
9826+
// states: `ciOnly` is the FIRST subtraction, so a family that were somehow
9827+
// both is reported as CI-measured — one bucket per command, chosen the same
9828+
// way in both places rather than two counts for one omission.
9829+
if (notRunnableCommands.has(entry.command)) {
9830+
explainedNotRunnable.push(entry.command);
9831+
continue;
9832+
}
98079833
if (pendingCommands.has(entry.command)) {
98089834
explainedPending.push(entry.command);
98099835
continue;
@@ -9823,6 +9849,7 @@ export function runReconciliation({
98239849
unrun,
98249850
notMeasured,
98259851
explainedCiOnly: explainedCiOnly.sort(),
9852+
explainedNotRunnable: explainedNotRunnable.sort(),
98269853
explainedPending: explainedPending.sort(),
98279854
extra: extra.sort(),
98289855
nearMiss,
@@ -9885,6 +9912,13 @@ export function runReconciliationLines(recon) {
98859912
);
98869913
for (const command of recon.explainedCiOnly) lines.push(` - ${command}`);
98879914
}
9915+
if (recon.explainedNotRunnable.length > 0) {
9916+
lines.push(
9917+
` Classified by this tool, no explanation owed (${recon.explainedNotRunnable.length}) — VALUE-BEARING famil(ies):`
9918+
+ ' its argv takes a value from the workflow, so it is recorded, not derived as runnable:',
9919+
);
9920+
for (const command of recon.explainedNotRunnable) lines.push(` - ${command}`);
9921+
}
98889922
if (recon.explainedPending.length > 0) {
98899923
lines.push(
98909924
` Classified by this tool, no explanation owed (${recon.explainedPending.length}) — pending-changeset famil(ies), derived against a path that did not exist at derivation time:`,
@@ -10159,9 +10193,10 @@ function derive(paths, { showResidue = false, mode = 'human', runRecord = [] } =
1015910193
const pending = pendingChangesetFamilies([...byCheck], new Set(matched.keys()));
1016010194

1016110195
if (mode === 'ran') {
10162-
// Built from the SAME three expressions the other renderings read, in this
10163-
// process, on this tree: the union `--commands` prints, the CI-measured set
10164-
// that union subtracts, and the pending families it holds back. A second
10196+
// Built from the SAME four expressions the other renderings read, in this
10197+
// process, on this tree: the union `--commands` prints, the two sets that
10198+
// union subtracts — CI-measured, and value-bearing — and the pending
10199+
// families it holds back. A second
1016510200
// traversal here would be a second answer to a question this file already
1016610201
// answers once — and it would be the answer the reconciliation is judged
1016710202
// against, which is the worst possible place to keep a duplicate.
@@ -10172,6 +10207,11 @@ function derive(paths, { showResidue = false, mode = 'human', runRecord = [] } =
1017210207
// makes it a contract rather than a note (#14189).
1017310208
derived: commandsFor({ matchedRows, kindGroups, alwaysRunsRows }),
1017410209
ciOnlyCommands: ciOnlyCommandSet(matchedRows, alwaysRunsRows),
10210+
// The SECOND set `commandsFor` subtracts, read from the SAME rows by the
10211+
// SAME expression it uses — so the union and the reconciliation cannot
10212+
// drift about which invocations are withheld, exactly as they cannot for
10213+
// the CI-measured set above it (#15115).
10214+
notRunnableCommands: notRunnableCommandSet(matchedRows, alwaysRunsRows),
1017510215
pendingCommands: new Set(pending.map(({ entry }) => runnableInvocation(entry))),
1017610216
record: runRecord,
1017710217
});
@@ -18770,16 +18810,51 @@ function selfTest() {
1877018810
t('the malformed line is reported against its line number too', unexplained.malformed.length === 1 && unexplained.malformed[0].line === 1);
1877118811

1877218812
// ── What the TOOL classifies, so no prose has to ────────────────────────
18813+
// ⭐ The value-bearing spelling is the LIVE one — `pr-automation.yml`
18814+
// really passes `--base "$MERGE_BASE"` to this script — for the reason the
18815+
// neighbouring cases take theirs from the live workflows: a fixture
18816+
// invented here would keep passing after the renderer that produces the
18817+
// real one changed shape, which is the failure this whole file is about.
18818+
const valueBearing = 'node scripts/check-empty-changeset.mjs --base "$MERGE_BASE"';
1877318819
const explained = runReconciliation({
1877418820
derived: ['pnpm check:a'],
1877518821
ciOnlyCommands: new Set(['node scripts/check-payload-guard.mjs']),
18822+
notRunnableCommands: new Set([valueBearing]),
1877618823
pendingCommands: new Set(['pnpm check:changeset-shape']),
18777-
record: parseRunRecord(['pnpm check:a', 'node scripts/check-payload-guard.mjs', 'pnpm check:changeset-shape'].join('\n')),
18824+
record: parseRunRecord(['pnpm check:a', 'node scripts/check-payload-guard.mjs', valueBearing, 'pnpm check:changeset-shape'].join('\n')),
1877818825
});
1877918826
t(
1878018827
'a CI-measured-only entry and a pending-changeset entry are classified by the tool, not dumped into the remainder',
1878118828
explained.ok && explained.explainedCiOnly.length === 1 && explained.explainedPending.length === 1 && explained.extra.length === 0,
1878218829
);
18830+
// ⭐ #15115: the THIRD class `commandsFor` withholds gets the same
18831+
// courtesy. Recorded, derived by this run, classified out of the union —
18832+
// so `extra`'s caption ("named by nothing this run derived") would be the
18833+
// one sentence that is false about it.
18834+
t(
18835+
'a recorded VALUE-BEARING family is classified by the tool too, never folded into the remainder',
18836+
explained.explainedNotRunnable.length === 1 && explained.explainedNotRunnable[0] === valueBearing && !explained.extra.includes(valueBearing),
18837+
);
18838+
// CONTROL, and it is the load-bearing half: the bucket explains the class
18839+
// it was given and nothing else. A command named by no set is still
18840+
// `extra` — a third bucket that swallowed unknowns would have deleted the
18841+
// remainder rather than shrunk it.
18842+
const unknownBeside = runReconciliation({
18843+
derived: ['pnpm check:a'],
18844+
notRunnableCommands: new Set([valueBearing]),
18845+
record: parseRunRecord(['pnpm check:a', valueBearing, 'pnpm check:not-a-family'].join('\n')),
18846+
});
18847+
t(
18848+
'and an unknown command beside it still reads `extra` — the new bucket explains its class only',
18849+
unknownBeside.explainedNotRunnable.length === 1 && unknownBeside.extra.length === 1 && unknownBeside.extra[0] === 'pnpm check:not-a-family',
18850+
);
18851+
// The bucket is DIAGNOSTIC, exactly like the two beside it: the verdict
18852+
// reads `unrun` and nothing else, so classifying an entry can never move
18853+
// it in either direction (#15115).
18854+
t(
18855+
'the new bucket cannot move the verdict — it is diagnostic, like the two beside it',
18856+
unknownBeside.ok && unknownBeside.unrun.length === 0 && unknownBeside.ran.length === 1,
18857+
);
1878318858

1878418859
// ── Bookkeeping the classes cannot lose ─────────────────────────────────
1878518860
const dupes = runReconciliation({ derived: ['pnpm check:a'], record: parseRunRecord(['pnpm check:a', 'pnpm check:a'].join('\n')) });
@@ -18823,6 +18898,21 @@ function selfTest() {
1882318898
'the near-miss line refuses the pairing out loud rather than quietly',
1882418899
runReconciliationLines(nearMiss).join('\n').includes('is NOT paired with it'),
1882518900
);
18901+
// ⭐ #15115, in the rendering: a bucket that classified an entry and then
18902+
// printed nothing would leave the runner exactly where `extra` left them.
18903+
const explainedText = runReconciliationLines(explained).join('\n');
18904+
t(
18905+
'the value-bearing bucket gets its OWN labelled line, naming the reason and the command',
18906+
explainedText.includes('VALUE-BEARING famil(ies)')
18907+
&& explainedText.includes('its argv takes a value from the workflow')
18908+
&& explainedText.includes(valueBearing),
18909+
);
18910+
t(
18911+
'and the two buckets beside it keep their own lines, with the remainder heading absent entirely',
18912+
explainedText.includes('CI-MEASURED ONLY')
18913+
&& explainedText.includes('pending-changeset famil(ies)')
18914+
&& !explainedText.includes("Outside this card's derivation"),
18915+
);
1882618916

1882718917
// ── argv: a two-token flag's value must not become a path ───────────────
1882818918
const ranSplit = splitArgv([RAN_FLAG, 'ran.list', 'packages/spec/src/index.ts', '--residue']);
@@ -18891,6 +18981,58 @@ function selfTest() {
1889118981
}
1889218982
}
1889318983

18984+
// ── END TO END: the VALUE-BEARING bucket is WIRED, not merely present (#15115) ──
18985+
//
18986+
// Every unit case above stays green if the `--ran` call site never PASSES
18987+
// the value-bearing set — a parameter that defaults to empty is exactly the
18988+
// shape that keeps its own tests green while the live mode still dumps the
18989+
// class in the remainder, which is the state this card was filed about.
18990+
// Only a real run reads the wiring.
18991+
//
18992+
// The fixture is this tool's OWN answer on this tree, read from `--json`,
18993+
// rather than an invocation typed here: a hardcoded spelling would keep
18994+
// passing after the renderer that produces the real one changed shape, and
18995+
// the two sides of the comparison would stop being the same strings — the
18996+
// property the whole `--ran` design rests on.
18997+
{
18998+
const vbTmp = mkdtempSync(nodePath.join(tmpdir(), 'dg-ran-vb-'));
18999+
try {
19000+
// A changeset path, because that is what reaches the live value-bearing
19001+
// families — and it is the tool's own constant rather than a spelling
19002+
// this test invented.
19003+
const vbCard = CHANGESET_PROBE_PATH;
19004+
const jsonRun = runCli(['--json', vbCard]);
19005+
const doc = jsonRun.status === 0 ? JSON.parse(jsonRun.stdout ?? '{}') : null;
19006+
const vbRows = [...(doc?.matched ?? []), ...(doc?.alwaysRunsPopulation ?? [])].filter((row) => row.notRunnable);
19007+
t('CONTROL: this tree still derives at least one VALUE-BEARING family for a changeset path', Boolean(doc) && vbRows.length >= 1);
19008+
if (doc && vbRows.length >= 1) {
19009+
const vbCommand = vbRows[0].command;
19010+
t('CONTROL: and the runnable union WITHHOLDS it — which is the whole reason the bucket exists', !doc.commands.includes(vbCommand));
19011+
const vbRecord = nodePath.join(vbTmp, 'ran-value-bearing.list');
19012+
writeFileSync(vbRecord, `${[...doc.commands, vbCommand].join('\n')}\n`);
19013+
const vbRun = runCli([RAN_FLAG, vbRecord, vbCard]);
19014+
const vbOut = vbRun.stdout ?? '';
19015+
t(
19016+
'⭐ a real run that RECORDS it lands it in the VALUE-BEARING bucket, with the remainder heading gone entirely',
19017+
vbRun.status === 0
19018+
&& vbOut.includes('VALUE-BEARING famil(ies)')
19019+
&& vbOut.includes(vbCommand)
19020+
&& !vbOut.includes("Outside this card's derivation"),
19021+
);
19022+
t(
19023+
'and the reason travels with it, so the runner learns why this invocation is not one they could have derived',
19024+
vbOut.includes('its argv takes a value from the workflow'),
19025+
);
19026+
t(
19027+
'while the verdict is unmoved — the bucket is diagnostic, and every derived family is still accounted for',
19028+
vbOut.includes(`${doc.commands.length} derived famil(ies) accounted for`),
19029+
);
19030+
}
19031+
} finally {
19032+
rmSync(vbTmp, { recursive: true, force: true });
19033+
}
19034+
}
19035+
1889419036
// The per-case line already printed inside `t()`, streamed as each verdict
1889519037
// was decided (#14281) — this tail is the summary only, unchanged in shape
1889619038
// and wording from the pre-streaming version.

0 commit comments

Comments
 (0)