feat(#2632): executor actuals and the closed estimate-calibration loop (#2672)

* feat(#2632): record executor actuals and close the estimate calibration loop

* fix(#2632): calibrate against the raw projection so the loop converges

* test(#2632): add closed-loop convergence guard and codify the feedback-loop rule

* fix(#2632): pair calibration samples per plan; atomic write; amend adr

* chore(#2632): backfill changeset pr to 2672

* fix(#2632): retry renameSync on transient windows errnos and clean up the temp
This commit is contained in:
Tom Boucher
2026-07-26 16:28:56 -04:00
committed by GitHub
parent 920a5f3f06
commit bd570618d4
37 changed files with 943 additions and 147 deletions

View File

@@ -33,6 +33,35 @@ const { output, error, ERROR_REASON } = io;
const { planningDir } = planningWorkspace;
const { CONFIG_DEFAULTS } = configLoader;
// WIN-1 parity (DEFECT.WINDOWS-FS-OPS): on Windows a concurrent reader, indexer,
// or AV scanner can transiently hold the rename target open. Retry the transient
// errnos with backoff, matching the writeLedger / writeConsentStore idiom.
const RENAME_RETRY_ERRNOS = new Set(['EPERM', 'EBUSY', 'EACCES']);
const RENAME_MAX_ATTEMPTS = 3;
const RENAME_RETRY_BACKOFF_MS = 50;
let _renameSleepBuf: Int32Array | null = null;
function renameBackoff(): void {
if (_renameSleepBuf === null) _renameSleepBuf = new Int32Array(new SharedArrayBuffer(4));
Atomics.wait(_renameSleepBuf, 0, 0, RENAME_RETRY_BACKOFF_MS);
}
/** Rename with a bounded retry on the transient Windows errnos. Rethrows anything else. */
function renameWithRetry(from: string, to: string): void {
for (let attempt = 1; ; attempt += 1) {
try {
fs.renameSync(from, to);
return;
} catch (err) {
const code = (err as NodeJS.ErrnoException).code ?? '';
if (attempt < RENAME_MAX_ATTEMPTS && RENAME_RETRY_ERRNOS.has(code)) {
renameBackoff();
continue;
}
throw err;
}
}
}
/** Filename of the persisted calibration document, written by extract-learnings (Phase 3). */
export const CALIBRATION_FILENAME = 'estimation-calibration.json';
@@ -151,6 +180,130 @@ export function cmdEstimateCheck(cwd: string, args: string[], raw: boolean): voi
}, raw);
}
/**
* Pair each completed phase's PLAN estimate with its SUMMARY actuals.
*
* A phase contributes a sample only when BOTH sides are present and well-formed.
* A plan with no `estimate` block, a summary with no `actuals`, or a malformed
* value is skipped rather than guessed — a fabricated sample would silently
* steer every future estimate.
*/
export function collectCalibrationSamples(cwd: string): estimation.CalibrationSample[] {
const phasesRoot = path.join(planningDir(cwd), 'phases');
let phases: string[];
try {
phases = fs.readdirSync(phasesRoot, { withFileTypes: true })
.filter((d) => d.isDirectory())
.map((d) => d.name)
.sort();
} catch {
return [];
}
const readBlock = (file: string, key: string): Record<string, unknown> | null => {
let text: string;
try {
text = fs.readFileSync(file, 'utf-8');
} catch {
return null;
}
return estimation.extractFrontmatterBlock(text, key);
};
const samples: estimation.CalibrationSample[] = [];
for (const phase of phases) {
const dir = path.join(phasesRoot, phase);
let files: string[];
try {
files = fs.readdirSync(dir).sort();
} catch {
continue;
}
// Pair PER PLAN, keyed on the `<NN>-<PP>` stem, NOT per phase directory.
// A phase routinely holds several plans (docs/reference/planning-artifacts.md:
// "one file per plan"). Taking the first plan with an estimate and the first
// summary with actuals independently cross-pairs one plan's projection with
// another's cost — a fabricated sample — and discards every later plan.
const stems = new Map<string, { plan?: string; summary?: string }>();
for (const f of files) {
const m = /^(.*?)-(PLAN|SUMMARY)\.md$/.exec(f);
if (m === null) continue;
const stem = m[1];
const entry = stems.get(stem) ?? {};
if (m[2] === 'PLAN') entry.plan = path.join(dir, f);
else entry.summary = path.join(dir, f);
stems.set(stem, entry);
}
for (const stem of [...stems.keys()].sort()) {
const { plan, summary } = stems.get(stem) as { plan?: string; summary?: string };
if (plan === undefined || summary === undefined) continue;
const estimate = estimation.parseEstimate(readBlock(plan, 'estimate'));
const actuals = estimation.parseActuals(readBlock(summary, 'actuals'));
if (estimate === null || actuals === null) continue;
// Measure against the RAW projection — see PhaseEstimate.rawTokens for why
// measuring against the calibrated figure makes the loop self-defeating.
samples.push({
estimateTokens: estimation.calibrationBasis(estimate),
actualTokens: actuals.tokens,
});
}
}
return samples;
}
/**
* `estimate-calibrate` — rebuild the calibration document from completed phases.
*
* Rebuilds from scratch every run rather than appending, so it is idempotent and
* a corrupt prior document is replaced rather than merged. This is the verb that
* closes the loop (#1952 AC4): extract-learnings invokes it, and the planner's
* next estimate reads the result.
*/
export function cmdEstimateCalibrate(cwd: string, _args: string[], raw: boolean): void {
const samples = collectCalibrationSamples(cwd);
const calibration = estimation.computeCalibration(samples);
const target = path.join(planningDir(cwd), CALIBRATION_FILENAME);
let written = true;
let writeError: string | null = null;
try {
// Write-then-rename: a direct writeFileSync can leave a truncated file if
// interrupted, and parseCalibrationDocument treats malformed JSON exactly
// like "no history yet" — so a torn write would silently erase the
// calibration instead of surfacing.
const tmp = `${target}.tmp-${String(process.pid)}`;
try {
fs.writeFileSync(tmp, estimation.renderCalibrationDocument(samples), 'utf-8');
renameWithRetry(tmp, target);
} catch (err) {
// Never leave the temp behind for a later run to trip over.
try { fs.rmSync(tmp, { force: true }); } catch { /* best effort */ }
throw err;
}
} catch (err) {
// Persisting is best-effort: a read-only .planning must not fail the phase.
// But report WHY — a genuine bug and a benign permission issue are otherwise
// indistinguishable to both the caller and the workflow.
written = false;
writeError = err instanceof Error ? (err.message || String(err)) : String(err);
}
output({
factor: calibration.factor,
applied: calibration.applied,
sample_count: calibration.sampleCount,
confidence: calibration.confidence,
clamped: calibration.clamped,
min_samples: estimation.MIN_CALIBRATION_SAMPLES,
written,
write_error: writeError,
}, raw);
}
/**
* `estimate-calibration` — report the current correction factor and the
* history behind it.

View File

@@ -56,6 +56,19 @@ export interface PhaseEstimate {
tokens: number;
tasks: number;
confidence: Confidence;
/**
* The planner's UNCALIBRATED projection, before the correction factor was
* applied. Optional for backward compatibility with plans written before
* #2632.
*
* Calibration MUST measure actual/raw, not actual/calibrated. Measuring
* against the already-corrected figure makes the loop self-defeating: once
* the correction works, the observed ratio approaches 1, which drags the
* median back toward 1, which un-corrects the next estimate. Simulated over
* 10 phases with a true 2x underestimate, that oscillates and settles at
* ~1.41 instead of converging on 2.0.
*/
rawTokens?: number;
}
export interface PhaseActuals {
@@ -230,6 +243,46 @@ export function applyCalibration(rawTokens: unknown, factor: unknown): number {
return Math.min(Number.MAX_SAFE_INTEGER, Math.max(1, scaled));
}
/**
* Extract a two-space-indented scalar block (`estimate:` / `actuals:`) out of a
* document's leading YAML frontmatter.
*
* Hand-rolled because gsd-core ships no external dependencies (CONTRIBUTING.md
* "No external dependencies in core") — js-yaml is a devDependency and is not
* available at runtime. Scope is deliberately narrow: the leading `---` block
* only, so a `estimate:` line inside a fenced code block in the body cannot be
* mistaken for frontmatter (the DEFECT.FRONTMATTER-SCALAR-BROAD-GREP class).
*
* Numeric-looking values are returned as numbers so parseEstimate/parseActuals
* see the types they validate; everything else stays a string.
*/
export function extractFrontmatterBlock(text: unknown, key: string): Record<string, unknown> | null {
if (typeof text !== 'string') return null;
// Anchor at byte 0 — CRLF-tolerant.
const fm = /^---\r?\n([\s\S]*?)\r?\n---\r?(?:\n|$)/.exec(text);
if (fm === null) return null;
const lines = fm[1].split(/\r?\n/);
const startIdx = lines.findIndex((l) => l === `${key}:` || l.startsWith(`${key}:`));
if (startIdx === -1) return null;
const out: Record<string, unknown> = Object.create(null) as Record<string, unknown>;
for (let i = startIdx + 1; i < lines.length; i += 1) {
const line = lines[i];
if (!/^\s/.test(line)) break; // dedent ends the block
const m = /^\s+([A-Za-z_][\w-]*):\s*(.*)$/.exec(line);
if (m === null) continue;
const rawValue = m[2].replace(/\s+#.*$/, '').trim();
if (rawValue === '') continue;
const asNumber = Number(rawValue);
out[m[1]] = /^-?\d+(?:\.\d+)?$/.test(rawValue) && Number.isFinite(asNumber)
? asNumber
: rawValue.replace(/^['"]|['"]$/g, '');
}
return Object.keys(out).length > 0 ? { ...out } : null;
}
/** Pull the `estimate:` mapping out of an already-parsed frontmatter object. */
function estimateBlockOf(input: unknown): unknown {
if (input === null || typeof input !== 'object') return null;
@@ -256,7 +309,10 @@ export function parseEstimate(input: unknown): PhaseEstimate | null {
if (!isPositiveInt(tokens) || !isPositiveInt(tasks) || !isConfidence(confidence)) return null;
return { tokens, tasks, confidence };
const rawTokens = record['raw_tokens'];
return isPositiveInt(rawTokens)
? { tokens, tasks, confidence, rawTokens }
: { tokens, tasks, confidence };
}
/** Pull the `actuals:` mapping out of an already-parsed frontmatter object. */
@@ -292,12 +348,22 @@ export function parseActuals(input: unknown): PhaseActuals | null {
* property test pins.
*/
export function renderEstimate(estimate: PhaseEstimate): string {
return [
const lines = [
'estimate:',
` tokens: ${estimate.tokens}`,
` tasks: ${estimate.tasks}`,
` confidence: ${estimate.confidence}`,
].join('\n');
];
if (isPositiveInt(estimate.rawTokens)) lines.push(` raw_tokens: ${estimate.rawTokens}`);
lines.push(` tasks: ${estimate.tasks}`, ` confidence: ${estimate.confidence}`);
return lines.join('\n');
}
/**
* The figure calibration must measure against: the uncalibrated projection when
* the plan recorded one, else the stored value (pre-#2632 plans, where the two
* were the same because no factor had yet been applied).
*/
export function calibrationBasis(estimate: PhaseEstimate): number {
return isPositiveInt(estimate.rawTokens) ? estimate.rawTokens : estimate.tokens;
}
/** Render an actuals block for SUMMARY.md frontmatter. Inverse of parseActuals. */