diff --git a/scripts/cacbg/fetch.mjs b/scripts/cacbg/fetch.mjs index f947d94a..a41a7530 100644 --- a/scripts/cacbg/fetch.mjs +++ b/scripts/cacbg/fetch.mjs @@ -68,6 +68,10 @@ export function assessCompleteness(perFolder, skippedFolders = []) { sourceGaps += f.missing; // 404 — listed but unpublished at source (expected, not a shortfall) unfetched += f.errors; // announced but not obtained for a non-404 reason — a real shortfall } + // Every announced row ends in exactly one bucket, so a full pass satisfies + // announced == obtained + sourceGaps + unfetched. A surplus means rows were never ATTEMPTED — the + // --limit case, which produces no errors and would otherwise sail through the checks below. + const notAttempted = Math.max(0, announcedDeclarations - (obtained + sourceGaps + unfetched)); return { reachedSets: Object.keys(perFolder).length, skippedSets: skippedFolders.length, @@ -75,7 +79,8 @@ export function assessCompleteness(perFolder, skippedFolders = []) { obtained, sourceGaps, unfetched, - incomplete: unfetched > 0 || skippedFolders.length > 0, + notAttempted, + incomplete: unfetched > 0 || notAttempted > 0 || skippedFolders.length > 0, }; } @@ -178,8 +183,12 @@ export async function run({ } atomicWrite(path.join(dir, 'list.xml'), listRes.body); // cache list for extract.mjs let rows = parseList(listRes.body.toString('utf8')); + // `announced` is what the SET declares, so it is read BEFORE --limit truncates the work. Taking it + // after the slice made a deliberately partial crawl report announced == obtained, i.e. the completeness + // gate certified a corpus it had never attempted to fetch (ydimitrof #226). + const announced = rows.length; if (Number.isFinite(limit)) rows = rows.slice(0, limit); - const fstat = { announced: rows.length, fetched: 0, cached: 0, missing: 0, errors: 0 }; + const fstat = { announced, fetched: 0, cached: 0, missing: 0, errors: 0 }; stats.folders[folder] = fstat; console.log(` ${folder}: ${rows.length} declarations`); @@ -240,6 +249,7 @@ export async function run({ stats.skippedFolders.map((s) => `${s.folder} (${s.status})`).join(', ') || 'none'; const msg = `INCOMPLETE CORPUS — ${completeness.unfetched} announced declaration(s) unfetched (non-404), ` + + `${completeness.notAttempted} never attempted (--limit), ` + `${completeness.skippedSets} set(s) skipped: ${skipped}. Publishing this would omit declarations ` + `the register lists.`; if (allowIncomplete) { diff --git a/scripts/cacbg/fetch.test.mjs b/scripts/cacbg/fetch.test.mjs index 7e0150f2..8227bdf4 100644 --- a/scripts/cacbg/fetch.test.mjs +++ b/scripts/cacbg/fetch.test.mjs @@ -106,3 +106,25 @@ test('assessCompleteness: an empty crawl of fully-obtained sets is complete', () assert.equal(r.announcedDeclarations, 2); assert.equal(r.incomplete, false); }); + +// --limit truncates the WORK, never the announcement. Before this, `announced` was read after the slice, +// so a deliberately partial crawl reported announced == obtained and the gate certified a corpus it had +// never tried to fetch. Rows that were never attempted produce no errors, so `incomplete` has to notice +// the arithmetic hole itself: announced > obtained + sourceGaps + unfetched. +test('assessCompleteness: rows announced but never attempted (--limit) mark the corpus INCOMPLETE', () => { + const r = assessCompleteness( + { 2024: { announced: 5000, fetched: 10, cached: 0, missing: 0, errors: 0 } }, + [], + ); + assert.equal(r.notAttempted, 4990); + assert.equal(r.unfetched, 0, 'no fetch was even tried, so nothing can have errored'); + assert.equal(r.incomplete, true); +}); +test('assessCompleteness: notAttempted is 0 when every announced row landed in a bucket', () => { + const r = assessCompleteness( + { 2024: { announced: 10, fetched: 6, cached: 3, missing: 1, errors: 0 } }, + [], + ); + assert.equal(r.notAttempted, 0); + assert.equal(r.incomplete, false); +}); diff --git a/scripts/cacbg/guard.mjs b/scripts/cacbg/guard.mjs index 8ff66e44..97a326bd 100644 --- a/scripts/cacbg/guard.mjs +++ b/scripts/cacbg/guard.mjs @@ -22,12 +22,26 @@ export function assertScratchIgnored() { // Path-sanitize an xml_file / year from the untrusted list.xml before using it in a filesystem path // or URL. Rejects traversal, absolute paths, and anything outside the expected shape. +// The shape of a real declaration filename. Single source of truth for both the throwing guard below +// (used on the fetch path) and the boolean twin (used by parseList to tell a real row from a phantom). +const XML_FILE_SHAPE = /^[A-Za-z0-9._-]+\.xml$/; + export function safeXmlFile(name) { const base = path.basename(String(name)); - if (!/^[A-Za-z0-9._-]+\.xml$/.test(base)) throw new Error(`unsafe xmlFile: ${name}`); + if (!XML_FILE_SHAPE.test(base)) throw new Error(`unsafe xmlFile: ${name}`); return base; } +/** + * Does this value name a declaration file at all? The boolean twin of safeXmlFile — same shape, no throw. + * parseList needs the question answered without an exception because a non-answer there is not an error: + * the register's list.xml carries placeholder rows (`U`) that announce no document. + * @returns {boolean} + */ +export function isXmlFile(name) { + return XML_FILE_SHAPE.test(path.basename(String(name ?? ''))); +} + export function safeYear(year) { const y = String(year); if (!/^20\d{2}$/.test(y)) throw new Error(`unsafe year: ${year}`); diff --git a/scripts/cacbg/parse.mjs b/scripts/cacbg/parse.mjs index 09d9e8c7..bf95f154 100644 --- a/scripts/cacbg/parse.mjs +++ b/scripts/cacbg/parse.mjs @@ -13,6 +13,7 @@ // XXE-safe: fast-xml-parser resolves no DTDs/external entities; we also reject DOCTYPE/ENTITY input. import { XMLParser } from 'fast-xml-parser'; +import { isXmlFile } from './guard.mjs'; const parser = new XMLParser({ ignoreAttributes: false, @@ -107,8 +108,15 @@ export function parseList(xml) { for (const pos of asArray(person?.Position)) { const position = flat(pos?.Name); for (const decl of asArray(pos?.Declaration)) { + // A row must NAME A FILE to be announced. The register also emits placeholder rows that + // announce no document — `FalseUУведомление` + // (87 of them across 15 sets). A bare truthiness test accepted 'U' as a filename, so the + // crawler counted a declaration that does not exist, then failed to fetch it and booked an + // error — permanently pinning the completeness gate for those sets below their announced + // count. Require the filename shape and a phantom is never announced in the first place. const xmlFile = flat(decl?.xmlFile); - if (xmlFile) out.push({ category, institution, person: name, position, xmlFile }); + if (isXmlFile(xmlFile)) + out.push({ category, institution, person: name, position, xmlFile }); } } } diff --git a/scripts/cacbg/parse.test.mjs b/scripts/cacbg/parse.test.mjs index 06790638..30495481 100644 --- a/scripts/cacbg/parse.test.mjs +++ b/scripts/cacbg/parse.test.mjs @@ -212,3 +212,40 @@ test('XXE guard rejects DOCTYPE/ENTITY input', () => { assert.throws(() => parseDeclaration(evil), /XXE guard/); assert.throws(() => parseList(evil), /XXE guard/); }); + +// The register announces placeholder rows that name no document — `FalseU` +// with `Уведомление`. 87 of them sit across 15 sets. A bare truthiness test on xmlFile +// accepted 'U' as a filename, so the crawler announced a declaration that does not exist, failed to fetch +// it, and booked an error — pinning the completeness gate below the announced count for those sets forever. +const LIST_WITH_PHANTOM = ` + + + Иван Петров Тестов + Директор + REAL.xml + FalseUУведомление + + + +`; + +test('parseList: a placeholder row naming no file is NOT announced', () => { + const rows = parseList(LIST_WITH_PHANTOM); + assert.equal(rows.length, 1, 'only the row bearing a real filename counts'); + assert.equal(rows[0].xmlFile, 'REAL.xml'); +}); + +test('parseList: only values shaped like a declaration filename are announced', () => { + const shape = (v) => + parseList(` + + Име Име Имеп + ${v} + +`).length; + assert.equal(shape('AAAA.xml'), 1); + assert.equal(shape('U'), 0, 'the register placeholder'); + assert.equal(shape(''), 0); + assert.equal(shape('Уведомление'), 0, 'a title slotted into the filename field'); + assert.equal(shape('AAAA.pdf'), 0, 'not a declaration document'); +});