From a5633bb32ff842714e05a6692c36081a7700e1a8 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 26 Jul 2026 21:42:50 -0400 Subject: [PATCH] enhance(#2671): brand raw vs calibrated token types so double-application is a compile error (#2676) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(#2671): add failing-first brand-typing compile fixtures * feat(#2671): brand raw vs calibrated token types * refactor(#2671): hoist type-compile into a before() hook Two review responses: - The fixture compile ran in the describe() body, so it executed at collection time even when the block was filtered out, and a failed precondition collapsed eight independent assertions into one opaque describe-level failure. A before() hook is this repo's documented idiom and preserves per-test granularity. - parseTokensFlag now records WHY it returns an unbranded number: it validates the magnitude of --tokens, but the basis is decided by --calibrated, so branding here would be wrong for half its callers. The assertion belongs to cmdEstimateCheck, its only caller. * test(#2671): pin each brand diagnostic to its OFFENDING marker Adversarial review demonstrated that asserting only exactly-one-diagnostic- at-code-N is not airtight. Repairing a fixture's brand violation while injecting an unrelated error of the same code (a string passed as the budget argument) still yielded exactly one TS2345, so the fixture would have reported green while no longer testing its regression at all. Each bad-* fixture now routes its violating value through a const named OFFENDING, and the test asserts the diagnostic's start offset falls inside that node — located through the AST, so it survives reformatting and never pattern-matches source text. Replaying the proof-of-concept against the new assertion rejects it: the diagnostic lands on the budget literal, not the marker. Also corrects a doc comment that claimed the program type-checks all of src/; it covers phase-estimation.cts and its transitive dependencies. * chore(#2671): backfill changeset PR number (#2676) --- .changeset/sturdy-dogs-wake.md | 5 + CONTEXT.md | 2 +- ...629-phase-effort-estimation-calibration.md | 1 + src/estimate-cli.cts | 19 ++- src/phase-estimation.cts | 114 +++++++++++-- tests/fixtures/brand-typing/README.md | 45 +++++ .../bad-calibrated-as-sample-basis.cts | 29 ++++ .../brand-typing/bad-double-calibration.cts | 17 ++ .../brand-typing/bad-raw-against-budget.cts | 16 ++ .../bad-rebrand-calibrated-as-raw.cts | 20 +++ .../bad-unbranded-number-as-raw.cts | 16 ++ .../brand-typing/ok-correct-composition.cts | 37 +++++ tests/phase-estimation.test.cjs | 155 +++++++++++++++++- 13 files changed, 460 insertions(+), 16 deletions(-) create mode 100644 .changeset/sturdy-dogs-wake.md create mode 100644 tests/fixtures/brand-typing/README.md create mode 100644 tests/fixtures/brand-typing/bad-calibrated-as-sample-basis.cts create mode 100644 tests/fixtures/brand-typing/bad-double-calibration.cts create mode 100644 tests/fixtures/brand-typing/bad-raw-against-budget.cts create mode 100644 tests/fixtures/brand-typing/bad-rebrand-calibrated-as-raw.cts create mode 100644 tests/fixtures/brand-typing/bad-unbranded-number-as-raw.cts create mode 100644 tests/fixtures/brand-typing/ok-correct-composition.cts diff --git a/.changeset/sturdy-dogs-wake.md b/.changeset/sturdy-dogs-wake.md new file mode 100644 index 000000000..cddbaf9b1 --- /dev/null +++ b/.changeset/sturdy-dogs-wake.md @@ -0,0 +1,5 @@ +--- +type: Changed +pr: 2676 +--- +**Raw and calibrated phase-estimate token counts are now distinct types** — the two states of an estimate (the planner's uncorrected projection and the same figure with the project's calibration factor applied) could previously be swapped at any seam without complaint, because both are plain positive integers. That produced two shipped defects in epic #1952: a doubly-applied correction (factor squared) and a calibration loop that measured against its own output and never converged. Both are now compile errors. No behavior, output, or schema change. (#2671) diff --git a/CONTEXT.md b/CONTEXT.md index fef92faf0..4c0145fed 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -21,7 +21,7 @@ Module owning the pure phase-id parsing and matching helpers: phase-name normali Module owning phase create, rename, complete, remove, list, and plan-index operations, plus phase-dir prefix validation, STATE.md staleness detection, and auto-prune behaviour. Entry point: `gsd-core/bin/lib/phase.cjs` (CJS surface). Typed phase events: `GSDPhaseStartEvent`, `GSDPhaseStepStartEvent`, `GSDPhaseStepCompleteEvent`, `GSDPhaseCompleteEvent`. (The SDK native-query surface, the `types.ts` event definitions, `phase-runner.ts`, and `phase-prompt.ts` were retired with the SDK package per ADR-0174.) ### Phase Estimation Module -Module owning phase-effort estimation and its calibration against measured reality (ADR-2629, epic #1952). Pure — no I/O, no config reads; the CLI seam (`src/estimate-cli.cts`, verbs `estimate-check` / `estimate-calibration`) owns reading `.planning/config.json` and `.planning/estimation-calibration.json`. Interface: `parseEstimate`/`renderEstimate` (the PLAN.md `estimate: {tokens, tasks, confidence}` block), `parseActuals`/`renderActuals` (the SUMMARY.md `actuals: {tokens, tasks, commits}` block), `deriveConfidence(sampleCount) → low|med|high`, `classifyAgainstBudget(estimate, budget) → {overBudget, ratio, recommendation, budgetValid}`, `computeCalibration(samples) → {factor, sampleCount, applied, confidence, clamped}`, `applyCalibration`, `parseCalibrationDocument`/`renderCalibrationDocument`, `extractFrontmatterBlock` (leading-`---`-anchored scalar-block reader; hand-rolled because core ships no external deps), `calibrationBasis` (returns `estimate.raw_tokens` when present, else `tokens` — calibration must measure actual/raw or the loop un-corrects itself), and `measureTokens` (a re-export of `prompt-budget`'s `estimateTokens`). **Domain terms: _smart zone_** — the usable prefix of a model's context window before output quality degrades, expressed as the configurable `workflow.smart_zone_tokens` budget (default 100000, a *policy default* rather than a benchmark constant since the effective ceiling is model/task-dependent); **_estimate/actuals_** — a projected phase cost recorded at plan time and the measured cost recorded at completion, both on the **same `estimateTokens` scale** so their ratio measures the miss rather than a difference between two measurement methods. Two invariants: (1) every signal is **exogenous** — the correction routes on a measured actual/estimate ratio and `confidence` routes on a calibration sample count, never on a model's self-assessment (this project measured self-rated confidence and found it weak — `gsd-core/references/honest-verifier.md:25-29`; see `.out-of-scope/general-purpose-agent-prompt-skills.md`); (2) the over-budget flag is **advisory** — a warning plus a split recommendation, never a block. Calibration is median-of-ratios, clamped to `[0.5, 3.0]`, and inert below 3 samples. CLI seam verbs: `estimate-check` (classify one figure; `--calibrated` when the input already has the factor applied — omitting it squares the correction), `estimate-calibration` (report the current factor), `estimate-calibrate` (#2632 — pair every completed phase's PLAN `estimate` with its SUMMARY `actuals`, rebuild `.planning/estimation-calibration.json` idempotently, and report the result; this is what closes the loop). Source of truth: `gsd-core/bin/lib/phase-estimation.cjs` (generated from `src/phase-estimation.cts`) and `src/estimate-cli.cts`. Test anchors: `tests/phase-estimation.test.cjs`, `tests/estimate-calibrate.test.cjs`. +Module owning phase-effort estimation and its calibration against measured reality (ADR-2629, epic #1952). Pure — no I/O, no config reads; the CLI seam (`src/estimate-cli.cts`, verbs `estimate-check` / `estimate-calibration`) owns reading `.planning/config.json` and `.planning/estimation-calibration.json`. Interface: `parseEstimate`/`renderEstimate` (the PLAN.md `estimate: {tokens, tasks, confidence}` block), `parseActuals`/`renderActuals` (the SUMMARY.md `actuals: {tokens, tasks, commits}` block), `deriveConfidence(sampleCount) → low|med|high`, `classifyAgainstBudget(estimate, budget) → {overBudget, ratio, recommendation, budgetValid}`, `computeCalibration(samples) → {factor, sampleCount, applied, confidence, clamped}`, `applyCalibration`, `parseCalibrationDocument`/`renderCalibrationDocument`, `extractFrontmatterBlock` (leading-`---`-anchored scalar-block reader; hand-rolled because core ships no external deps), `calibrationBasis` (returns `estimate.raw_tokens` when present, else `tokens` — calibration must measure actual/raw or the loop un-corrects itself), and `measureTokens` (a re-export of `prompt-budget`'s `estimateTokens`). **Domain terms: _raw_ vs _calibrated_ tokens** — the same token count in two mutually incompatible states, carried by the compile-time brands `RawTokens` (the planner's uncorrected projection, and the only legal calibration denominator) and `CalibratedTokens` (the projection with the project's factor applied, and the only figure meaningful against the budget), constructed at trust boundaries via `asRawTokens` / `asCalibratedTokens` (#2671). The brands erase at compile time — the emitted `.cjs`, the CLI JSON, and both frontmatter schemas are unchanged — and exist because mixing the two states was NOT catchable at runtime: both are positive integers of the same magnitude, and the mix-up shipped twice past a green ~26,800-test suite (#2631 factor², #2632 self-defeating loop). `asRawTokens` refuses a `CalibratedTokens` by design; the single legitimate crossover (a pre-#2632 plan whose `tokens` IS the raw projection) lives behind one commented assertion in `calibrationBasis`. Compile fixtures: `tests/fixtures/brand-typing/`. **_smart zone_** — the usable prefix of a model's context window before output quality degrades, expressed as the configurable `workflow.smart_zone_tokens` budget (default 100000, a *policy default* rather than a benchmark constant since the effective ceiling is model/task-dependent); **_estimate/actuals_** — a projected phase cost recorded at plan time and the measured cost recorded at completion, both on the **same `estimateTokens` scale** so their ratio measures the miss rather than a difference between two measurement methods. Two invariants: (1) every signal is **exogenous** — the correction routes on a measured actual/estimate ratio and `confidence` routes on a calibration sample count, never on a model's self-assessment (this project measured self-rated confidence and found it weak — `gsd-core/references/honest-verifier.md:25-29`; see `.out-of-scope/general-purpose-agent-prompt-skills.md`); (2) the over-budget flag is **advisory** — a warning plus a split recommendation, never a block. Calibration is median-of-ratios, clamped to `[0.5, 3.0]`, and inert below 3 samples. CLI seam verbs: `estimate-check` (classify one figure; `--calibrated` when the input already has the factor applied — omitting it squares the correction), `estimate-calibration` (report the current factor), `estimate-calibrate` (#2632 — pair every completed phase's PLAN `estimate` with its SUMMARY `actuals`, rebuild `.planning/estimation-calibration.json` idempotently, and report the result; this is what closes the loop). Source of truth: `gsd-core/bin/lib/phase-estimation.cjs` (generated from `src/phase-estimation.cts`) and `src/estimate-cli.cts`. Test anchors: `tests/phase-estimation.test.cjs`, `tests/estimate-calibrate.test.cjs`. ### Verification Module Module owning the canonical phase-verification status projection shared by phase transition, progress, manager, autonomous, and closeout readiness paths. `readVerificationStatus(phaseDir, opts?)` reads the first `*-VERIFICATION.md` frontmatter `status`, maps it through `VERIFICATION_ROUTING_TABLE`, and fail-closes — only `{passed}` satisfies the canonical gate; `missing`/`unknown`/`gaps_found`/`human_needed`/`stale` all route away from "complete" (#1522). `findStaleVerificationSummary` flags a SUMMARY newer than the VERIFICATION file (status `stale`). Both honor a no-throw, degrade-to-safe contract (any FS error → `missing` / not-stale) and an injectable `opts.fs` seam. Source of truth: `gsd-core/bin/lib/verification.cjs` (generated from `src/verification.cts`). diff --git a/docs/adr/2629-phase-effort-estimation-calibration.md b/docs/adr/2629-phase-effort-estimation-calibration.md index 578832db0..dcb104c62 100644 --- a/docs/adr/2629-phase-effort-estimation-calibration.md +++ b/docs/adr/2629-phase-effort-estimation-calibration.md @@ -76,6 +76,7 @@ factor = 1.0 when n < 3 - **`n >= 3` before any correction applies** — below that, the sample says more about variance than about bias. - **The denominator is the RAW projection, not the emitted (already-corrected) figure** — amended #2632. Measuring `actual / calibrated` is self-defeating: once the correction works the observed ratio approaches 1, which drags the median back toward 1 and un-corrects the next estimate. Simulated over 10 phases against a true 2x miss it oscillates and settles near 1.41 instead of converging on 2.0. Plans therefore record `estimate.raw_tokens` alongside the calibrated `estimate.tokens`, and `calibrationBasis()` prefers it (falling back to `tokens` for plans written before #2632, where no factor had yet been applied). - **Samples are per PLAN, not per phase** — amended #2632. A phase holds several `--PLAN.md` files; pairing at phase granularity cross-pairs one plan's projection with another's cost and discards the rest. +- **The raw/calibrated distinction is enforced by the type system, not by convention** — amended #2671. `--calibrated` and `raw_tokens` fix the two known call sites but remain conventions the *next* caller must also remember, and the failure mode is silent: both states are positive integers of the same magnitude, so no runtime check can tell them apart. `src/phase-estimation.cts` therefore gives them distinct branded types, `RawTokens` and `CalibratedTokens`, so `applyCalibration(alreadyCalibrated, factor)` and `{ estimateTokens: estimate.tokens }` are compile errors under `npm run build:lib` rather than review findings. The brands erase at compile time — no `.cjs` behavior change, no wire-format change, and untyped `.cjs` callers are unaffected, which is why every runtime guard in the module stays in place. Compile fixtures live in `tests/fixtures/brand-typing/` and are driven through the TypeScript compiler API by `tests/phase-estimation.test.cjs`. Persisted to `.planning/estimation-calibration.json` with a `schema_version` field, written by `extract-learnings`, read at plan time. Versioned from the first write so the schema can migrate without a silent misread. diff --git a/src/estimate-cli.cts b/src/estimate-cli.cts index 89b7a09cc..3f3645c9e 100644 --- a/src/estimate-cli.cts +++ b/src/estimate-cli.cts @@ -114,6 +114,15 @@ export function readCalibrationSamples(cwd: string): ReturnType; + // The RawTokens brand on estimateTokens is asserted here, at the disk trust + // boundary — a persisted sample's basis is a fact about the writer, and the + // only writers are collectCalibrationSamples() (which reads it through + // calibrationBasis()) and this module's own renderCalibrationDocument(). return isPositiveFinite(record['estimateTokens']) && isPositiveFinite(record['actualTokens']); } @@ -169,7 +242,10 @@ export function deriveConfidence(sampleCount: unknown): Confidence { * violation: it reports budgetValid=false and overBudget=false, so a broken * config cannot spam split recommendations. */ -export function classifyAgainstBudget(estimate: unknown, budget: unknown): BudgetClassification { +export function classifyAgainstBudget(estimate: CalibratedTokens, budget: number): BudgetClassification { + // Kept for untyped `.cjs` callers — see the note in applyCalibration. A + // hand-edited config reaches `budget` as anything at runtime regardless of + // what the TypeScript signature promises. if (!isPositiveFinite(budget) || !isPositiveFinite(estimate)) { return { overBudget: false, ratio: 0, recommendation: null, budgetValid: isPositiveFinite(budget) }; } @@ -232,15 +308,20 @@ export function computeCalibration(samples: unknown): CalibrationResult { * `estimate.tokens` is an integer field; floors at 1 so a heavy shrink factor * can never produce a zero-token estimate. */ -export function applyCalibration(rawTokens: unknown, factor: unknown): number { - if (!isPositiveFinite(rawTokens)) return 0; - if (!isPositiveFinite(factor)) return Math.max(1, Math.round(rawTokens)); +export function applyCalibration(rawTokens: RawTokens, factor: number): CalibratedTokens { + // These two guards look dead to the type-checker and are not: this module is + // compiled to `.cjs` and consumed by untyped callers (gsd-tools.cjs, the test + // suite), which reach it with NaN, null, 0 and worse. The brands are a + // compile-time contract for TypeScript callers; validation is what defends + // everyone else. Do not delete either one because the parameter is now typed. + if (!isPositiveFinite(rawTokens)) return asCalibratedTokens(0); + if (!isPositiveFinite(factor)) return asCalibratedTokens(Math.max(1, Math.round(rawTokens))); // Bound the product: an inexact float past MAX_SAFE_INTEGER would masquerade // as an integer token count. Unreachable through today's CLI (which is // safe-integer bounded) but the function is exported and must not depend on // its caller for that guarantee. const scaled = Math.round(rawTokens * factor); - return Math.min(Number.MAX_SAFE_INTEGER, Math.max(1, scaled)); + return asCalibratedTokens(Math.min(Number.MAX_SAFE_INTEGER, Math.max(1, scaled))); } /** @@ -309,10 +390,13 @@ export function parseEstimate(input: unknown): PhaseEstimate | null { if (!isPositiveInt(tokens) || !isPositiveInt(tasks) || !isConfidence(confidence)) return null; + // The frontmatter trust boundary: `tokens` is calibrated-at-emission and + // `raw_tokens` is the uncorrected projection (ADR-2629 Decision 1/4), so this + // is where each figure's basis becomes a type rather than a field name. const rawTokens = record['raw_tokens']; return isPositiveInt(rawTokens) - ? { tokens, tasks, confidence, rawTokens } - : { tokens, tasks, confidence }; + ? { tokens: asCalibratedTokens(tokens), tasks, confidence, rawTokens: asRawTokens(rawTokens) } + : { tokens: asCalibratedTokens(tokens), tasks, confidence }; } /** Pull the `actuals:` mapping out of an already-parsed frontmatter object. */ @@ -362,8 +446,14 @@ export function renderEstimate(estimate: PhaseEstimate): string { * 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; +export function calibrationBasis(estimate: PhaseEstimate): RawTokens { + if (isPositiveInt(estimate.rawTokens)) return estimate.rawTokens; + // THE one legitimate crossover in this module, and the reason asRawTokens() + // refuses a CalibratedTokens rather than being permissive: on a plan written + // before #2632 no factor had been applied yet, so `tokens` IS the raw + // projection. Deliberately an explicit assertion so it stays a single + // auditable line instead of a hole in the brand. + return estimate.tokens as unknown as RawTokens; } /** Render an actuals block for SUMMARY.md frontmatter. Inverse of parseActuals. */ diff --git a/tests/fixtures/brand-typing/README.md b/tests/fixtures/brand-typing/README.md new file mode 100644 index 000000000..245407981 --- /dev/null +++ b/tests/fixtures/brand-typing/README.md @@ -0,0 +1,45 @@ +# Brand-typing compile fixtures (#2671) + +These `.cts` files are **compiler inputs, not runtime code**. They are deliberately +excluded from `tsconfig.json` / `tsconfig.build.json` (both include `src/**/*.cts` +only), so they never enter `npm run build:lib` and never emit a `.cjs`. `tests/` is +not in the package's `files` list either, so they do not ship. + +`tests/phase-estimation.test.cjs` compiles them in-process with the TypeScript +compiler API, using the repo's real `tsconfig.build.json` options, and asserts on +the returned **diagnostic objects** (`code`, `file`, `start`) — never on rendered +compiler prose. + +| Fixture | Must | Guards | +|---|---|---| +| `ok-correct-composition.cts` | compile clean | the positive control — proves the harness, imports, and option set are sound | +| `bad-double-calibration.cts` | fail | #2631 — applying the factor to an already-corrected figure (factor²) | +| `bad-raw-against-budget.cts` | fail | comparing an uncorrected projection against the smart-zone budget | +| `bad-calibrated-as-sample-basis.cts` | fail | #2632 — calibrating against the emitted figure instead of the raw basis | +| `bad-rebrand-calibrated-as-raw.cts` | fail | laundering a corrected figure back into the raw basis | +| `bad-unbranded-number-as-raw.cts` | fail | proves the brand is not vacuously `number` | + +## The three rules that keep these non-vacuous + +A negative-compile test is worthless if it can pass for the wrong reason. Three +independent checks prevent that, and each exists because of a demonstrated failure: + +1. **The positive control must compile clean.** Otherwise a broken import path or + an unusable option set would make every `bad-*` fixture "fail correctly" while + testing nothing. +2. **No diagnostic may originate outside this directory.** Otherwise a real compile + error in `src/` could hide inside fixture noise. +3. **Each `bad-*` fixture routes its violating value through a const named + `OFFENDING`, and the diagnostic must land on that node.** Code-and-count alone + is not enough — an adversarial review proved that a fixture whose brand + violation had been *repaired*, but which gained an unrelated error of the same + code (a string passed as the budget), still produced "exactly one TS2345" and + would have reported green while no longer testing its regression at all. + +Each `bad-*` fixture therefore contains **exactly one** deliberate type error, on +its `OFFENDING` marker. Adding a second error, or moving the violation off the +marker, breaks the contract on purpose — add a new fixture instead. + +TypeScript reports an object-literal property mismatch on the property *name* +rather than its initializer, so the marker check also accepts the name of a +property initialized from `OFFENDING` (see `bad-calibrated-as-sample-basis.cts`). diff --git a/tests/fixtures/brand-typing/bad-calibrated-as-sample-basis.cts b/tests/fixtures/brand-typing/bad-calibrated-as-sample-basis.cts new file mode 100644 index 000000000..81a26707f --- /dev/null +++ b/tests/fixtures/brand-typing/bad-calibrated-as-sample-basis.cts @@ -0,0 +1,29 @@ +/** + * MUST NOT COMPILE (#2671) — the #2632 defect: the self-defeating loop. + * + * Calibration must measure actual/raw. Measuring actual/calibrated makes the + * loop un-correct itself: once the correction works the observed ratio + * approaches 1, which drags the median back toward 1. Simulated over 10 phases + * against a true 2x miss it oscillates and settles near 1.41 instead of + * converging on 2.0 (ADR-2629 Decision 4, amended #2632). + * + * `OFFENDING` is the marker the test pins the diagnostic to — see the README. + * TypeScript reports an object-literal property mismatch on the property NAME, + * so the test accepts that span too. + */ + +import estimation = require('../../../src/phase-estimation.cjs'); + +const estimate: estimation.PhaseEstimate = { + tokens: estimation.asCalibratedTokens(100000), + rawTokens: estimation.asRawTokens(50000), + tasks: 5, + confidence: 'med', +}; + +const OFFENDING = estimate.tokens; + +export const sample: estimation.CalibrationSample = { + estimateTokens: OFFENDING, + actualTokens: 74000, +}; diff --git a/tests/fixtures/brand-typing/bad-double-calibration.cts b/tests/fixtures/brand-typing/bad-double-calibration.cts new file mode 100644 index 000000000..32d571ee1 --- /dev/null +++ b/tests/fixtures/brand-typing/bad-double-calibration.cts @@ -0,0 +1,17 @@ +/** + * MUST NOT COMPILE (#2671) — the #2631 defect in its pure form. + * + * `gsd-planner` emitted an already-calibrated figure; `gsd-plan-checker` fed it + * to a parameter named `rawTokens`, which applied the factor a second time. The + * effective correction became factor^2 — with the [0.5, 3.0] clamp, anywhere + * from 4x under to 9x over — and it was invisible below 3 samples because there + * factor === 1 and 1^2 === 1. + * + * `OFFENDING` is the marker the test pins the diagnostic to — see the README. + */ + +import estimation = require('../../../src/phase-estimation.cjs'); + +const OFFENDING = estimation.applyCalibration(estimation.asRawTokens(50000), 2); + +export const squared = estimation.applyCalibration(OFFENDING, 2); diff --git a/tests/fixtures/brand-typing/bad-raw-against-budget.cts b/tests/fixtures/brand-typing/bad-raw-against-budget.cts new file mode 100644 index 000000000..7edb2e786 --- /dev/null +++ b/tests/fixtures/brand-typing/bad-raw-against-budget.cts @@ -0,0 +1,16 @@ +/** + * MUST NOT COMPILE (#2671) — an uncorrected projection compared to the budget. + * + * The smart-zone verdict is only meaningful against the project's corrected + * figure. Classifying the raw projection reports the estimator's bias as if it + * were the phase's size, and silently under-reports every over-budget phase on + * a project whose factor is above 1. + * + * `OFFENDING` is the marker the test pins the diagnostic to — see the README. + */ + +import estimation = require('../../../src/phase-estimation.cjs'); + +const OFFENDING = estimation.asRawTokens(50000); + +export const verdict = estimation.classifyAgainstBudget(OFFENDING, 100000); diff --git a/tests/fixtures/brand-typing/bad-rebrand-calibrated-as-raw.cts b/tests/fixtures/brand-typing/bad-rebrand-calibrated-as-raw.cts new file mode 100644 index 000000000..d63a5acd2 --- /dev/null +++ b/tests/fixtures/brand-typing/bad-rebrand-calibrated-as-raw.cts @@ -0,0 +1,20 @@ +/** + * MUST NOT COMPILE (#2671) — laundering a corrected figure back into the basis. + * + * Without this guard the brand would be decorative: `asRawTokens()` would accept + * any number, so re-labelling a calibrated figure as raw would restore both + * shipped defects through the front door. + * + * The one legitimate crossover — a pre-#2632 plan whose `tokens` IS the raw + * projection because no factor had been applied yet — lives in + * `calibrationBasis()` behind an explicit, commented assertion, so it stays a + * single auditable line rather than an open door. + * + * `OFFENDING` is the marker the test pins the diagnostic to — see the README. + */ + +import estimation = require('../../../src/phase-estimation.cjs'); + +const OFFENDING = estimation.asCalibratedTokens(100000); + +export const relabelled = estimation.asRawTokens(OFFENDING); diff --git a/tests/fixtures/brand-typing/bad-unbranded-number-as-raw.cts b/tests/fixtures/brand-typing/bad-unbranded-number-as-raw.cts new file mode 100644 index 000000000..33b4650c3 --- /dev/null +++ b/tests/fixtures/brand-typing/bad-unbranded-number-as-raw.cts @@ -0,0 +1,16 @@ +/** + * MUST NOT COMPILE (#2671) — proves the brand is not vacuously `number`. + * + * If `RawTokens` were a plain alias for `number`, every other fixture here would + * still compile and the whole guard would be theatre. A bare number carries no + * basis, so it must pass through `asRawTokens` / `asCalibratedTokens` and the + * caller must state which one it is. + * + * `OFFENDING` is the marker the test pins the diagnostic to — see the README. + */ + +import estimation = require('../../../src/phase-estimation.cjs'); + +const OFFENDING = 50000; + +export const calibrated = estimation.applyCalibration(OFFENDING, 2); diff --git a/tests/fixtures/brand-typing/ok-correct-composition.cts b/tests/fixtures/brand-typing/ok-correct-composition.cts new file mode 100644 index 000000000..070b634cd --- /dev/null +++ b/tests/fixtures/brand-typing/ok-correct-composition.cts @@ -0,0 +1,37 @@ +/** + * POSITIVE CONTROL (#2671) — the correct composition must compile clean. + * + * Without this, every `bad-*` fixture could be passing for the wrong reason (a + * typo, a bad import path, a mis-built compiler option set) and the suite would + * still be green. This file is what makes the negative fixtures non-vacuous. + */ + +import estimation = require('../../../src/phase-estimation.cjs'); + +const FACTOR = 2; +const BUDGET = 100000; + +// argv/disk figure -> caller asserts the basis -> correction -> budget verdict. +const raw = estimation.asRawTokens(50000); +const calibrated = estimation.applyCalibration(raw, FACTOR); +export const verdict = estimation.classifyAgainstBudget(calibrated, BUDGET); + +// `estimate-check --calibrated`: the caller states the factor is already applied, +// so the figure goes straight to the budget without a second correction. +export const preCalibratedVerdict = estimation.classifyAgainstBudget( + estimation.asCalibratedTokens(50000), + BUDGET, +); + +// ADR-2629 Decision 4: the calibration denominator is the RAW basis. +const estimate: estimation.PhaseEstimate = { + tokens: estimation.asCalibratedTokens(100000), + rawTokens: estimation.asRawTokens(50000), + tasks: 5, + confidence: 'med', +}; + +export const sample: estimation.CalibrationSample = { + estimateTokens: estimation.calibrationBasis(estimate), + actualTokens: 74000, +}; diff --git a/tests/phase-estimation.test.cjs b/tests/phase-estimation.test.cjs index 2c43742ad..94dda9780 100644 --- a/tests/phase-estimation.test.cjs +++ b/tests/phase-estimation.test.cjs @@ -15,7 +15,7 @@ * limit-1 / limit / limit+1 per RULESET.TESTS.boundary-coverage.fixtures. */ -const { describe, test } = require('node:test'); +const { describe, test, before } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); @@ -733,3 +733,156 @@ describe('estimate-check --calibrated', () => { assert.equal(pre.over_budget, false, 'the honest figure is under budget'); }); }); + +// ─── branded raw-vs-calibrated basis (#2671) ─────────────────────────────── + +describe('RawTokens / CalibratedTokens brands', () => { + // The behavioural guards above pin the two shipped defects (#2631 factor^2, + // #2632 self-defeating loop) at the CLI surface. Both were composition errors + // between individually-correct functions, and ~26,800 unit/boundary/property + // tests were green for both. This block asserts the stronger property: with + // the brands in place the wrong composition is not merely wrong, it is + // UNREPRESENTABLE — `npm run build:lib` refuses it. + // + // The oracle is the TypeScript compiler, driven in-process through its API + // (no subprocess, so no timeout and no spawn flake) against the repo's REAL + // tsconfig.build.json options — the same strictness the publish build uses. + // Assertions are on the returned diagnostic OBJECTS (`code`, `file`), never + // on rendered compiler prose. + const ts = require('typescript'); + + const REPO_ROOT = path.join(__dirname, '..'); + const FIXTURE_DIR = path.join(__dirname, 'fixtures', 'brand-typing'); + + /** TS "argument of type X is not assignable to parameter of type Y". */ + const TS_ARG_NOT_ASSIGNABLE = 2345; + /** TS "type X is not assignable to type Y" (object-literal property). */ + const TS_TYPE_NOT_ASSIGNABLE = 2322; + + const CASES = [ + { fixture: 'ok-correct-composition.cts', expected: null }, + { fixture: 'bad-double-calibration.cts', expected: TS_ARG_NOT_ASSIGNABLE }, + { fixture: 'bad-raw-against-budget.cts', expected: TS_ARG_NOT_ASSIGNABLE }, + { fixture: 'bad-calibrated-as-sample-basis.cts', expected: TS_TYPE_NOT_ASSIGNABLE }, + { fixture: 'bad-rebrand-calibrated-as-raw.cts', expected: TS_ARG_NOT_ASSIGNABLE }, + { fixture: 'bad-unbranded-number-as-raw.cts', expected: TS_ARG_NOT_ASSIGNABLE }, + ]; + + /** + * Every `bad-*` fixture routes its violating value through a const with this + * name, and the test asserts the diagnostic lands ON that node. + * + * Code-and-count alone is NOT enough: a fixture that stops exercising its + * brand violation but acquires an unrelated error of the same code still + * yields "exactly one TS2345" and would report green while testing nothing. + * That was demonstrated against an earlier version of this block, so the + * position check is a regression guard, not a precaution. + */ + const OFFENDING = 'OFFENDING'; + + /** + * Spans a diagnostic is allowed to occupy: any occurrence of the marker + * identifier, plus — because TypeScript reports an object-literal property + * mismatch on the property NAME rather than its initializer — the name of any + * property initialized from the marker. Located through the AST, so this + * survives reformatting and never pattern-matches source text. + */ + const markerSpans = (sourceFile) => { + const spans = []; + const visit = (node) => { + if (ts.isIdentifier(node) && node.text === OFFENDING) { + spans.push([node.getStart(sourceFile), node.getEnd()]); + } else if (ts.isPropertyAssignment(node) + && ts.isIdentifier(node.initializer) + && node.initializer.text === OFFENDING) { + spans.push([node.name.getStart(sourceFile), node.name.getEnd()]); + } + ts.forEachChild(node, visit); + }; + visit(sourceFile); + return spans; + }; + + /** + * Compile every fixture in ONE program and bucket the diagnostics by source + * file. The program covers `phase-estimation.cts` and its transitive + * dependencies — not all of `src/`, which `npm run build:lib` gates + * separately — which is what makes the "no foreign diagnostics" assertion + * below meaningful: anything outside the fixture directory is a real compile + * error in the module under test. + */ + let byFixture; + let foreign; + let sourceFileOf; + + before(() => { + const configPath = path.join(REPO_ROOT, 'tsconfig.build.json'); + const readConfig = ts.readConfigFile(configPath, ts.sys.readFile); + assert.equal(readConfig.error, undefined, 'tsconfig.build.json must parse'); + + const parsed = ts.parseJsonConfigFileContent(readConfig.config, ts.sys, REPO_ROOT); + assert.deepEqual(parsed.errors, [], 'tsconfig.build.json must yield usable compiler options'); + + const options = { + ...parsed.options, + // The fixtures live outside `src/`, so the emit-shaped settings have to go. + // Everything that governs STRICTNESS is inherited untouched — that is the + // whole point of reading the real config instead of hand-rolling options. + noEmit: true, + rootDir: undefined, + outDir: undefined, + incremental: false, + tsBuildInfoFile: undefined, + }; + + const roots = CASES.map((c) => path.join(FIXTURE_DIR, c.fixture)); + const program = ts.createProgram(roots, options); + + byFixture = new Map(CASES.map((c) => [c.fixture, []])); + foreign = []; + sourceFileOf = new Map( + CASES.map((c) => [c.fixture, program.getSourceFile(path.join(FIXTURE_DIR, c.fixture))]), + ); + for (const diagnostic of ts.getPreEmitDiagnostics(program)) { + const name = diagnostic.file === undefined ? null : path.basename(diagnostic.file.fileName); + if (name !== null && byFixture.has(name)) byFixture.get(name).push(diagnostic); + else foreign.push(diagnostic); + } + }); + + test('the module and its real build options compile clean', () => { + // A diagnostic outside the fixture directory means `src/` itself is broken, + // or the harness picked up the wrong options. Either way the negative cases + // below would be passing for the wrong reason. + assert.deepEqual(foreign.map((d) => d.code), [], + 'no diagnostic may originate outside tests/fixtures/brand-typing/'); + }); + + test('the correct composition compiles — the positive control', () => { + // This is what makes every "must not compile" case non-vacuous: it proves + // the fixture imports resolve and the option set is usable, so a diagnostic + // in a bad-* fixture is the brand rejecting rather than a broken harness. + assert.deepEqual(byFixture.get('ok-correct-composition.cts').map((d) => d.code), []); + }); + + for (const { fixture, expected } of CASES.filter((c) => c.expected !== null)) { + test(`${fixture} is a compile error on its ${OFFENDING} marker`, () => { + const diagnostics = byFixture.get(fixture); + assert.equal(diagnostics.length, 1, + `${fixture} must produce exactly one diagnostic — see the fixture README`); + assert.equal(diagnostics[0].code, expected); + + // The diagnostic must land on the marker. Without this a fixture that + // stopped exercising its brand violation, but gained an unrelated error + // of the same code, would still pass. + const spans = markerSpans(sourceFileOf.get(fixture)); + assert.ok(spans.length > 0, `${fixture} must declare a ${OFFENDING} marker`); + const start = diagnostics[0].start; + assert.ok( + spans.some(([from, to]) => start >= from && start < to), + `${fixture}: diagnostic at offset ${start} is not on the ${OFFENDING} marker ` + + `(marker spans: ${JSON.stringify(spans)}) — the fixture is failing for the wrong reason`, + ); + }); + } +});