feat(#3674): extract shared file-overlap wave partitioner (#4166)

* test(#3674): characterize existing wave-dispatch output and add tests for the extracted partitioner

Pins resolveWaveDispatch's and emitWorkflowScript's current, unextracted
output (chain-overlap, disjoint-empty-set, and a multi-wave/multi-stage
golden script) as a regression safety net ahead of extracting
partitionStages into a standalone module. Also adds the new module's
unit and property tests (test matrix rows 1-11) against its expected
public API, which does not exist yet and is added in the next commit.

* feat(#3674): extract file-overlap partitioner into a shared, generic module

Moves partitionStages' greedy first-fit file-overlap algorithm into a new,
dependency-free src/file-overlap-partitioner.cts module (partitionByFileOverlap),
generalized over a plain {id, files}[] shape rather than claude-orchestration.cts's
Plan/Wave interfaces. partitionStages becomes a thin adapter mapping its own
Plan[] shape onto the generic input and back — behavior-preserving, no dependency
ordering, no path normalization, no filesystem access moved or added. Enables a
future consumer (quick-batch, #3675 / ADR-1239) to reuse the same primitive
without pulling in orchestration internals.

* docs(#3674): register the file-overlap-partitioner module bookkeeping

New src/*.cts -> bin/lib/*.cjs modules need four hand-maintained
registrations beyond the code itself: .gitignore (compiled artifact),
eslint.config.mjs (ADR-457: lint the .cts, not the emitted .cjs),
docs/INVENTORY.md's CLI Modules roster row (regenerated via
gen-inventory-manifest.cjs --write), and a CONTEXT.md glossary entry
matching the convention set by similarly-scoped leaf modules
(text-lines.cts, plan-dependency-graph.cts, spec-section.cts).

* fix(#3674): alphabetize INVENTORY.md row, manifest regen no-op, fast-check import already correct

- docs/INVENTORY.md: move file-overlap-partitioner.cjs row to alphabetical position
- docs/INVENTORY-MANIFEST.json: regenerated via gen-inventory-manifest.cjs --write,
  produced no diff (manifest is keyed by content, not row order)
- tests/claude-orchestration.test.cjs's direct require('fast-check') is correct as-is:
  tests/helpers/fast-check-setup.cjs's own docstring scopes the shared-seed wrapper to
  "every *.property.test.cjs file"; claude-orchestration.test.cjs is not a .property.test.cjs
  file, and every .property.test.cjs file sampled uses the wrapper consistently. No outlier.

* fix(#3674): constrain the no-overlap property test to unique ids, fixing an ambiguous duplicate-id reconstruction

The `no two plans in the same stage share a modified file` property
reconstructs which physical item produced each output id via
`remaining.findIndex(r => r.id === id)`. Under duplicate ids (an
explicitly-supported input shape for `partitionByFileOverlap`) that
reconstruction can pick the wrong physical occurrence, producing a
false-positive overlap failure (observed counterexample: p0(f1),
p208(f1), p208([]) — correctly staged as [[p0,p208#2],[p208#1]], but
misread by id-order as [[p0,p208#1],...], which do overlap).
Properties (a) determinism and (b) totality already exercise
duplicate ids correctly and are left unchanged; only this property's
generated items are now constrained to unique ids via
`fc.uniqueArray`, where the reconstruction is unambiguous.

---------

Co-authored-by: sim <sim@local>
This commit is contained in:
Tom Boucher
2026-09-02 07:33:42 -04:00
committed by GitHub
parent fa107c0461
commit b0572c0108
10 changed files with 473 additions and 21 deletions

View File

@@ -601,6 +601,166 @@ describe('emitWorkflowScript — per-plan use_worktree (#2772 / #2285 finding 1)
});
});
// ─── 3.6. #3674 — pre-extraction characterization (golden output pins) ────────
//
// `partitionStages` is being extracted into a standalone, generic module
// (`file-overlap-partitioner.cts`). The extraction's acceptance bar is
// byte-identical output from `resolveWaveDispatch` and `emitWorkflowScript`
// (#3674's own callers) — NOT improved output. These tests pin the ACTUAL
// current production output (captured from the unmodified, pre-extraction
// code) for representative multi-plan fixtures, so the extraction cannot
// silently change what either function returns. They must pass BOTH before
// AND after the extraction lands.
describe('#3674 — pre-extraction characterization (golden output pins)', () => {
test('characterization: chain-overlap plans (A∩B, B∩C, A∌C) produce the greedy, non-optimal stage assignment', () => {
// A and B share f2; B and C share f3; A and C share nothing. A minimal
// packing could put A and C together after B — greedy first-fit does NOT
// find that; it processes in input order and is not required to be optimal.
const r = emitWorkflowScript({
phaseDir: '.planning/phases/01-chain',
runId: 'run-chain-3674',
waves: [{
id: 'w1',
plans: [
{ id: 'A', brief: 'Plan A', files_modified: ['f1.ts', 'f2.ts'] },
{ id: 'B', brief: 'Plan B', files_modified: ['f2.ts', 'f3.ts'] },
{ id: 'C', brief: 'Plan C', files_modified: ['f3.ts', 'f4.ts'] },
],
}],
});
assert.strictEqual(r.ok, true);
assert.deepStrictEqual(r.summary.stagesByWave, [[['A', 'C'], ['B']]]);
});
test('characterization: two plans with empty files_modified coalesce into stage 0 without colliding', () => {
const r = emitWorkflowScript({
phaseDir: '.planning/phases/01-empty',
runId: 'run-empty-3674',
waves: [{
id: 'w1',
plans: [
{ id: 'p1', brief: 'Plan with no files', files_modified: [] },
{ id: 'p2', brief: 'Another empty plan', files_modified: [] },
],
}],
});
assert.strictEqual(r.ok, true);
assert.deepStrictEqual(r.summary.stagesByWave, [[['p1', 'p2']]]);
});
test('characterization: resolveWaveDispatch output is captured before extraction and is byte-identical after (multi-wave, multi-stage fixture)', () => {
const CAPABLE_HOST_3674 = { dispatch: { namedDispatch: true, nested: true, background: true, backgroundDispatch: false } };
const input = {
runtimeId: 'claude',
hostIntegration: CAPABLE_HOST_3674,
agentSdkVersion: '1.2.0',
config: { 'claude_orchestration.enabled': true, 'claude_orchestration.execution_backend': 'workflow' },
phaseDir: '.planning/phases/01-resolve3674',
runId: 'run-resolve-3674',
waves: [
{ id: 'w1', plans: [
{ id: 'p1', brief: 'Plan P1', files_modified: ['src/shared.cts', 'src/a.cts'] },
{ id: 'p2', brief: 'Plan P2', files_modified: ['src/shared.cts', 'src/b.cts'] },
{ id: 'p3', brief: 'Plan P3', files_modified: ['src/c.cts'] },
] },
{ id: 'w2', plans: [
{ id: 'p4', brief: 'Plan P4', files_modified: [] },
] },
],
};
const r = resolveWaveDispatch(input);
assert.strictEqual(r.backend, 'workflow');
assert.strictEqual(r.reason, 'workflow_backend_active');
assert.deepStrictEqual(r.summary, {
waves: 2,
plans: 4,
worktreePlans: 4,
stagesByWave: [[['p1', 'p3'], ['p2']], [['p4']]],
resumeRunId: 'run-resolve-3674',
budgetTokens: null,
});
// Golden script captured verbatim from the unmodified pre-extraction
// production code (`node -e` against `gsd-core/bin/lib/claude-orchestration.cjs`
// before any source under this phase was touched). Any divergence here is
// an observable behavior change in the emitted Workflow script.
const GOLDEN_SCRIPT_3674 = [
'export const meta = {',
' name: "gsd-execute-run-resolve-3674",',
' description: "GSD wave dispatch for .planning/phases/01-resolve3674",',
' phases: [',
' { title: "Wave w1", detail: "3 plan(s)" },',
' { title: "Wave w2", detail: "1 plan(s)" },',
' ],',
'}',
'',
'// GSD Workflow script — generated by the claude-orchestration capability (#1143)',
'// phase: .planning/phases/01-resolve3674',
'// BETA: preview-grade; on any failure the orchestrator falls back to inline dispatch.',
'// Composes the SAME gsd-executor agent as the inline path, so artifacts (SUMMARY.md)',
'// and commits are produced identically. Worktree isolation is per-plan (use_worktree)',
'// and mirrors execute-phase.md step 2.5\'s submodule gate exactly (#2772 / #2285).',
'// model: none applied — resolved to "inherit"/empty, so each agent inherits the',
'// orchestrator model (#2517: emitting an empty model 404s on some runtimes).',
'//',
'// resume: pass "run-resolve-3674" as the Workflow tool\'s resumeFromRunId input',
'// (it is a tool parameter, NOT a script function).',
'',
'// #3302: extract the executor-returned <worktree_metadata> JSON so the',
'// orchestrator can record it into WAVE_WORKTREE_MANIFEST after the run',
'// (worktree.record-agent -> worktree.cleanup-wave, the same manifest-scoped',
'// merge chain inline dispatch feeds). null = absent/unparseable/interrupted.',
'function gsdWorktreeMetadata(agentResult) {',
' if (typeof agentResult !== \'string\') return null;',
' const m = agentResult.match(/<worktree_metadata>([\\s\\S]*?)<\\/worktree_metadata>/);',
' if (m === null) return null;',
' try {',
' const parsed = JSON.parse(m[1]);',
' return (parsed !== null && typeof parsed === \'object\') ? parsed : null;',
' } catch (e) {',
' return null;',
' }',
'}',
'const gsdAgentOutcomes = [];',
'',
'// Wave w1',
'phase("Wave w1")',
'// Stage 0',
'const gsdStage_0_0 = await parallel([',
' () => agent("Plan P1", { agentType: "gsd-executor", isolation: "worktree" }),',
' () => agent("Plan P3", { agentType: "gsd-executor", isolation: "worktree" }),',
'])',
'gsdAgentOutcomes.push(',
' { plan: "p1", expects_worktree: true, metadata: gsdWorktreeMetadata(gsdStage_0_0[0]) },',
' { plan: "p3", expects_worktree: true, metadata: gsdWorktreeMetadata(gsdStage_0_0[1]) }',
')',
'// Stage 1 (sequential — files_modified overlap)',
'const gsdStage_0_1 = await parallel([',
' () => agent("Plan P2", { agentType: "gsd-executor", isolation: "worktree" }),',
'])',
'gsdAgentOutcomes.push(',
' { plan: "p2", expects_worktree: true, metadata: gsdWorktreeMetadata(gsdStage_0_1[0]) }',
')',
'',
'// Wave w2',
'phase("Wave w2")',
'const gsdStage_1_0 = await parallel([',
' () => agent("Plan P4", { agentType: "gsd-executor", isolation: "worktree" }),',
'])',
'gsdAgentOutcomes.push(',
' { plan: "p4", expects_worktree: true, metadata: gsdWorktreeMetadata(gsdStage_1_0[0]) }',
')',
'return gsdAgentOutcomes',
].join('\n');
assert.strictEqual(r.script, GOLDEN_SCRIPT_3674);
});
});
// ─── 4. Capability declaration validation ─────────────────────────────────────
describe('capability declaration (capabilities/claude-orchestration/capability.json)', () => {

View File

@@ -0,0 +1,103 @@
'use strict';
/**
* Property-based tests for file-overlap-partitioner.cjs (#3674)
*
* Module: gsd-core/bin/lib/file-overlap-partitioner.cjs
* Exported: partitionByFileOverlap(items: { id: string; files: string[] }[]) -> string[][]
*
* Properties tested (test matrix rows 9-11):
* (a) determinism — two runs on the same input produce an identical partition
* (b) totality — every input item appears in exactly one output stage,
* never lost or duplicated
* (c) the core invariant — no two items in the same stage share a file
*
* Duplicate `id`s are a real, intentionally-supported input shape (see
* `partitionByFileOverlap`'s doc comment) and properties (a) and (b) verify
* it correctly — (a) needs no per-item identity at all, and (b) only compares
* the sorted id multiset, so it never needs to pick out *which* physical item
* a given output id refers to. Property (c) is different: checking "do these
* two co-staged items' file sets overlap" requires reconstructing which
* physical item (files included) produced each id in the output, and under
* duplicate ids that reconstruction is ambiguous — a stage's "p208" could be
* either physical p208 occurrence, and picking the wrong one produces a false
* failure (see #3674 regression: `p0(f1)`, `p208(f1)`, `p208([])` staged
* correctly as `[[p0,p208#2],[p208#1]]`, misread as `[[p0,p208#1],...]` by an
* id-order reconstruction). So (c) alone constrains its generated items to
* unique ids, where the reconstruction is unambiguous by construction.
*/
const { describe, test } = require('node:test');
const assert = require('node:assert/strict');
const fc = require('./helpers/fast-check-setup.cjs');
const { partitionByFileOverlap } = require('../gsd-core/bin/lib/file-overlap-partitioner.cjs');
/** A single overlap item: a small id plus a small set of file tokens (some shared, some not). */
const itemArb = fc.record({
id: fc.integer({ min: 0, max: 999 }).map((n) => 'p' + n),
files: fc.array(fc.constantFrom('f0', 'f1', 'f2', 'f3', 'f4', 'f5', 'f6', 'f7'), { maxLength: 4 }),
});
describe('partitionByFileOverlap: properties', () => {
test('property: repeated runs on the same input are deterministic', () => {
fc.assert(fc.property(
fc.array(itemArb, { minLength: 0, maxLength: 60 }),
(items) => {
const a = partitionByFileOverlap(items);
const b = partitionByFileOverlap(items);
assert.deepStrictEqual(a, b);
},
));
});
test('property: every input plan appears in exactly one output stage', () => {
fc.assert(fc.property(
fc.array(itemArb, { minLength: 0, maxLength: 60 }),
(items) => {
const stages = partitionByFileOverlap(items);
const flat = stages.flatMap((s) => s);
assert.strictEqual(flat.length, items.length, 'no item lost or duplicated across stages');
// Positional identity: nth occurrence in flattened output must match
// input order's ids, one-to-one (id alone is not unique under
// duplicates, so compare the full multiset via sorted copies).
assert.deepStrictEqual(flat.slice().sort(), items.map((i) => i.id).sort());
},
));
});
test('property: no two plans in the same stage share a modified file', () => {
fc.assert(fc.property(
// Unique ids only: this property reconstructs which physical item
// produced each output id, and that reconstruction is ambiguous when
// ids duplicate (see the module doc comment above). Properties (a)
// and (b) still fuzz duplicate ids via the shared `itemArb`.
fc.uniqueArray(itemArb, { minLength: 0, maxLength: 60, selector: (it) => it.id }),
(items) => {
const stages = partitionByFileOverlap(items);
// Positional lookup (not strictly required once ids are unique, but
// kept for symmetry with the reconstruction shape and to tolerate
// any future relaxation of the uniqueness constraint above).
const remaining = items.map((i) => ({ id: i.id, files: new Set(i.files) }));
for (const stageIds of stages) {
const stageItems = stageIds.map((id) => {
const idx = remaining.findIndex((r) => r.id === id);
const found = remaining[idx];
remaining.splice(idx, 1);
return found;
});
for (let i = 0; i < stageItems.length; i++) {
for (let j = i + 1; j < stageItems.length; j++) {
const a = stageItems[i].files;
const b = stageItems[j].files;
let overlap = false;
for (const f of a) if (b.has(f)) { overlap = true; break; }
assert.ok(!overlap, `stage-mates ${stageItems[i].id} and ${stageItems[j].id} must not share a file`);
}
}
}
},
));
});
});

View File

@@ -0,0 +1,113 @@
'use strict';
/**
* file-overlap-partitioner.test.cjs — Behavioral tests for the shared
* file-overlap wave partitioner (#3674), extracted from
* `claude-orchestration.cts`'s `partitionStages` (#1143).
*
* Module: gsd-core/bin/lib/file-overlap-partitioner.cjs
* Exported: partitionByFileOverlap(items: { id: string; files: string[] }[]) -> string[][]
*
* These tests pin the generic module's own contract, independent of any
* `Plan`/`Wave` shape from `claude-orchestration.cts`. Several test names
* carry a "(characterization)" suffix per the #3674 test matrix (rows 5, 6):
* they characterize `partitionStages`' original algorithm — traced from its
* source, since duplicate-id and chain-overlap inputs were never reachable
* through `emitWorkflowScript`'s public API (it validates plan ids unique
* before ever calling the partitioner) — now pinned against the extracted,
* generic entry point.
*/
const { describe, test } = require('node:test');
const assert = require('node:assert/strict');
const { partitionByFileOverlap } = require('../gsd-core/bin/lib/file-overlap-partitioner.cjs');
describe('partitionByFileOverlap', () => {
test('partitions overlapping plans into separate stages', () => {
const stages = partitionByFileOverlap([
{ id: 'p1', files: ['shared.ts'] },
{ id: 'p2', files: ['shared.ts'] },
{ id: 'p3', files: ['shared.ts'] },
]);
assert.strictEqual(stages.length, 3, 'every plan shares the same file -> each gets its own stage');
assert.deepStrictEqual(stages, [['p1'], ['p2'], ['p3']]);
});
test('coalesces disjoint plans into stage 0', () => {
const stages = partitionByFileOverlap([
{ id: 'p1', files: ['a.ts'] },
{ id: 'p2', files: ['b.ts'] },
{ id: 'p3', files: ['c.ts'] },
]);
assert.strictEqual(stages.length, 1, 'fully disjoint plans coalesce into one stage');
assert.deepStrictEqual(stages[0].slice().sort(), ['p1', 'p2', 'p3']);
});
test('an empty file set joins the first stage', () => {
const stages = partitionByFileOverlap([
{ id: 'p1', files: [] },
]);
assert.deepStrictEqual(stages, [['p1']]);
});
test('two plans with empty file sets do not collide', () => {
const stages = partitionByFileOverlap([
{ id: 'p1', files: [] },
{ id: 'p2', files: [] },
]);
assert.strictEqual(stages.length, 1, 'two empty file sets never overlap each other');
assert.deepStrictEqual(stages[0], ['p1', 'p2']);
});
test('duplicate plan ids are not deduplicated by the partitioner (characterization)', () => {
// Same id, overlapping files: each occurrence is placed independently by
// the greedy first-fit walk (id is never used as a dedup/merge key) — the
// second occurrence overlaps the first's file set and lands in stage 1.
const overlapping = partitionByFileOverlap([
{ id: 'p1', files: ['a.ts'] },
{ id: 'p1', files: ['a.ts'] },
]);
assert.deepStrictEqual(overlapping, [['p1'], ['p1']], 'both occurrences of the duplicate id are preserved, one per stage');
// Same id, disjoint files: both occurrences independently coalesce into
// stage 0 (files are what is checked for overlap, never the id).
const disjoint = partitionByFileOverlap([
{ id: 'p1', files: ['a.ts'] },
{ id: 'p1', files: ['b.ts'] },
]);
assert.deepStrictEqual(disjoint, [['p1', 'p1']], 'duplicate ids with disjoint files both land in stage 0, not merged');
});
test('chain-overlap plans produce the greedy, non-optimal assignment (characterization)', () => {
// A∩B (share f2), B∩C (share f3), A∌C (no shared file). A minimal
// assignment could place A and C together after B, but greedy first-fit
// processes in input order: A -> stage0; B overlaps A -> stage1; C does
// NOT overlap stage0 (A's files are f1,f2; C's are f3,f4) -> stage0.
const stages = partitionByFileOverlap([
{ id: 'A', files: ['f1', 'f2'] },
{ id: 'B', files: ['f2', 'f3'] },
{ id: 'C', files: ['f3', 'f4'] },
]);
assert.deepStrictEqual(stages, [['A', 'C'], ['B']], 'greedy first-fit, not optimal bin-packing');
});
test('path casing is compared by exact string, not normalized', () => {
const stages = partitionByFileOverlap([
{ id: 'p1', files: ['Foo.ts'] },
{ id: 'p2', files: ['foo.ts'] },
]);
assert.strictEqual(stages.length, 1, '"Foo.ts" and "foo.ts" are treated as distinct files -> no forced split');
assert.deepStrictEqual(stages[0], ['p1', 'p2']);
});
test('backslash and forward-slash paths are not normalized to the same file', () => {
const stages = partitionByFileOverlap([
{ id: 'p1', files: ['src\\foo.ts'] },
{ id: 'p2', files: ['src/foo.ts'] },
]);
assert.strictEqual(stages.length, 1, 'no separator normalization -> the two strings never compare equal');
assert.deepStrictEqual(stages[0], ['p1', 'p2']);
});
});