* fix(#3850): surface gaps_found VERIFICATION files in audit-uat cmdAuditUat admits `human_needed` OR `gaps_found`, but parseVerificationItems had a body only for the first and returned an empty array for the second — standing on a comment deferring to `plan-phase --gaps`, a different command audit-uat never reaches. Since cmdAuditUat pushes a file into `results` only when `items.length > 0`, a `gaps_found` report did not under-report: it vanished, taking its phase's `by_phase` row with it, so a clean-looking total gave the reader no cue anything was skipped. Eligibility now has one owner (the caller) and parseVerificationItems reports what the file says. The closed-entry filter could not be built on extractFrontmatter: its array-item parser keeps only each `- ` entry's FIRST line and has no notion of nested key/value objects, so an entry's `status:`/ `resolution:` siblings never reach its output and a closed entry is indistinguishable from an open one downstream. Rather than grow a competing object-list parser — or change extractFrontmatter, whose blast radius is every frontmatter consumer in the repo — this reads the raw segment BEFORE the flattening, via the existing anchored sliceTopLevelFrontmatterSegments, and hands it to the `## Gaps` machinery that already parses exactly this `- `-opened, indentation- continued shape. The human_needed path is byte-for-byte unchanged: same reader, same display names, same numbering, no resolved-entry filtering — pinned by a test and verified by identical CLI output on base and head. parseGapsItems keeps its narrower `status: resolved` rule so no *-UAT.md behaviour moves. Closes #3850 * chore(#3850): backfill changeset pr number for #3879 * fix(#3850): one parse per entry, one fence parser, one resolved-entry rule Adversarial review on #3879: B1, B2, M3, m5, m8 and n9. B1 — `sliceFrontmatterArrayEntries` hand-rolled a second frontmatter fence regex, which re-asserted the byte-0 rule #2977 removed: a BOM'd file (PowerShell 5.1 `>`/`Out-File` writes one by default) sliced nothing, so a `gaps_found` report vanished from the audit exactly as it did before this fix — this issue's own symptom, on a platform the repo already has a named defect class for. `extractFrontmatter`'s BOM+fence logic is now factored out as `frontmatterRegion` and shared. One fence parser, not two. B2 — the resolved-entry skip paired two DIFFERENT parsers by array index: `parseYamlRegion` is indent-blind, `splitGapsEntries` is indent-anchored. A block sequence written at its key's indent — ordinary, legal YAML — makes them disagree about entry count, and from the first disagreement every index names a different entry, so an OPEN entry inherits a CLOSED one's resolution and is silently dropped. That is the defect this PR exists to fix, reintroduced inside the fix. Display name and sibling fields now come from ONE parse of the raw slice; `frontmatterEntryDisplayName` applies `parseQuotedScalar` exactly as `parseYamlRegion` does, so the string is byte-identical to what `extractFrontmatter` produced. The flattened array remains the #2286 GATE, but is no longer the source of items. `sliceFrontmatterArrayEntries` also takes the LAST duplicate key, matching `parseYamlRegion`'s last-wins assignment. M3 — `frontmatterEntryToUatItem` is the single entry->UatItem mapper both readers use, rather than two copies differing only in `result`. m8 — closed entries are skipped on BOTH statuses. The earlier asymmetry cited an acceptance criterion #3850 does not contain: the issue has no AC section, and its suggested fix (2) states the skip unconditionally, naming a file with 14 of 16 entries resolved. That file is `human_needed`, so the asymmetry left the reporter's own scenario over-reporting by 14. m5 — `sliceTopLevelFrontmatterSegments`' contract doc names both consumers and says the column-0 boundary rule is now a cross-module contract. n9 — the vestigial bare block is gone and its body de-indented. Tests: the B1 BOM case, B2's nested-sequence and bare-bullet repros, a CRLF fixture (M4 — it survived by accident, now pinned) and the unified skip rule. Fail-first verified by running the new tests against the pre-fix build: the BOM, nested-sequence and unified-skip cases are red there. * fix(#3850): read the entries as objects, not as re-parsed display text Rebased onto `next`, which changed the ground this fix stood on. ADR-3473 §8.1 (#3881) replaced the hand-rolled frontmatter scanner with the vendored js-yaml: `parseQuotedScalar` and `parseYamlRegion` no longer exist, and an object entry now flattens to `test: A, resolution: R` rather than to its first line. The original mechanism existed ONLY to work around that lossy first-line flattening — it sliced the raw frontmatter segment and re-parsed each entry by hand so a `resolution:` sibling was visible at all. With a real parser upstream that workaround is obsolete, so it is deleted rather than repaired: `sliceFrontmatterArrayEntries`, `frontmatterEntryDisplayName`, the `splitGapsEntries`/`extractGapEntryFields` reuse and the second fence regex are all gone. `frontmatter.cts` instead exposes `frontmatterObjectListEntries(content, key)` — the same parse `extractFrontmatter` runs (same BOM strip, same byte-0 fence, same anchor/alias and sentinel guards, same ambiguous-colon repair), stopping one step before the display flattening. `flattenObjectListItem` is exposed alongside it so a caller deriving a display name produces the byte-identical string `extractFrontmatter` would have. That collapses the review's blockers into properties of the parse rather than things this fix has to get right: - B1 (BOM) — shares `extractFrontmatter`'s strip; verified through the CLI. - B2 (index pairing) — there is no second reader. Display name and sibling fields come from one object. - M3 (duplicate mapper) — one `frontmatterEntryToUatItem` for both readers. - M4 (CRLF) — js-yaml's, not ours; verified through the CLI. Also confirmed on the rebased base, per review: #3850 still reproduces on `next` after #3707 landed (`total_files: 0`, `total_items: 0` on a `gaps_found` fixture), so this PR is still doing work #3707 did not do. Nothing was dropped as redundant. One behaviour note: `entryField` returns a present value verbatim and treats only whitespace-only as absent. Trimming would rewrite an author's `truth:` on its way to becoming the display name. * fix(#3850): keep every frontmatter list entry at its own row Review round 3's Blocker. `frontmatterObjectListEntries` filtered its result to objects, and filtering COMPACTS: `parseHumanVerificationItems` then numbered the survivors by their position in the compacted array. On a list mixing object and non-object entries the non-object rows disappeared outright and the rest were renumbered — #3850's own vanishing-row defect, reached through entry SHAPE instead of file STATUS. Base never had it: it walked the display array, so every row surfaced at its own position. Renamed to `frontmatterListEntries` and it no longer filters (the name now matches what it returns). Deciding what a non-object entry MEANS is a caller's judgement; dropping it is nobody's. Both readers now walk the DISPLAY array — one element per row, the array #2286 already gates on — and consult the parsed array only for "does this entry carry a closure field?". `parsedEntriesFor` owns that pairing and checks the two lengths agree before trusting an index; all-null is the correct degradation, since over-reporting a closed row is recoverable and closing the wrong one is not. Names stay byte-identical to base for every entry shape, including a nested sequence (`[nested]`, not `["nested"]`). Same class closed in the gaps reader: a non-object `gaps:` entry surfaced nothing at all and now surfaces as `unknown`, which is this module's documented fail-safe direction (`parseGapsItems`) on a false-negative bug. Also restores the shared fence parser round 2 accepted. The ADR-3473 rebase dropped `frontmatterRegion` and left the BOM strip and byte-0 fence rule inlined twice; `extractFrontmatter` now routes through it, so "one fence parser" is enforced rather than asserted in a comment. Minors: `frontmatterEntryToUatItem`'s dead `forcedResult` option deleted and its "shared by both readers" comment corrected — it has one call site, and the two readers differ deliberately, each mirroring its own established sibling (`parseGapsItems` vs #2286). Documented at the divergence. Tests: `B2` asserted a name substring, so it passed while the row was mis-numbered and would have passed through outright loss; it now asserts positions and count. B2b pins the reviewer's 6-entry mixed fixture verbatim, B2c the survivors' file positions across skipped rows, B2d the gaps reader. All four fail-first against the reviewed head; 332/332 green with the fix. * fix(#3850): make status authoritative, and let the two gaps readers agree Round 4 review, all five findings. Major. `isFrontmatterEntryResolved` treated a non-empty `resolution:` as closure regardless of `status:`, so `status: failed` + `resolution: "attempted retry, still failing"` vanished from the report — the silently-vanishing-item defect #3850 exists to close, reached by field combination instead of file status. Closure is now per key, because the two keys have different conventions and one rule cannot serve both: `gaps:` `status: resolved` only, byte-identical to the rule `parseGapsItems` applies to a `## Gaps` markdown section, so one authored entry cannot read closed in one reader and open in the other. `human_verification:` a bare `resolution:` still closes, since that is how verifier-written entries record it — but a readable `status:` that contradicts it wins. A single unified rule was the first draft and is wrong: it closes a frontmatter `gaps:` entry carrying `resolution:` and no `status:`, which `parseGapsItems` surfaces, and `parseVerificationGapsItems`' own docstring claims it mirrors that reader's fail-safe status handling. The contradiction guard is not a judgment call about YAML. It is the rule this codebase already applies to the same field pair: `validateResolution` (probe-core.cts) rejects a populated `resolution:` on a non-resolved status outright — "a populated payload is an authoring mistake ... Reject it so the mistake surfaces." A reporter cannot throw, so it surfaces the item. Minor 1. Direct unit tests for `frontmatterListEntries` and `flattenObjectListItem` in `tests/frontmatter.unit.test.cjs`, the file that historically co-changes with `frontmatter.cts`. They were reachable only through `uat.cts`' readers before. Minor 2. `parsedEntriesFor`'s degrade-to-all-null branch is asserted directly. Verified unreachable through content rather than assumed: both readers enter through `frontmatterRegion`, `extractFrontmatter`'s only extra argument gates a warning, and `normalizeParsedValue`'s `value.map` is 1:1. It is a drift alarm for a future edit to either parser, so the helper is exported for tests rather than left as the one unpinned branch. Minor 3. The vestigial `const skipResolved = true` and its dead conditional are gone. Minor 4. `frontmatterEntryToUatItem` no longer reads `test:`. A `gaps:` entry has no `test:` in its vocabulary — the template's entries carry truth/status/reason/artifacts/missing — so it was speculative support for a field the shape does not have, and it collided with the 1..N row numbers `parseHumanVerificationItems` assigns by array position. Not reading it makes the collision impossible; an offset would have rewritten an authored value, against `entryField`'s verbatim contract. Docs, changeset and the dispatcher docstring all stated the unconditional rule and are corrected — three prior rounds here were comment/code drift. Fail-first proven: restoring the universal rule reddens all three new unit tests and both rewritten properties. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H3eK225hgcnEDZsnmtaP1U --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
This commit is contained in:
Adnan
11
.changeset/olive-moons-listen.md
Normal file
11
.changeset/olive-moons-listen.md
Normal file
@@ -0,0 +1,11 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 3879
|
||||
---
|
||||
**`/gsd-audit-uat` now surfaces a `gaps_found` verification report's frontmatter debt instead of dropping the phase entirely** — a `*-VERIFICATION.md` whose status is `gaps_found` reported zero items, so the file never entered the results and its phase disappeared from the report. `cmdAuditUat` admitted both non-passing statuses, then `parseVerificationItems` honoured only `human_needed` and returned an empty array for the other, standing on a comment that deferred to `plan-phase --gaps` — a different command the audit never reaches.
|
||||
|
||||
Entries already closed are skipped on **both** statuses, so a `human_needed` file whose entries are mostly resolved no longer over-reports either. Closure is read from the parsed fields, so a `truth:` whose text merely mentions "resolution:" is not mistaken for a closed entry.
|
||||
|
||||
What counts as closed follows the key. A `gaps:` entry closes on `status: resolved` and nothing else, matching the rule the `## Gaps` markdown reader already applies, so the same authored entry cannot read closed in one reader and open in the other. A `human_verification:` entry also closes on a bare `resolution:` field, because verifier-written entries record closure that way — but only where no `status:` contradicts it. An entry reading `status: failed` alongside a `resolution:` note is reported, not dropped.
|
||||
|
||||
Scope, stated precisely: this covers gaps recorded in a report's **frontmatter**. A report authored to the template's `## Gaps Summary` prose shape (`gsd-core/templates/verification-report.md`) records its gaps in the body, and those are still not counted. (#3850)
|
||||
@@ -1260,7 +1260,9 @@ After Level 3 wiring verification passes, spot-check individual exports for actu
|
||||
**Components:**
|
||||
|
||||
**1. Cross-Phase Health Check** (progress.md Step 1.6)
|
||||
Every `/gsd-progress` call scans ALL phases in the current milestone for outstanding items (pending, skipped, blocked, human_needed). Displays a non-blocking warning section with actionable links.
|
||||
Every `/gsd-progress` call scans ALL phases in the current milestone for outstanding items (pending, skipped, blocked, human_needed, gaps_found). Displays a non-blocking warning section with actionable links.
|
||||
|
||||
A verification report counts as outstanding under EITHER terminal non-passing status: `human_needed` contributes its `human_verification:` entries, and `gaps_found` contributes both its `human_verification:` and its `gaps:` entries, excluding any already closed. What counts as closed is per key: a `gaps:` entry closes on `status: resolved` and nothing else — the same rule the `## Gaps` markdown reader applies, so one authored entry cannot read closed in one reader and open in the other — while a `human_verification:` entry also closes on a bare `resolution:` field, provided no `status:` contradicts it (#3850).
|
||||
|
||||
**2. `status: partial`** (verify-work.md, UAT.md)
|
||||
New UAT status that distinguishes between "session ended" and "all tests resolved". Prevents `status: complete` when tests are still pending, blocked, or skipped without reason.
|
||||
|
||||
@@ -11,7 +11,9 @@ group: Infrastructure Features
|
||||
**Components:**
|
||||
|
||||
**1. Cross-Phase Health Check** (progress.md Step 1.6)
|
||||
Every `/gsd-progress` call scans ALL phases in the current milestone for outstanding items (pending, skipped, blocked, human_needed). Displays a non-blocking warning section with actionable links.
|
||||
Every `/gsd-progress` call scans ALL phases in the current milestone for outstanding items (pending, skipped, blocked, human_needed, gaps_found). Displays a non-blocking warning section with actionable links.
|
||||
|
||||
A verification report counts as outstanding under EITHER terminal non-passing status: `human_needed` contributes its `human_verification:` entries, and `gaps_found` contributes both its `human_verification:` and its `gaps:` entries, excluding any already closed. What counts as closed is per key: a `gaps:` entry closes on `status: resolved` and nothing else — the same rule the `## Gaps` markdown reader applies, so one authored entry cannot read closed in one reader and open in the other — while a `human_verification:` entry also closes on a bare `resolution:` field, provided no `status:` contradicts it (#3850).
|
||||
|
||||
**2. `status: partial`** (verify-work.md, UAT.md)
|
||||
New UAT status that distinguishes between "session ended" and "all tests resolved". Prevents `status: complete` when tests are still pending, blocked, or skipped without reason.
|
||||
|
||||
@@ -665,47 +665,138 @@ function countTopLevelKeyShapedLines(region: string): number {
|
||||
* key its deduplication. Optional because this function has 50-odd call sites and several
|
||||
* hold only an in-memory string; those dedup on a content digest instead.
|
||||
*/
|
||||
function extractFrontmatter(content: string, sourcePath?: string): Frontmatter {
|
||||
/**
|
||||
* The frontmatter REGION of a document: the text between the opening `---`
|
||||
* fence at byte 0 and its closing fence.
|
||||
*
|
||||
* One fence parser, not two (#3850 review round 2, B1). `extractFrontmatter`
|
||||
* and `frontmatterListEntries` need the identical answer to "where does the
|
||||
* frontmatter start and stop" — same BOM tolerance (#2977), same byte-0-only
|
||||
* fence rule, same CR handling before the closing fence — and differ only in
|
||||
* what they do with the region text afterwards. A second copy of this logic is
|
||||
* the `DEFECT.GENERATIVE-FIX` shape: it silently stops agreeing the first time
|
||||
* either is taught something.
|
||||
*
|
||||
* `terminated: false` is the fence-opened-but-never-closed case. It is not an
|
||||
* error here because the two callers disagree about it: `extractFrontmatter`
|
||||
* runs the #1882 truncation probe over the region and may warn, while
|
||||
* `frontmatterListEntries` has no array to return and gives up. So the shape
|
||||
* is reported and the decision is left to them.
|
||||
*
|
||||
* `content` is returned alongside because it is the BOM-STRIPPED text, and a
|
||||
* caller reporting on the document (the truncation diagnostic) must describe
|
||||
* the same bytes the offsets were computed against.
|
||||
*/
|
||||
function frontmatterRegion(
|
||||
content: string,
|
||||
): { region: string; terminated: boolean; content: string } | null {
|
||||
// #2977: tolerate a single leading UTF-8 BOM (U+FEFF), which Windows tooling
|
||||
// (PowerShell `>`/`Out-File` on PS 5.1, several editors) writes by default. Without this
|
||||
// strip, the byte-0 `startsWith('---')` fence check below fails on the BOM and the whole
|
||||
// parse collapses to {} — every frontmatter field silently disappears, and the engine
|
||||
// proceeds as though the file had no frontmatter at all. The BOM is a single codepoint;
|
||||
// stripping it here restores byte-0 alignment so the rest of the function is unchanged.
|
||||
// Scope: BOM only. Arbitrary non-BOM content before the fence (leading whitespace/blank
|
||||
// line/comment) is a separate product-intent decision (tolerate vs diagnose) left to a
|
||||
// future change — this fix does not broaden the byte-0 fence rule beyond the BOM.
|
||||
if (content.charCodeAt(0) === 0xFEFF) {
|
||||
content = content.slice(1);
|
||||
}
|
||||
// Match frontmatter only at byte 0 — a `---` block later in the document
|
||||
// body (YAML examples, horizontal rules) must never be treated as frontmatter.
|
||||
// (PowerShell `>`/`Out-File` on PS 5.1, several editors) writes by default.
|
||||
// Without this strip, the byte-0 `startsWith('---')` fence check below fails
|
||||
// on the BOM and the whole parse collapses — every frontmatter field silently
|
||||
// disappears, and the engine proceeds as though the file had no frontmatter
|
||||
// at all. The BOM is a single codepoint; stripping it here restores byte-0
|
||||
// alignment. Scope: BOM only. Arbitrary non-BOM content before the fence
|
||||
// (leading whitespace/blank line/comment) is a separate product-intent
|
||||
// decision (tolerate vs diagnose) left to a future change.
|
||||
if (content.charCodeAt(0) === 0xFEFF) content = content.slice(1);
|
||||
// Match frontmatter only at byte 0 — a `---` block later in the document body
|
||||
// (YAML examples, horizontal rules) must never be treated as frontmatter.
|
||||
const headerEnd = content.startsWith('---\r\n') ? 5 : content.startsWith('---\n') ? 4 : -1;
|
||||
if (headerEnd === -1) return {};
|
||||
if (headerEnd === -1) return null;
|
||||
|
||||
const closingLineStart = content.indexOf('\n---', headerEnd);
|
||||
if (closingLineStart === -1) {
|
||||
const region = content.slice(headerEnd);
|
||||
const keyCount = countKeysBeforeTruncation(region);
|
||||
if (keyCount >= UNTERMINATED_KEY_THRESHOLD && isFrontmatterShaped(region)) {
|
||||
return { region: content.slice(headerEnd), terminated: false, content };
|
||||
}
|
||||
const yamlEnd = content[closingLineStart - 1] === '\r' ? closingLineStart - 1 : closingLineStart;
|
||||
return { region: content.slice(headerEnd, yamlEnd), terminated: true, content };
|
||||
}
|
||||
|
||||
function extractFrontmatter(content: string, sourcePath?: string): Frontmatter {
|
||||
// Fence location (BOM strip, byte-0 rule, CR handling) lives in
|
||||
// `frontmatterRegion` so this and `frontmatterListEntries` cannot drift
|
||||
// apart on where the frontmatter is.
|
||||
const found = frontmatterRegion(content);
|
||||
if (!found) return {};
|
||||
|
||||
if (!found.terminated) {
|
||||
const keyCount = countKeysBeforeTruncation(found.region);
|
||||
if (keyCount >= UNTERMINATED_KEY_THRESHOLD && isFrontmatterShaped(found.region)) {
|
||||
warnUnusableInput({
|
||||
reason: UNUSABLE_REASON.FRONTMATTER_UNTERMINATED,
|
||||
source: sourcePath,
|
||||
content,
|
||||
content: found.content,
|
||||
});
|
||||
}
|
||||
return {};
|
||||
}
|
||||
|
||||
const yamlEnd = content[closingLineStart - 1] === '\r' ? closingLineStart - 1 : closingLineStart;
|
||||
const region = content.slice(headerEnd, yamlEnd);
|
||||
try {
|
||||
return parseGuardedYamlRegion(region);
|
||||
return parseGuardedYamlRegion(found.region);
|
||||
} catch {
|
||||
return unparseableResult();
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The entries of a top-level frontmatter ARRAY key, VERBATIM — as the values
|
||||
* YAML actually describes rather than the display-flattened strings
|
||||
* `extractFrontmatter` returns (#3850).
|
||||
*
|
||||
* `extractFrontmatter` renders each object entry for HUMANS —
|
||||
* `normalizeParsedValue` maps `{test, resolution}` to `"test: …, resolution: …"`
|
||||
* via `flattenObjectListItem`. That is the right contract for a reader printing
|
||||
* a list, and the wrong one for a reader that needs to branch on a specific
|
||||
* field: `status`, `resolution` and `reason` are recoverable from that string
|
||||
* only by re-parsing prose, which cannot distinguish a real `resolution:` field
|
||||
* from the same text inside a quoted `truth:`.
|
||||
*
|
||||
* This returns the same entries BEFORE that display step, off the same
|
||||
* `extractFrontmatter` parse path — same BOM strip (#2977), same byte-0 fence
|
||||
* rule (shared via `frontmatterRegion`), same anchor/alias and sentinel guards,
|
||||
* same ambiguous-colon repair. It is deliberately NOT a second parser: a
|
||||
* hand-rolled fence regex or entry slicer re-loses whatever the real one
|
||||
* learned, which is the `DEFECT.GENERATIVE-FIX` shape.
|
||||
*
|
||||
* EVERY element is returned, at its own index, whatever its type (#3850 review
|
||||
* round 3, Blocker). An earlier revision filtered to objects, which compacted
|
||||
* the array: a caller numbering entries by array position then numbered the
|
||||
* SURVIVORS, so a list mixing object and non-object entries lost rows outright
|
||||
* and mis-numbered the rest — the same silently-vanishing-row defect this whole
|
||||
* issue exists to close, triggered by entry shape instead of file status.
|
||||
* Deciding what a non-object entry MEANS is a caller's judgement (the two
|
||||
* readers in `uat.cts` answer it differently and both are right for their own
|
||||
* vocabulary); dropping it is nobody's.
|
||||
*
|
||||
* Returns `null` when the document has no frontmatter, the frontmatter is
|
||||
* unterminated or unparseable, the key is absent, or the key is not an array —
|
||||
* "nothing to iterate" cases the caller should not have to tell apart.
|
||||
*/
|
||||
function frontmatterListEntries(content: string, key: string): unknown[] | null {
|
||||
const found = frontmatterRegion(content);
|
||||
if (!found || !found.terminated) return null;
|
||||
|
||||
let raw: unknown;
|
||||
try {
|
||||
refuseAnchorsAndAliases(found.region);
|
||||
refuseIfSentinelPresent(found.region);
|
||||
raw = restoreNullBytesDeep(loadWithAmbiguousColonRepair(escapeNullBytesForParse(found.region)));
|
||||
} catch {
|
||||
// Same posture as `extractFrontmatter`: an unparseable region is "no
|
||||
// frontmatter", never a throw into a caller that was only reading a field.
|
||||
return null;
|
||||
}
|
||||
if (!raw || typeof raw !== 'object' || Array.isArray(raw)) return null;
|
||||
const value = (raw as Record<string, unknown>)[key];
|
||||
// `Array.isArray` narrows an `unknown` to `any[]`, and returning that
|
||||
// unchecked is how `any` escapes a guarded parser into every caller. The
|
||||
// element type genuinely IS unknown here — that is the point of this
|
||||
// function — so say so.
|
||||
if (!Array.isArray(value)) return null;
|
||||
return value as unknown[];
|
||||
}
|
||||
|
||||
/**
|
||||
* Escape a string for emission inside a YAML double-quoted scalar (#1779). ADR-3473 §8.1
|
||||
* (#3881): routed through the vendored js-yaml's `dump()` (forced double-quoted style) rather
|
||||
@@ -1468,6 +1559,13 @@ export = {
|
||||
stripFrontmatter,
|
||||
noOpObjectListSetError,
|
||||
parseMustHavesBlock,
|
||||
// #3850: an array key's entries as parsed OBJECTS, for a caller that must
|
||||
// branch on an entry's `status:`/`resolution:` rather than print it. Off the
|
||||
// same parse path as `extractFrontmatter`, minus only the display flattening.
|
||||
frontmatterListEntries,
|
||||
// #3850: the display rendering itself, so a caller deriving a name from those
|
||||
// objects produces the byte-identical string `extractFrontmatter` would have.
|
||||
flattenObjectListItem,
|
||||
FRONTMATTER_SCHEMAS,
|
||||
cmdFrontmatterGet,
|
||||
cmdFrontmatterSet,
|
||||
|
||||
325
src/uat.cts
325
src/uat.cts
@@ -28,7 +28,7 @@ import planningWorkspace = require('./planning-workspace.cjs');
|
||||
const { planningDir } = planningWorkspace;
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import frontmatter = require('./frontmatter.cjs');
|
||||
const { extractFrontmatter } = frontmatter;
|
||||
const { extractFrontmatter, frontmatterListEntries, flattenObjectListItem } = frontmatter;
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import phaseIdMod = require('./phase-id.cjs');
|
||||
const { PHASE_NUMBER_TOKEN_SOURCE, scopeToPhase } = phaseIdMod;
|
||||
@@ -3450,9 +3450,279 @@ function rawGapEntryText(
|
||||
|
||||
// ─── parseVerificationItems ───────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* The entry's `status:`, lowercased, or undefined when absent/blank/non-scalar.
|
||||
*
|
||||
* The entry is a PARSED OBJECT, so this reads a named field rather than
|
||||
* matching prose. That distinction is the whole point: against the
|
||||
* display-flattened string, a `truth:` whose text mentions "status: resolved"
|
||||
* is indistinguishable from an entry that carries the field.
|
||||
*/
|
||||
function frontmatterEntryStatus(entry: Record<string, unknown>): string | undefined {
|
||||
const status = entry['status'];
|
||||
if (typeof status !== 'string' || status.trim() === '') return undefined;
|
||||
return status.trim().toLowerCase();
|
||||
}
|
||||
|
||||
/**
|
||||
* Is this `gaps:` frontmatter entry already closed? (#3850)
|
||||
*
|
||||
* `status: resolved`, and nothing else. Byte-identical to the rule
|
||||
* `parseGapsItems` applies to a `## Gaps` markdown section, deliberately: the
|
||||
* two readers see the SAME authored vocabulary in two places, and a closure
|
||||
* rule that differed between them would let one entry read closed in one
|
||||
* reader and open in the other. `parseVerificationGapsItems`' docstring claims
|
||||
* it mirrors `parseGapsItems`' fail-safe status handling; this is the line
|
||||
* that makes that claim true rather than approximately true.
|
||||
*
|
||||
* So a `gaps:` entry carrying `resolution:` and no `status:` SURFACES, via the
|
||||
* same 'unknown'-status fallback `parseGapsItems` already gives it (#3879
|
||||
* review round 4, Major).
|
||||
*/
|
||||
function isGapsEntryResolved(entry: Record<string, unknown> | undefined): boolean {
|
||||
if (!entry) return false;
|
||||
return frontmatterEntryStatus(entry) === 'resolved';
|
||||
}
|
||||
|
||||
/**
|
||||
* Is this `human_verification:` frontmatter entry already closed? (#3850)
|
||||
*
|
||||
* Verifier-written entries record closure as a `resolution:` field with no
|
||||
* `status:` at all, so `resolution:` closes — but ONLY when no `status:`
|
||||
* contradicts it. `status:` is authoritative wherever it is readable.
|
||||
*
|
||||
* The contradiction guard is the #3879 round-4 Major fix. Without it,
|
||||
* `status: failed` + `resolution: "attempted retry, still failing"` — a
|
||||
* plausible informational note, not a closure assertion — is silently dropped
|
||||
* from the report, which is the exact silently-vanishing-item defect class
|
||||
* #3850 exists to close, reached by field COMBINATION instead of file STATUS.
|
||||
*
|
||||
* This is not a judgment call about YAML: it is the rule this codebase already
|
||||
* applies to the same field pair one module over. `validateResolution`
|
||||
* (`probe-core.cts`) rejects a populated `resolution:` on a non-resolved status
|
||||
* outright — "a populated payload is an authoring mistake (the author meant
|
||||
* resolved/dismissed) that would otherwise be silently dropped into the
|
||||
* unresolved count with no error pointing at it. Reject it so the mistake
|
||||
* surfaces." A reporter cannot throw, so the fail-safe equivalent of surfacing
|
||||
* the mistake is to surface the ITEM.
|
||||
*
|
||||
* #3850's suggested fix (2) states the skip unconditionally — "Skip entries
|
||||
* carrying a `resolution:` field" — and its named scenario (one file with 14 of
|
||||
* 16 entries resolved) is unaffected by the guard: those entries close either
|
||||
* on `resolution:` with no contradicting status, or on `status: resolved`.
|
||||
* Both still skip. The guard only changes entries whose own two fields
|
||||
* disagree, and for those the fail-safe direction on a false-NEGATIVE bug is to
|
||||
* report, not to drop.
|
||||
*/
|
||||
function isHumanVerificationEntryResolved(entry: Record<string, unknown> | undefined): boolean {
|
||||
if (!entry) return false;
|
||||
const status = frontmatterEntryStatus(entry);
|
||||
if (status !== undefined) return status === 'resolved';
|
||||
const resolution = entry['resolution'];
|
||||
return typeof resolution === 'string' && resolution.trim() !== '';
|
||||
}
|
||||
|
||||
/**
|
||||
* A named string field of a parsed entry, or undefined when absent/non-scalar.
|
||||
*
|
||||
* A whitespace-only value counts as absent, but a present value is returned
|
||||
* VERBATIM — trimming it here would silently rewrite an author's `truth:` on
|
||||
* its way to becoming the item's display name, which is a different string from
|
||||
* the one in the file.
|
||||
*/
|
||||
function isFrontmatterObjectEntry(entry: unknown): entry is Record<string, unknown> {
|
||||
return !!entry && typeof entry === 'object' && !Array.isArray(entry);
|
||||
}
|
||||
|
||||
/**
|
||||
* The PARSED object behind each element of a frontmatter array, positionally
|
||||
* aligned with that array's DISPLAY renderings — `null` at any index whose
|
||||
* entry is not an object (#3850).
|
||||
*
|
||||
* Both frontmatter readers below need the same two things about one array: the
|
||||
* string each entry has always displayed as, and the fields it actually
|
||||
* carries. `extractFrontmatter` gives the first, `frontmatterListEntries` the
|
||||
* second, and the ONLY safe way to use them together is by index — so the
|
||||
* pairing is done once, here, rather than open-coded twice.
|
||||
*
|
||||
* Alignment is checked, not assumed. Both arrays come from one parse of one
|
||||
* region (they share a fence parser), so they agree in practice; if they ever
|
||||
* did not, an index would name a DIFFERENT entry's fields and the resolved-skip
|
||||
* would close the wrong row. All-`null` is the correct degradation: no entry is
|
||||
* skipped as closed, which over-reports rather than mis-attributes.
|
||||
*
|
||||
* The length check is UNREACHABLE through content today and is kept anyway
|
||||
* (#3879 review round 4, Minor 2). It was verified unreachable rather than
|
||||
* assumed: both readers enter through `frontmatterRegion`, `extractFrontmatter`'s
|
||||
* only extra argument (`sourcePath`) gates a warning and nothing else, and the
|
||||
* display step — `normalizeParsedValue`'s `value.map(...)` — is 1:1 and drops no
|
||||
* element. So the guard is a drift alarm for a future edit to either parser, not
|
||||
* a live branch. That makes it untestable through the two readers, which is why
|
||||
* this helper is exported for tests: the degradation is asserted against the
|
||||
* function directly rather than left as the one unpinned branch in the family.
|
||||
*/
|
||||
function parsedEntriesFor(
|
||||
content: string,
|
||||
key: string,
|
||||
flattened: unknown[],
|
||||
): Array<Record<string, unknown> | null> {
|
||||
const parsed = frontmatterListEntries(content, key);
|
||||
if (!parsed || parsed.length !== flattened.length) return flattened.map(() => null);
|
||||
return parsed.map((entry) => (isFrontmatterObjectEntry(entry) ? entry : null));
|
||||
}
|
||||
|
||||
function entryField(entry: Record<string, unknown>, key: string): string | undefined {
|
||||
const v = entry[key];
|
||||
if (typeof v === 'string') return v.trim() === '' ? undefined : v;
|
||||
if (typeof v === 'number' || typeof v === 'boolean') return String(v);
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* One parsed `gaps:` entry -> one `UatItem`.
|
||||
*
|
||||
* ONE call site, `parseVerificationGapsItems` (#3850 review round 3, Minor 1 —
|
||||
* an earlier revision's comment claimed both frontmatter readers shared this,
|
||||
* and a dead `forcedResult` option existed to serve the second one; neither was
|
||||
* ever true, and the claim made a deliberate difference read as an accident).
|
||||
*
|
||||
* WHY the two frontmatter readers derive fields differently, since they sit
|
||||
* side by side and it is a fair question: each mirrors its OWN established
|
||||
* sibling rather than each other.
|
||||
*
|
||||
* - This one mirrors `parseGapsItems`, the `## Gaps` markdown reader, field
|
||||
* for field: `status:` supplies `result` with the module's documented
|
||||
* fail-safe `'unknown'` when absent (surface a questionable entry rather
|
||||
* than drop a real one), `test` is taken ONLY when the entry declares one,
|
||||
* and `reason` passes through. A `gaps:` entry carries its own status, so
|
||||
* inventing one would be a lie.
|
||||
* - `parseHumanVerificationItems` mirrors #2286's `human_verification:`
|
||||
* behaviour: the array IS the outstanding list, so every surviving entry is
|
||||
* `human_needed` by construction and its `test` is its ROW, because those
|
||||
* entries carry no number of their own.
|
||||
*
|
||||
* Converging them would mean changing one of those two established contracts
|
||||
* for the convenience of symmetry. See `parseVerificationItems` for the one
|
||||
* consequence that is genuinely open (a `test` number is unique per array, not
|
||||
* per report).
|
||||
*
|
||||
* The display name falls back to `flattenObjectListItem` — the SAME renderer
|
||||
* `extractFrontmatter` applies — so an entry with no `truth:` reads exactly as
|
||||
* it always did, byte for byte.
|
||||
*/
|
||||
function frontmatterEntryToUatItem(entry: Record<string, unknown>): UatItem {
|
||||
const status = entryField(entry, 'status') ?? 'unknown';
|
||||
const reason = entryField(entry, 'reason');
|
||||
const item: UatItem = {
|
||||
name: entryField(entry, 'truth') || flattenObjectListItem(entry),
|
||||
result: status,
|
||||
category: categorizeItem(status, reason, undefined),
|
||||
};
|
||||
// No `test:` read (#3879 review round 4, Minor 4). A `gaps:` entry has no
|
||||
// `test:` in its vocabulary — the verification template's entries carry
|
||||
// `truth` / `status` / `reason` / `artifacts` / `missing` — so reading one was
|
||||
// speculative support for a field this shape does not have. It also collided:
|
||||
// `parseHumanVerificationItems` numbers its items 1..N by array POSITION,
|
||||
// so a `gaps:` entry that did carry `test: 1` produced two items numbered 1
|
||||
// in one file's combined list. Not reading it makes the collision impossible
|
||||
// rather than unlikely, and does not renumber anything: an offset would have
|
||||
// rewritten an authored value, which is the opposite of `entryField`'s
|
||||
// verbatim contract.
|
||||
if (reason) item.reason = reason;
|
||||
return item;
|
||||
}
|
||||
|
||||
/**
|
||||
* Surface a `gaps_found` report's frontmatter `gaps:` array (#3850).
|
||||
*
|
||||
* Mirrors `parseGapsItems`' field vocabulary and fail-safe status handling, but
|
||||
* reads the FRONTMATTER array rather than a `## Gaps` markdown section —
|
||||
* `parseGapsItems` is reached only from `parseUatItems`, and the verification
|
||||
* template puts gaps in frontmatter, so no existing reader covers this shape.
|
||||
*/
|
||||
function parseVerificationGapsItems(content: string): UatItem[] {
|
||||
const flattened = extractFrontmatter(content)['gaps'];
|
||||
if (!Array.isArray(flattened)) return [];
|
||||
const parsed = parsedEntriesFor(content, 'gaps', flattened);
|
||||
|
||||
const items: UatItem[] = [];
|
||||
flattened.forEach((display, idx) => {
|
||||
const entry = parsed[idx];
|
||||
// A non-object entry (a bare scalar, a null from a `- ` with nothing after
|
||||
// it, a nested sequence) still surfaces, named by the SAME renderer every
|
||||
// other frontmatter reader names it by. Dropping it would be this module's
|
||||
// wrong direction on a false-NEGATIVE bug: `parseGapsItems`' own
|
||||
// 'unknown'-status fallback exists to surface a questionable entry rather
|
||||
// than lose a real one, and an entry with no readable status is exactly
|
||||
// that. It carries no fields, so it can never be skipped as closed.
|
||||
if (!entry) {
|
||||
items.push({
|
||||
name: normalizeHumanVerificationEntry(display),
|
||||
result: 'unknown',
|
||||
category: categorizeItem('unknown'),
|
||||
});
|
||||
return;
|
||||
}
|
||||
if (isGapsEntryResolved(entry)) return;
|
||||
items.push(frontmatterEntryToUatItem(entry));
|
||||
});
|
||||
return items;
|
||||
}
|
||||
|
||||
/**
|
||||
* #3850: `gaps_found` is as outstanding as `human_needed`.
|
||||
*
|
||||
* `cmdAuditUat` admits BOTH statuses, then this function honoured only one and
|
||||
* returned an empty array for the other. Because `cmdAuditUat` pushes a file
|
||||
* into `results` only when `items.length > 0`, a `gaps_found` report did not
|
||||
* merely under-report — it VANISHED, taking its phase's row out of `by_phase`
|
||||
* with it, so a clean-looking total gave the reader no cue that anything was
|
||||
* skipped. The trailing `plan-phase --gaps` note that stood in for a
|
||||
* `gaps_found` branch pointed at a DIFFERENT command that `audit-uat` never
|
||||
* reaches.
|
||||
*
|
||||
* Eligibility now has ONE owner — the caller — and this function reports what
|
||||
* the file says.
|
||||
*
|
||||
* Resolved entries are skipped on BOTH statuses (#3850 review m8). An earlier
|
||||
* revision skipped them only on `gaps_found`, citing an acceptance criterion
|
||||
* that the issue does not contain: #3850 has no AC section, and its suggested
|
||||
* fix (2) states the skip unconditionally — "Skip entries carrying a
|
||||
* `resolution:` field, or the fix trades one wrong number for another — one
|
||||
* file here has 14 of 16 entries resolved". That file is `human_needed`, so the
|
||||
* asymmetry left the reporter's own named scenario over-reporting by 14. The
|
||||
* SKIP applies on both paths.
|
||||
*
|
||||
* WHAT COUNTS AS RESOLVED is per-key, not universal (#3879 review round 4,
|
||||
* Major): `isGapsEntryResolved` takes `parseGapsItems`' `status: resolved` rule
|
||||
* verbatim so the two `gaps` readers cannot disagree, and
|
||||
* `isHumanVerificationEntryResolved` honours the `resolution:`-only closure the
|
||||
* issue names, guarded so a `status:` that contradicts it wins. The issue's
|
||||
* "skip entries carrying a `resolution:` field" is quoted above as written; it
|
||||
* holds for every entry whose fields agree, which is every entry the reporter's
|
||||
* own scenario contains.
|
||||
*/
|
||||
function parseVerificationItems(content: string, status: string, sourcePath?: string): UatItem[] {
|
||||
const items: UatItem[] = [];
|
||||
if (status === 'gaps_found') {
|
||||
items.push(...parseHumanVerificationItems(content, sourcePath));
|
||||
items.push(...parseVerificationGapsItems(content));
|
||||
return items;
|
||||
}
|
||||
if (status === 'human_needed') {
|
||||
return parseHumanVerificationItems(content, sourcePath);
|
||||
}
|
||||
return items;
|
||||
}
|
||||
|
||||
/**
|
||||
* The `human_verification:` reader, extracted from `parseVerificationItems` so
|
||||
* `gaps_found` and `human_needed` share ONE implementation rather than a second
|
||||
* copy that drifts (ref `DEFECT.GENERATIVE-FIX`). Both statuses now take the
|
||||
* identical path, resolved-entry skip included — see the dispatcher above.
|
||||
*/
|
||||
function parseHumanVerificationItems(content: string, sourcePath?: string): UatItem[] {
|
||||
const items: UatItem[] = [];
|
||||
// #2286: the frontmatter's structured `human_verification:` YAML array
|
||||
// (extractFrontmatter) is the PRIMARY source of truth when present and
|
||||
// non-empty — it fully bypasses the body-shape scan below, so a file
|
||||
@@ -3462,10 +3732,49 @@ function parseVerificationItems(content: string, status: string, sourcePath?: st
|
||||
const frontmatter = extractFrontmatter(content, sourcePath);
|
||||
const humanVerification = frontmatter.human_verification;
|
||||
if (Array.isArray(humanVerification) && humanVerification.length > 0) {
|
||||
humanVerification.forEach((entry, idx) => {
|
||||
// #3850: ONE source for both the display name and the sibling fields.
|
||||
//
|
||||
// `extractFrontmatter` renders each object entry for humans
|
||||
// (`flattenObjectListItem`), which is right for printing and wrong for
|
||||
// branching: `resolution:` is recoverable from that string only by matching
|
||||
// prose, and prose cannot tell a real field from the same text quoted
|
||||
// inside `truth:`. `frontmatterListEntries` returns the same entries one
|
||||
// step earlier, off the same parse.
|
||||
//
|
||||
// The flattened array stays the #2286 GATE — a non-empty
|
||||
// `human_verification:` fully bypasses the body-shape scan below — but the
|
||||
// raw entries are the source of the items, so there is no second reader to
|
||||
// desynchronise against.
|
||||
//
|
||||
// WALK THE FLATTENED ARRAY, and use the parsed one only to answer "is this
|
||||
// entry closed?" (#3850 review round 3, Blocker).
|
||||
//
|
||||
// This is base's loop — every element, at its own index, named by the
|
||||
// renderer it has always been named by — plus one skip. It is deliberately
|
||||
// NOT "iterate the parsed entries": an earlier revision did that against an
|
||||
// object-FILTERED array, which compacted it, so a list mixing object and
|
||||
// non-object entries lost the non-object rows outright and renumbered the
|
||||
// survivors. That is the silently-vanishing row this issue exists to close,
|
||||
// reintroduced by entry SHAPE instead of file STATUS. Numbering off the
|
||||
// flattened array cannot drift from what the file says, because that array
|
||||
// is the one #2286 already gated on.
|
||||
//
|
||||
// The name therefore stays byte-identical to base for every entry shape,
|
||||
// including the ones with no object to read: a YAML null renders `''`, a
|
||||
// nested sequence renders `[nested]`. Re-deriving those from the parsed
|
||||
// value would have printed `["nested"]` — a rendering nobody asked this
|
||||
// change to alter.
|
||||
//
|
||||
// `parsedEntriesFor` owns the pairing and its alignment check.
|
||||
const parsed = parsedEntriesFor(content, 'human_verification', humanVerification);
|
||||
humanVerification.forEach((flattened, idx) => {
|
||||
const object = parsed[idx];
|
||||
if (object && isHumanVerificationEntryResolved(object)) return;
|
||||
items.push({
|
||||
// The entry's ORIGINAL 1-based position, so a surfaced item still
|
||||
// names its row in the file when a closed sibling was skipped.
|
||||
test: idx + 1,
|
||||
name: normalizeHumanVerificationEntry(entry),
|
||||
name: normalizeHumanVerificationEntry(flattened),
|
||||
result: 'human_needed',
|
||||
category: 'human_uat',
|
||||
});
|
||||
@@ -3595,8 +3904,6 @@ function parseVerificationItems(content: string, status: string, sourcePath?: st
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
// gaps_found items are already handled by plan-phase --gaps pipeline
|
||||
return items;
|
||||
}
|
||||
|
||||
@@ -3687,4 +3994,12 @@ export = {
|
||||
// `iterateBullets` actually reads.
|
||||
DEFERRED_MARKER_ALT,
|
||||
DEFERRED_BULLET_MARKERS,
|
||||
// #3850: exported so the `gaps_found` partition invariant is asserted
|
||||
// against the parser itself rather than only through a CLI round-trip
|
||||
// (RULESET.TESTS.property-based-testing).
|
||||
parseVerificationItems,
|
||||
// #3879 review round 4, Minor 2: exported for tests so the degrade-to-all-null
|
||||
// branch is asserted directly. It cannot be reached through the two readers —
|
||||
// see the alignment note on the function.
|
||||
parsedEntriesFor,
|
||||
};
|
||||
|
||||
@@ -24,6 +24,8 @@ const {
|
||||
stripFrontmatter,
|
||||
noOpObjectListSetError,
|
||||
parseMustHavesBlock,
|
||||
frontmatterListEntries,
|
||||
flattenObjectListItem,
|
||||
FRONTMATTER_SCHEMAS,
|
||||
agentScalarNeedsDoubleQuoting,
|
||||
escapeDoubleQuotedScalar,
|
||||
@@ -1857,3 +1859,100 @@ describe('#3742: propagateCommentChannel — merge, root filter, trailing dedupe
|
||||
assert.deepEqual(ch.trailing, ['# trail']);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── frontmatterListEntries (#3850) ───────────────────────────────────────────
|
||||
//
|
||||
// Direct coverage for the two exports #3850 added (#3879 review round 4, Minor
|
||||
// 1). They were previously exercised only through `uat.cts`' readers, so a
|
||||
// change in either primitive could only be caught by a test about something
|
||||
// else.
|
||||
|
||||
describe('frontmatterListEntries: returns parsed entries, not display strings', () => {
|
||||
const doc = ['---',
|
||||
'gaps:',
|
||||
' - truth: "The widget renders"',
|
||||
' status: failed',
|
||||
' reason: "only on one platform"',
|
||||
' - truth: "The other thing"',
|
||||
' status: resolved',
|
||||
'---',
|
||||
'',
|
||||
'# Body',
|
||||
''].join('\n');
|
||||
|
||||
test('object entries come back as objects with their fields intact', () => {
|
||||
const entries = frontmatterListEntries(doc, 'gaps');
|
||||
assert.equal(entries.length, 2);
|
||||
assert.equal(entries[0].status, 'failed');
|
||||
assert.equal(entries[0].truth, 'The widget renders');
|
||||
assert.equal(entries[0].reason, 'only on one platform');
|
||||
assert.equal(entries[1].status, 'resolved');
|
||||
});
|
||||
|
||||
test('a field is readable as a field, not recoverable only from prose', () => {
|
||||
// The whole reason this export exists: `extractFrontmatter` flattens the
|
||||
// same entry to a display string in which a real `resolution:` field and
|
||||
// the same text quoted inside `truth:` are indistinguishable.
|
||||
const trap = ['---',
|
||||
'gaps:',
|
||||
' - truth: "the report said resolution: done"',
|
||||
' status: failed',
|
||||
'---',
|
||||
''].join('\n');
|
||||
const [entry] = frontmatterListEntries(trap, 'gaps');
|
||||
assert.equal(entry.resolution, undefined, 'prose inside truth is not a resolution field');
|
||||
assert.equal(entry.status, 'failed');
|
||||
});
|
||||
|
||||
test('EVERY element is returned at its own index, whatever its type', () => {
|
||||
// #3850 review round 3, Blocker: filtering to objects compacted the array
|
||||
// and renumbered a caller iterating by position.
|
||||
const mixed = ['---',
|
||||
'gaps:',
|
||||
' - truth: "an object"',
|
||||
' - a bare scalar',
|
||||
' -',
|
||||
' - - nested',
|
||||
' - sequence',
|
||||
'---',
|
||||
''].join('\n');
|
||||
const entries = frontmatterListEntries(mixed, 'gaps');
|
||||
assert.equal(entries.length, 4);
|
||||
assert.equal(typeof entries[0], 'object');
|
||||
assert.equal(entries[1], 'a bare scalar');
|
||||
assert.equal(entries[2], null);
|
||||
assert.ok(Array.isArray(entries[3]));
|
||||
});
|
||||
|
||||
test('null for every "nothing to iterate" case, never a throw', () => {
|
||||
assert.equal(frontmatterListEntries('no frontmatter here', 'gaps'), null);
|
||||
assert.equal(frontmatterListEntries('---\ngaps:\n - a\n', 'gaps'), null, 'unterminated');
|
||||
assert.equal(frontmatterListEntries('---\nother: 1\n---\n', 'gaps'), null, 'key absent');
|
||||
assert.equal(frontmatterListEntries('---\ngaps: not-an-array\n---\n', 'gaps'), null, 'not an array');
|
||||
assert.equal(frontmatterListEntries('---\n : : :\n---\n', 'gaps'), null, 'unparseable');
|
||||
});
|
||||
|
||||
test('agrees index-for-index with extractFrontmatter\'s display array', () => {
|
||||
// `parsedEntriesFor` in uat.cts pairs these two by index and degrades to
|
||||
// all-null if their lengths ever disagree. Pin the agreement here, where
|
||||
// the two parsers actually live.
|
||||
const display = extractFrontmatter(doc).gaps;
|
||||
const parsed = frontmatterListEntries(doc, 'gaps');
|
||||
assert.equal(display.length, parsed.length);
|
||||
});
|
||||
});
|
||||
|
||||
describe('flattenObjectListItem (#3850)', () => {
|
||||
test('renders an object entry the way extractFrontmatter displays it', () => {
|
||||
const entry = { test: 'Do the thing', expected: 'it works' };
|
||||
const rendered = flattenObjectListItem(entry);
|
||||
const viaExtract = extractFrontmatter(['---',
|
||||
'human_verification:',
|
||||
' - test: "Do the thing"',
|
||||
' expected: "it works"',
|
||||
'---',
|
||||
''].join('\n')).human_verification[0];
|
||||
assert.equal(rendered, viaExtract,
|
||||
'a caller naming an item from the parsed object must produce the byte-identical string');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -22,6 +22,8 @@ const {
|
||||
parseUatItemsWithStats,
|
||||
DEFERRED_MARKER_ALT,
|
||||
DEFERRED_BULLET_MARKERS,
|
||||
parseVerificationItems,
|
||||
parsedEntriesFor,
|
||||
} = require('../gsd-core/bin/lib/uat.cjs');
|
||||
const { iterateBullets } = require('../gsd-core/bin/lib/markdown-sectionizer.cjs');
|
||||
|
||||
@@ -815,6 +817,128 @@ All checks passed.
|
||||
assert.strictEqual(output.summary.total_items, 0);
|
||||
assert.strictEqual(output.summary.total_files, 0);
|
||||
});
|
||||
|
||||
// ─── #3879 review round 4, Major: `status:` is authoritative ──────────────
|
||||
//
|
||||
// A `resolution:` note beside a status that is not `resolved` is an
|
||||
// authoring mistake, not a closure assertion — the rule `validateResolution`
|
||||
// (probe-core.cts) already applies to this same field pair, where it rejects
|
||||
// the combination outright rather than counting the item as closed. A
|
||||
// reporter cannot throw, so the fail-safe equivalent is to SURFACE the item.
|
||||
// Dropping it would be the silently-vanishing-item defect #3850 exists to
|
||||
// close, reached by field COMBINATION instead of file STATUS.
|
||||
|
||||
test('a human_verification entry whose status contradicts its resolution still surfaces', () => {
|
||||
const items = parseVerificationItems(`---
|
||||
status: human_needed
|
||||
human_verification:
|
||||
- test: "Retry the upload"
|
||||
status: failed
|
||||
resolution: "attempted retry, still failing"
|
||||
---
|
||||
`, 'human_needed');
|
||||
assert.strictEqual(items.length, 1);
|
||||
});
|
||||
|
||||
test('a gaps entry carrying only a resolution note surfaces — gaps closes on status alone', () => {
|
||||
// `parseGapsItems`, the `## Gaps` markdown reader, closes on
|
||||
// `status: resolved` and nothing else. This frontmatter reader takes that
|
||||
// rule verbatim so the same authored entry cannot read closed in one and
|
||||
// open in the other.
|
||||
const items = parseVerificationItems(`---
|
||||
status: gaps_found
|
||||
gaps:
|
||||
- truth: "The widget renders"
|
||||
resolution: "a note, not a closure assertion"
|
||||
---
|
||||
`, 'gaps_found');
|
||||
assert.strictEqual(items.length, 1);
|
||||
assert.strictEqual(items[0].result, 'unknown');
|
||||
});
|
||||
|
||||
test('a gaps entry whose status contradicts its resolution still surfaces', () => {
|
||||
const items = parseVerificationItems(`---
|
||||
status: gaps_found
|
||||
gaps:
|
||||
- truth: "The widget renders"
|
||||
status: failed
|
||||
resolution: "attempted retry, still failing"
|
||||
---
|
||||
`, 'gaps_found');
|
||||
assert.strictEqual(items.length, 1);
|
||||
assert.strictEqual(items[0].result, 'failed');
|
||||
});
|
||||
|
||||
// ─── #3879 review round 4, Minor 2: the alignment guard ──────────────────
|
||||
//
|
||||
// `parsedEntriesFor` pairs the display array with the parsed array BY INDEX
|
||||
// and degrades to all-null if their lengths disagree, so a mis-paired index
|
||||
// can never close the wrong row. That branch is unreachable through the two
|
||||
// readers — both parsers share `frontmatterRegion`, and the display step is
|
||||
// 1:1 — so it is asserted against the function directly. Called with the
|
||||
// real content the readers pass, plus a `flattened` array of the wrong
|
||||
// length, which is exactly the drift the guard exists to catch.
|
||||
|
||||
describe('parsedEntriesFor: index pairing and its degradation', () => {
|
||||
const doc = `---
|
||||
status: gaps_found
|
||||
gaps:
|
||||
- truth: "first"
|
||||
status: failed
|
||||
- truth: "second"
|
||||
status: resolved
|
||||
---
|
||||
`;
|
||||
|
||||
test('pairs each display entry with its own parsed object', () => {
|
||||
const paired = parsedEntriesFor(doc, 'gaps', ['first-display', 'second-display']);
|
||||
assert.equal(paired.length, 2);
|
||||
assert.equal(paired[0].truth, 'first');
|
||||
assert.equal(paired[1].truth, 'second');
|
||||
});
|
||||
|
||||
test('a length disagreement degrades to all-null — no entry is skipped as closed', () => {
|
||||
// Over-reporting is the correct degradation: a wrong index would name a
|
||||
// DIFFERENT entry's fields and close the wrong row.
|
||||
const shorter = parsedEntriesFor(doc, 'gaps', ['only-one']);
|
||||
assert.deepEqual(shorter, [null]);
|
||||
const longer = parsedEntriesFor(doc, 'gaps', ['a', 'b', 'c']);
|
||||
assert.deepEqual(longer, [null, null, null]);
|
||||
});
|
||||
|
||||
test('a non-object entry is null at its own index, not dropped', () => {
|
||||
const mixed = `---
|
||||
gaps:
|
||||
- truth: "an object"
|
||||
- a bare scalar
|
||||
---
|
||||
`;
|
||||
const paired = parsedEntriesFor(mixed, 'gaps', ['x', 'y']);
|
||||
assert.equal(paired.length, 2);
|
||||
assert.equal(paired[0].truth, 'an object');
|
||||
assert.equal(paired[1], null);
|
||||
});
|
||||
|
||||
test('an absent key degrades to all-null rather than throwing', () => {
|
||||
assert.deepEqual(parsedEntriesFor('---\nother: 1\n---\n', 'gaps', ['a', 'b']), [null, null]);
|
||||
});
|
||||
});
|
||||
|
||||
test('both closure spellings still close a human_verification entry', () => {
|
||||
const items = parseVerificationItems(`---
|
||||
status: human_needed
|
||||
human_verification:
|
||||
- test: "Answered"
|
||||
resolution: "RESOLVED"
|
||||
- test: "Also answered"
|
||||
status: resolved
|
||||
- test: "Still open"
|
||||
status: partial
|
||||
---
|
||||
`, 'human_needed');
|
||||
assert.strictEqual(items.length, 1);
|
||||
assert.strictEqual(items[0].test, 3, 'the surfaced entry keeps its original row');
|
||||
});
|
||||
});
|
||||
|
||||
// Regression: #2286 — parseVerificationItems never read the frontmatter's
|
||||
@@ -8289,3 +8413,514 @@ describe('#3781: acknowledge supports the heading-delimited entry shape', () =>
|
||||
'the pre-existing headless splice shape must be untouched');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── #3850: gaps_found VERIFICATION files ────────────────────────────────────
|
||||
//
|
||||
// cmdAuditUat admits `human_needed` OR `gaps_found`, but parseVerificationItems
|
||||
// honoured only the first and returned [] for the second. Because cmdAuditUat
|
||||
// pushes a file into `results` only when `items.length > 0`, a `gaps_found`
|
||||
// report did not under-report — the whole file, and its phase's `by_phase` row,
|
||||
// VANISHED. Every test below fails on base: the gate returns [] regardless of
|
||||
// what the frontmatter holds.
|
||||
describe('#3850 gaps_found VERIFICATION files', () => {
|
||||
let tmpDir;
|
||||
|
||||
beforeEach(() => { tmpDir = createTempProject(); });
|
||||
afterEach(() => { cleanup(tmpDir); });
|
||||
|
||||
/** Write one VERIFICATION file and return the parsed audit-uat payload. */
|
||||
const auditWith = (body, phase = '01-demo') => {
|
||||
const phaseDir = path.join(tmpDir, '.planning', 'phases', phase);
|
||||
fs.mkdirSync(phaseDir, { recursive: true });
|
||||
fs.writeFileSync(path.join(phaseDir, `${phase.split('-')[0]}-VERIFICATION.md`), body);
|
||||
const result = runGsdTools('audit-uat --raw', tmpDir);
|
||||
assert.ok(result.success, `Command failed: ${result.error}`);
|
||||
return JSON.parse(result.output);
|
||||
};
|
||||
|
||||
const HV_TWO = `---
|
||||
phase: 01-demo
|
||||
status: gaps_found
|
||||
human_verification:
|
||||
- test: "Confirm the widget renders on a physical device"
|
||||
expected: "Widget appears within 2s"
|
||||
why_human: "Needs real hardware"
|
||||
- test: "Confirm the audible alert fires"
|
||||
expected: "Alert is audible"
|
||||
why_human: "Needs a human ear"
|
||||
---
|
||||
|
||||
# Verification
|
||||
`;
|
||||
|
||||
test('surfaces a gaps_found file\'s human_verification array (the #3850 defect)', () => {
|
||||
const output = auditWith(HV_TWO);
|
||||
assert.strictEqual(output.summary.total_items, 2);
|
||||
assert.strictEqual(output.summary.total_files, 1);
|
||||
assert.strictEqual(output.results[0].type, 'verification');
|
||||
assert.strictEqual(output.results[0].status, 'gaps_found');
|
||||
assert.strictEqual(output.results[0].items[0].category, 'human_uat');
|
||||
// by_phase carries the phase — the omission mechanism this issue is about.
|
||||
assert.strictEqual(output.summary.by_phase['01'], 2);
|
||||
});
|
||||
|
||||
test('the SAME file under human_needed and gaps_found yields identical human_verification items', () => {
|
||||
// The status token alone decided visibility; it must now decide nothing
|
||||
// about the human_verification reading itself.
|
||||
const asGaps = parseVerificationItems(HV_TWO, 'gaps_found');
|
||||
const asHuman = parseVerificationItems(HV_TWO.replace('status: gaps_found', 'status: human_needed'), 'human_needed');
|
||||
assert.deepStrictEqual(asGaps, asHuman);
|
||||
assert.strictEqual(asGaps.length, 2);
|
||||
});
|
||||
|
||||
test('surfaces a gaps_found file\'s frontmatter gaps array', () => {
|
||||
const output = auditWith(`---
|
||||
phase: 02-gaps
|
||||
status: gaps_found
|
||||
gaps:
|
||||
- truth: "The widget renders"
|
||||
status: partial
|
||||
reason: "Only observed on one platform"
|
||||
test: 7
|
||||
---
|
||||
|
||||
# Verification
|
||||
`, '02-gaps');
|
||||
assert.strictEqual(output.summary.total_items, 1);
|
||||
const item = output.results[0].items[0];
|
||||
assert.strictEqual(item.name, 'The widget renders');
|
||||
assert.strictEqual(item.result, 'partial');
|
||||
assert.strictEqual(item.reason, 'Only observed on one platform');
|
||||
// No `test:` number (#3879 review round 4, Minor 4). A `gaps:` entry has no
|
||||
// `test:` in its vocabulary, and reading one collided with the 1..N row
|
||||
// numbers `parseHumanVerificationItems` assigns by array position — two
|
||||
// items numbered 1 in a single file's combined list.
|
||||
assert.strictEqual(item.test, undefined);
|
||||
});
|
||||
|
||||
test('a gaps entry with no parseable status surfaces as unknown, never dropped (fail-safe)', () => {
|
||||
const items = parseVerificationItems(`---
|
||||
status: gaps_found
|
||||
gaps:
|
||||
- truth: "Garbled entry"
|
||||
---
|
||||
`, 'gaps_found');
|
||||
assert.strictEqual(items.length, 1);
|
||||
assert.strictEqual(items[0].result, 'unknown');
|
||||
});
|
||||
|
||||
test('folded >- scalars inside an entry do not break the entry (real verifier shape)', () => {
|
||||
const items = parseVerificationItems(`---
|
||||
status: gaps_found
|
||||
gaps:
|
||||
- truth: "Gate is fail-open on absence of information"
|
||||
status: partial
|
||||
reason: >-
|
||||
The hub learns a collection's schema version only from a push carrying
|
||||
rows, so a zero-row collection is invisible and the RAM-only hub
|
||||
re-blinds itself on every host restart.
|
||||
---
|
||||
`, 'gaps_found');
|
||||
assert.strictEqual(items.length, 1);
|
||||
assert.strictEqual(items[0].name, 'Gate is fail-open on absence of information');
|
||||
assert.strictEqual(items[0].result, 'partial');
|
||||
});
|
||||
|
||||
describe('closed entries are excluded on the gaps_found path', () => {
|
||||
test('skips resolution:-marked human_verification entries, keeping original 1-based positions', () => {
|
||||
const items = parseVerificationItems(`---
|
||||
status: gaps_found
|
||||
human_verification:
|
||||
- test: "Already answered"
|
||||
resolution: "RESOLVED 2026-01-01 by the phase 4 rig run"
|
||||
- test: "Still outstanding"
|
||||
why_human: "Needs real hardware"
|
||||
---
|
||||
`, 'gaps_found');
|
||||
assert.strictEqual(items.length, 1);
|
||||
// Entry 2 keeps position 2 so a surfaced item still names its row.
|
||||
assert.strictEqual(items[0].test, 2);
|
||||
assert.match(items[0].name, /Still outstanding/);
|
||||
});
|
||||
|
||||
test('skips status: resolved gaps entries', () => {
|
||||
const items = parseVerificationItems(`---
|
||||
status: gaps_found
|
||||
gaps:
|
||||
- truth: "Open"
|
||||
status: partial
|
||||
- truth: "Closed"
|
||||
status: resolved
|
||||
---
|
||||
`, 'gaps_found');
|
||||
assert.strictEqual(items.length, 1);
|
||||
assert.strictEqual(items[0].name, 'Open');
|
||||
});
|
||||
|
||||
test('a resolution:-shaped substring MID-LINE is not a closure marker', () => {
|
||||
// extractGapEntryFields anchors a field to the start of its own trimmed
|
||||
// line; a quoted value mentioning "resolution:" must never close an entry.
|
||||
const items = parseVerificationItems(`---
|
||||
status: gaps_found
|
||||
gaps:
|
||||
- truth: "Deferred until the resolution: pending owner ruling"
|
||||
status: partial
|
||||
---
|
||||
`, 'gaps_found');
|
||||
assert.strictEqual(items.length, 1);
|
||||
});
|
||||
|
||||
test('closed entries are excluded on the human_needed path too', () => {
|
||||
// #3850 review m8: an earlier revision skipped closed entries only on
|
||||
// `gaps_found`, citing an acceptance criterion the issue does not
|
||||
// contain. #3850 has no AC section; its suggested fix (2) states the skip
|
||||
// unconditionally, and the file it cites — 14 of 16 entries resolved — is
|
||||
// a `human_needed` one. The asymmetry left the reporter's own scenario
|
||||
// over-reporting by 14. One rule, both statuses.
|
||||
const items = parseVerificationItems(`---
|
||||
status: human_needed
|
||||
human_verification:
|
||||
- test: "Already answered"
|
||||
resolution: "RESOLVED 2026-01-01"
|
||||
- test: "Still outstanding"
|
||||
---
|
||||
`, 'human_needed');
|
||||
assert.strictEqual(items.length, 1);
|
||||
// The display name keeps `extractFrontmatter`'s flattened shape verbatim
|
||||
// (`normalizeHumanVerificationEntry` strips wrapping quotes only) — this
|
||||
// fix derives the SAME string from the raw slice, it does not prettify it.
|
||||
assert.match(items[0].name, /Still outstanding/);
|
||||
// The surfaced entry keeps its ORIGINAL 1-based row, so it still names
|
||||
// its position in the file after a closed sibling was skipped.
|
||||
assert.strictEqual(items[0].test, 2);
|
||||
});
|
||||
|
||||
test('B2: a nested block sequence at key indent does not drop an OPEN entry', () => {
|
||||
// The review's executed repro. `parseYamlRegion` is indent-blind and
|
||||
// `splitGapsEntries` is indent-anchored, so pairing them by ordinal
|
||||
// position made entry B inherit entry C's `resolution:` and disappear.
|
||||
// Both readings now come from one parse, so there is no index to skew.
|
||||
const items = parseVerificationItems(`---
|
||||
status: human_needed
|
||||
human_verification:
|
||||
- test: "A"
|
||||
steps:
|
||||
- s1
|
||||
- test: "B"
|
||||
- test: "C"
|
||||
resolution: "done"
|
||||
---
|
||||
`, 'human_needed');
|
||||
const names = items.map((i) => i.name).join(' | ');
|
||||
// The display rendering comes from `flattenObjectListItem`, which emits
|
||||
// `test: B` — unquoted — where the pre-#3881 flattener emitted `test: "B`.
|
||||
assert.match(names, /\btest: B\b/, `open entry "B" must survive; got ${JSON.stringify(names)}`);
|
||||
assert.ok(!/\btest: C\b/.test(names), `closed entry "C" must be skipped; got ${JSON.stringify(names)}`);
|
||||
});
|
||||
|
||||
test('B2: a bare bullet does not skew the entry list', () => {
|
||||
// Round-3 Blocker. The original assertion matched `test: B` anywhere in a
|
||||
// joined name string, so it passed while "B" was reported at position 2 —
|
||||
// and would still have passed had the bare bullet been dropped entirely.
|
||||
// The defect was never about the NAME surviving; it was about the ROW
|
||||
// number and the row count. Assert both.
|
||||
const items = parseVerificationItems(`---
|
||||
status: human_needed
|
||||
human_verification:
|
||||
- test: "A"
|
||||
-
|
||||
- test: "B"
|
||||
---
|
||||
`, 'human_needed');
|
||||
assert.deepStrictEqual(
|
||||
items.map((i) => [i.test, i.name]),
|
||||
[[1, 'test: A'], [2, ''], [3, 'test: B']],
|
||||
'every row surfaces at its own 1-based position, bare bullet included',
|
||||
);
|
||||
});
|
||||
|
||||
test('B2b: a mixed object/non-object list keeps every row at its true position', () => {
|
||||
// The reviewer's own 6-entry fixture, verbatim. Against the filtered
|
||||
// implementation this returned THREE items — "A", "D", "F" numbered
|
||||
// 1, 2, 3 — with rows 2, 3 and 5 gone and no trace that anything had
|
||||
// been dropped. That is #3850's defect reached through entry SHAPE
|
||||
// rather than file STATUS, so it is pinned on `gaps_found` where the
|
||||
// union path runs.
|
||||
const items = parseVerificationItems(`---
|
||||
status: gaps_found
|
||||
human_verification:
|
||||
- test: "A"
|
||||
-
|
||||
- bare scalar
|
||||
- test: "D"
|
||||
-
|
||||
- nested
|
||||
- test: "F"
|
||||
---
|
||||
`, 'gaps_found');
|
||||
assert.deepStrictEqual(
|
||||
items.map((i) => [i.test, i.name]),
|
||||
[
|
||||
[1, 'test: A'],
|
||||
[2, ''],
|
||||
[3, 'bare scalar'],
|
||||
[4, 'test: D'],
|
||||
[5, '[nested]'],
|
||||
[6, 'test: F'],
|
||||
],
|
||||
'all six rows surface, each at its own position, named as base named them',
|
||||
);
|
||||
});
|
||||
|
||||
test('B2c: skipped-because-resolved rows leave the survivors numbered by FILE row', () => {
|
||||
// The property the positional index exists for, and the one a count-only
|
||||
// assertion cannot see: rows 2 and 4 are closed, so 1, 3 and 5 surface
|
||||
// with their own numbers rather than being renumbered 1, 2, 3.
|
||||
const items = parseVerificationItems(`---
|
||||
status: human_needed
|
||||
human_verification:
|
||||
- test: "A"
|
||||
- test: "B"
|
||||
resolution: done
|
||||
-
|
||||
- test: "D"
|
||||
status: resolved
|
||||
- test: "E"
|
||||
---
|
||||
`, 'human_needed');
|
||||
assert.deepStrictEqual(
|
||||
items.map((i) => [i.test, i.name]),
|
||||
[[1, 'test: A'], [3, ''], [5, 'test: E']],
|
||||
'closed rows are skipped and the survivors keep their file positions',
|
||||
);
|
||||
});
|
||||
|
||||
test('B2d: a non-object gaps entry surfaces instead of vanishing', () => {
|
||||
// Same class, other reader. `gaps:` entries carry their own status, so a
|
||||
// non-object one has none to read — this module's documented fail-safe
|
||||
// (`parseGapsItems`' 'unknown' fallback) is to surface it rather than
|
||||
// drop it. It is named by the same renderer every other reader uses, so
|
||||
// a YAML null reads '' and not the string "null".
|
||||
const items = parseVerificationItems(`---
|
||||
status: gaps_found
|
||||
gaps:
|
||||
- truth: "G1"
|
||||
status: failed
|
||||
-
|
||||
- bare gap
|
||||
- truth: "G3"
|
||||
status: resolved
|
||||
---
|
||||
`, 'gaps_found');
|
||||
assert.deepStrictEqual(
|
||||
items.map((i) => [i.name, i.result]),
|
||||
[['G1', 'failed'], ['', 'unknown'], ['bare gap', 'unknown']],
|
||||
'every unresolved gaps row surfaces; the resolved one is skipped',
|
||||
);
|
||||
});
|
||||
|
||||
test('B1: a BOM does not make a gaps_found report vanish', () => {
|
||||
// #2977's defect class. A hand-rolled fence regex re-asserts the byte-0
|
||||
// rule and slices nothing, which is this issue's symptom verbatim on the
|
||||
// platform the repo already has a named class for.
|
||||
const items = parseVerificationItems(`\uFEFF---
|
||||
status: gaps_found
|
||||
gaps:
|
||||
- truth: "G1"
|
||||
status: partial
|
||||
---
|
||||
`, 'gaps_found');
|
||||
assert.strictEqual(items.length, 1, 'a BOM-prefixed report must still surface its gaps');
|
||||
assert.strictEqual(items[0].name, 'G1');
|
||||
});
|
||||
|
||||
test('M4: CRLF frontmatter surfaces gaps', () => {
|
||||
const crlf = ['---', 'status: gaps_found', 'gaps:', ' - truth: "G1"', ' status: partial', '---', ''].join('\r\n');
|
||||
const items = parseVerificationItems(crlf, 'gaps_found');
|
||||
assert.strictEqual(items.length, 1);
|
||||
assert.strictEqual(items[0].name, 'G1');
|
||||
});
|
||||
});
|
||||
|
||||
describe('array-population boundaries', () => {
|
||||
const build = (hv, gaps) => [
|
||||
'---', 'status: gaps_found',
|
||||
...(hv === null ? [] : ['human_verification:', ...hv.map((t) => ` - test: "${t}"`)]),
|
||||
...(gaps === null ? [] : ['gaps:', ...gaps.map((t) => ` - truth: "${t}"\n status: partial`)]),
|
||||
'---', '',
|
||||
].join('\n');
|
||||
|
||||
test('empty human_verification + populated gaps still surfaces the gaps', () => {
|
||||
const items = parseVerificationItems(build([], ['G1']), 'gaps_found');
|
||||
assert.strictEqual(items.length, 1);
|
||||
assert.strictEqual(items[0].name, 'G1');
|
||||
});
|
||||
|
||||
test('populated human_verification + no gaps key surfaces only the hv entries', () => {
|
||||
const items = parseVerificationItems(build(['H1'], null), 'gaps_found');
|
||||
assert.strictEqual(items.length, 1);
|
||||
assert.strictEqual(items[0].category, 'human_uat');
|
||||
});
|
||||
|
||||
test('both arrays populated surfaces the union, human_verification first', () => {
|
||||
const items = parseVerificationItems(build(['H1', 'H2'], ['G1']), 'gaps_found');
|
||||
assert.strictEqual(items.length, 3);
|
||||
assert.strictEqual(items[0].category, 'human_uat');
|
||||
assert.strictEqual(items[2].name, 'G1');
|
||||
});
|
||||
|
||||
test('neither key present yields zero items', () => {
|
||||
assert.strictEqual(parseVerificationItems(build(null, null), 'gaps_found').length, 0);
|
||||
});
|
||||
|
||||
test('a single entry (N=1) surfaces', () => {
|
||||
assert.strictEqual(parseVerificationItems(build(['H1'], null), 'gaps_found').length, 1);
|
||||
});
|
||||
|
||||
test('every entry closed yields zero items, so the file is omitted — intended, not the #3850 bug', () => {
|
||||
// The omission this issue reports is a file with OUTSTANDING items
|
||||
// vanishing. A file whose every entry is closed has nothing outstanding,
|
||||
// so `items.length > 0` correctly drops it. Pinned to keep the two cases
|
||||
// distinguishable.
|
||||
const output = auditWith(`---
|
||||
phase: 03-allclosed
|
||||
status: gaps_found
|
||||
human_verification:
|
||||
- test: "Answered"
|
||||
resolution: "RESOLVED"
|
||||
gaps:
|
||||
- truth: "Closed"
|
||||
status: resolved
|
||||
---
|
||||
`, '03-allclosed');
|
||||
assert.strictEqual(output.summary.total_items, 0);
|
||||
assert.strictEqual(output.summary.total_files, 0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('property: the closed/open partition', () => {
|
||||
// Text that can never be mistaken for a field line or a closure marker.
|
||||
const bodyArb = fc.string({ minLength: 1, maxLength: 24 })
|
||||
.map((s) => s.replace(/["\\\r\n:]/g, ''))
|
||||
.filter((s) => s.trim().length > 0);
|
||||
|
||||
// Closure is PER KEY (#3879 review round 4, Major), so the property is too.
|
||||
// `gaps:` takes `parseGapsItems`' rule verbatim — `status: resolved` and
|
||||
// nothing else — while `human_verification:` also honours a bare
|
||||
// `resolution:`. Generating one spelling set for both keys is what let the
|
||||
// old universal rule read green.
|
||||
//
|
||||
// `contradiction` is the round-4 Major itself: a `status:` that is not
|
||||
// `resolved` sitting beside a `resolution:` note. It is OPEN under both
|
||||
// keys — `status:` is authoritative wherever it is readable.
|
||||
const OPEN_SPELLINGS = {
|
||||
gaps: {
|
||||
plain: [' status: partial'],
|
||||
// No `status:` at all. Under the gaps rule this is not closure, and
|
||||
// `parseGapsItems` already surfaces it via its 'unknown' fallback.
|
||||
resolutionOnly: [' resolution: "a note, not a closure assertion"'],
|
||||
contradiction: [' status: failed', ' resolution: "attempted retry, still failing"'],
|
||||
},
|
||||
human_verification: {
|
||||
plain: [' status: partial'],
|
||||
contradiction: [' status: failed', ' resolution: "attempted retry, still failing"'],
|
||||
},
|
||||
};
|
||||
const CLOSED_SPELLINGS = {
|
||||
gaps: {
|
||||
status: [' status: resolved'],
|
||||
},
|
||||
human_verification: {
|
||||
status: [' status: resolved'],
|
||||
resolutionOnly: [' resolution: "closed upstream"'],
|
||||
},
|
||||
};
|
||||
|
||||
const entryArb = (key) => fc.record({
|
||||
body: bodyArb,
|
||||
closed: fc.boolean(),
|
||||
openSpelling: fc.constantFrom(...Object.keys(OPEN_SPELLINGS[key])),
|
||||
closedSpelling: fc.constantFrom(...Object.keys(CLOSED_SPELLINGS[key])),
|
||||
});
|
||||
|
||||
const renderEntry = (key, e) => {
|
||||
const nameLine = key === 'gaps' ? ` - truth: "${e.name}"` : ` - test: "${e.name}"`;
|
||||
const fields = e.closed
|
||||
? CLOSED_SPELLINGS[key][e.closedSpelling]
|
||||
: OPEN_SPELLINGS[key][e.openSpelling];
|
||||
return [nameLine, ...fields];
|
||||
};
|
||||
|
||||
test('property: a gaps entry surfaces iff its own status is not resolved; surfaced + skipped == total', () => {
|
||||
fc.assert(
|
||||
fc.property(
|
||||
fc.array(entryArb('gaps'), { maxLength: 12 }),
|
||||
(raw) => {
|
||||
// Index-prefix so surfaced names map back unambiguously even when
|
||||
// the generated bodies collide.
|
||||
const entries = raw.map((e, i) => ({ ...e, name: `E${i}_${e.body}` }));
|
||||
const lines = ['---', 'status: gaps_found', 'gaps:'];
|
||||
for (const e of entries) lines.push(...renderEntry('gaps', e));
|
||||
lines.push('---', '');
|
||||
|
||||
const items = parseVerificationItems(lines.join('\n'), 'gaps_found');
|
||||
const surfaced = new Set(items.map((it) => it.name));
|
||||
const open = entries.filter((e) => !e.closed);
|
||||
const closed = entries.filter((e) => e.closed);
|
||||
|
||||
// Partition: surfaced count is exactly the open count...
|
||||
assert.strictEqual(items.length, open.length);
|
||||
// ...every open entry surfaces, INCLUDING the two spellings the old
|
||||
// universal rule swallowed (a bare `resolution:`, and a
|
||||
// `resolution:` beside a non-resolved `status:`)...
|
||||
for (const e of open) assert.ok(surfaced.has(e.name), `open entry missing: ${e.name}`);
|
||||
// ...no closed entry ever does...
|
||||
for (const e of closed) assert.ok(!surfaced.has(e.name), `closed entry surfaced: ${e.name}`);
|
||||
// ...and the two parts account for the whole.
|
||||
assert.strictEqual(open.length + closed.length, entries.length);
|
||||
},
|
||||
),
|
||||
);
|
||||
});
|
||||
|
||||
test('property: a human_verification entry surfaces iff no readable status contradicts its closure', () => {
|
||||
fc.assert(
|
||||
fc.property(
|
||||
fc.array(entryArb('human_verification'), { maxLength: 12 }),
|
||||
(raw) => {
|
||||
const entries = raw.map((e, i) => ({ ...e, name: `E${i}_${e.body}` }));
|
||||
const lines = ['---', 'status: human_needed', 'human_verification:'];
|
||||
for (const e of entries) lines.push(...renderEntry('human_verification', e));
|
||||
lines.push('---', '');
|
||||
|
||||
const items = parseVerificationItems(lines.join('\n'), 'human_needed');
|
||||
const open = entries.filter((e) => !e.closed);
|
||||
|
||||
// Counts only: this reader names items through
|
||||
// `normalizeHumanVerificationEntry`'s display rendering, so asserting
|
||||
// the partition by NAME would pin that renderer rather than the
|
||||
// closure rule under test.
|
||||
assert.strictEqual(items.length, open.length);
|
||||
// Numbering stays the entry's ORIGINAL 1-based row even when a
|
||||
// closed sibling was skipped (the #3850 round-3 Blocker).
|
||||
const expectedRows = entries
|
||||
.map((e, i) => (e.closed ? null : i + 1))
|
||||
.filter((n) => n !== null);
|
||||
assert.deepStrictEqual(items.map((it) => it.test), expectedRows);
|
||||
},
|
||||
),
|
||||
);
|
||||
});
|
||||
|
||||
test('property: an all-open array surfaces every entry (no silent cap)', () => {
|
||||
fc.assert(
|
||||
fc.property(fc.array(bodyArb, { minLength: 1, maxLength: 20 }), (bodies) => {
|
||||
const names = bodies.map((b, i) => `E${i}_${b}`);
|
||||
const content = ['---', 'status: gaps_found', 'gaps:',
|
||||
...names.map((n) => ` - truth: "${n}"\n status: partial`), '---', ''].join('\n');
|
||||
assert.strictEqual(parseVerificationItems(content, 'gaps_found').length, names.length);
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user