Skip to content

Commit edafa33

Browse files
committed
fix(spec): file-description skips doc blocks the lazify codemod detached from their symbols
1 parent 3322527 commit edafa33

2 files changed

Lines changed: 293 additions & 7 deletions

File tree

packages/spec/scripts/file-description.test.ts

Lines changed: 202 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -198,16 +198,26 @@ describe('findModuleDocBlock — a block documents a symbol, or it documents the
198198
});
199199

200200
it('keeps a header the lazify codemod separated from the imports', () => {
201-
// `scripts/lazify-schemas.ts` inserts its import after the leading run of
202-
// comments and imports — and that run swallows a doc block, so a header can
203-
// end up with imports on both sides. It is still a header.
201+
// `api/analytics.zod.ts`, as it really is on `main`. `lazify-schemas.ts`
202+
// inserts its import after the leading run of comments and imports — and
203+
// that run swallows a doc block, so a header can end up with imports on
204+
// both sides. It is still a header, and the banner is why: the block was
205+
// not sitting against the declaration before the codemod ran either.
206+
//
207+
// ⚠️ This case used to be written WITHOUT the banner, and that reduction
208+
// dropped the one line carrying the verdict (#13263) — see the case below,
209+
// which is that bannerless shape and now asserts the opposite.
204210
const source = [
205211
"import { z } from 'zod';",
206212
'',
207213
'/**',
208214
' * Analytics API Protocol',
209215
' */',
210216
'',
217+
'// ==========================================',
218+
'// 1. API Endpoints',
219+
'// ==========================================',
220+
'',
211221
"import { lazySchema } from '../shared/lazy-schema';",
212222
'export const AnalyticsEndpoint = z.enum([]);',
213223
'',
@@ -220,6 +230,143 @@ describe('findModuleDocBlock — a block documents a symbol, or it documents the
220230
});
221231
});
222232

233+
/**
234+
* #13263 — an injected import is not a separator.
235+
*
236+
* `lazify-schemas.ts` injects its `lazySchema` import at the END of the file's
237+
* leading run of comments, blank lines and imports, and its regex for that run
238+
* counts a doc block among the comments. A module written as an import, a blank
239+
* line, `Service Status Enum` and then `export const ServiceStatus` therefore
240+
* came out of the codemod with the import BETWEEN the block and its symbol —
241+
* and condition 3, which skipped only blank lines, then read the block as
242+
* documenting nothing. 28 reference pages opened with one schema's comment, and
243+
* `gen:skill-refs` copied each first line into the published skill indexes.
244+
*
245+
* The tightening applies only INSIDE the import block and stops at a comment of
246+
* any kind, which is what the four `keeps` cases below pin: without the first
247+
* limb, three real headers written above their imports go blank; without the
248+
* second, `api/analytics` and `system/cache` do.
249+
*
250+
* MEASURED over `packages/spec/src` (193 sources, base `3322527f`): 28 pages
251+
* lose a misattributed opening, 0 change to a different block, 165 are
252+
* byte-identical. The corpus limb at the end of this file re-derives that
253+
* rather than restating it.
254+
*/
255+
describe('findModuleDocBlock — #13263: an import injected between a block and its symbol', () => {
256+
it('rejects a block the codemod separated from the declaration it documents', () => {
257+
// `api/discovery.zod.ts`, `data/field.zod.ts`, and 26 more. This is the
258+
// previous case's source with the banner removed — the whole difference.
259+
const source = [
260+
"import { z } from 'zod';",
261+
'',
262+
'/**',
263+
' * Service Status Enum',
264+
' * Describes the operational state of a service in the discovery response.',
265+
' */',
266+
"import { lazySchema } from '../shared/lazy-schema';",
267+
"export const ServiceStatus = z.enum(['available', 'stub']);",
268+
'',
269+
].join('\n');
270+
expect(findModuleDocBlock(source)).toBeNull();
271+
});
272+
273+
it('rejects it across a blank line and several injected imports', () => {
274+
// `automation/flow.zod.ts` has five between the block and `FlowNodeAction`;
275+
// `data/object.zod.ts` six. Distance in plumbing lines is not a signal.
276+
const source = [
277+
"import { z } from 'zod';",
278+
"import { ProtectionSchema } from '../shared/protection.zod';",
279+
'',
280+
'/**',
281+
' * Flow Node Types — built-in seed set (ADR-0018).',
282+
' */',
283+
"import { lazySchema } from '../shared/lazy-schema';",
284+
"import { retiredKey } from '../shared/retired-key';",
285+
"import { strictObject } from '../shared/strict-object';",
286+
'',
287+
"export const FlowNodeAction = z.enum(['start', 'end']);",
288+
'',
289+
].join('\n');
290+
expect(findModuleDocBlock(source)).toBeNull();
291+
});
292+
293+
it('rejects it across a MULTI-LINE import — continuation lines are plumbing too', () => {
294+
// `data/document.zod.ts` and `kernel/execution-context.zod.ts` reach their
295+
// declaration only over a wrapped import's `Foo,` and `} from '…';` lines.
296+
const source = [
297+
"import { z } from 'zod';",
298+
'',
299+
'/**',
300+
' * Document Version Schema',
301+
' */',
302+
'import {',
303+
' MetadataProtectionFields,',
304+
' ProtectionSchema,',
305+
"} from '../kernel/metadata-protection.zod';",
306+
'export const DocumentVersionSchema = z.object({});',
307+
'',
308+
].join('\n');
309+
expect(findModuleDocBlock(source)).toBeNull();
310+
});
311+
312+
it('keeps a header written ABOVE the imports, even with a declaration right after them', () => {
313+
// `system/doc.zod.ts`, `cloud/template-manifest.zod.ts`,
314+
// `api/error-code-ledger.zod.ts`. The codemod injects AFTER the last
315+
// import, so a block preceding every import preceded them beforehand too —
316+
// the position is the proof, and dropping this limb blanks all three.
317+
const source = [
318+
'// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.',
319+
'',
320+
'/**',
321+
' * Package Documentation Metadata Protocol (ADR-0046)',
322+
' */',
323+
'',
324+
"import { z } from 'zod';",
325+
"import { lazySchema } from '../shared/lazy-schema';",
326+
'',
327+
'export const DocSchema = z.object({});',
328+
'',
329+
].join('\n');
330+
expect(opening(findModuleDocBlock(source))).toBe('Package Documentation Metadata Protocol (ADR-0046)');
331+
});
332+
333+
it('keeps a header inside the imports when the next schema carries its own JSDoc', () => {
334+
// `system/cache.zod.ts` — its block names itself ("This File") and the
335+
// declaration beyond the injected import is already documented, so that
336+
// import says nothing about what the block documents.
337+
const source = [
338+
"import { z } from 'zod';",
339+
"import { CronExpressionInputSchema } from '../shared/expression.zod';",
340+
'',
341+
'/**',
342+
' * Application-Level Cache Protocol',
343+
' */',
344+
"import { lazySchema } from '../shared/lazy-schema';",
345+
'',
346+
'/** Cache eviction strategy. */',
347+
"export const CacheStrategySchema = z.enum(['lru']);",
348+
'',
349+
].join('\n');
350+
expect(opening(findModuleDocBlock(source))).toBe('Application-Level Cache Protocol');
351+
});
352+
353+
it('keeps a block with no declaration on the far side at all', () => {
354+
// A module that is nothing but re-exports — the plumbing runs to the end of
355+
// the file, so there is no symbol for the block to have been torn from.
356+
const source = [
357+
"import { z } from 'zod';",
358+
'',
359+
'/**',
360+
' * Environment Artifact Envelope — re-export',
361+
' */',
362+
"export { EnvironmentArtifactSchema } from './artifact.zod';",
363+
"export type { EnvironmentArtifact } from './artifact.zod';",
364+
'',
365+
].join('\n');
366+
expect(opening(findModuleDocBlock(source))).toBe('Environment Artifact Envelope — re-export');
367+
});
368+
});
369+
223370
describe('renderFileDescription', () => {
224371
// `fromCategory` is the directory the rendered module lives in (#6484); these
225372
// cases reference `automation/` and are written as if from there.
@@ -1023,6 +1170,58 @@ describe('corpus — no reference source donates a symbol comment to its page',
10231170
expect(openingOf('api/error-code-ledger.zod.ts'))
10241171
.toBe('Error-Code Ledger (ADR-0112 D3).');
10251172
});
1173+
1174+
/**
1175+
* #13263's corpus limb — the half that cannot rot.
1176+
*
1177+
* Re-derives the verdict from the real tree instead of restating a file list:
1178+
* a selected block that sits inside the import block must have a COMMENT on
1179+
* the far side of that plumbing, never a declaration. A source that acquires
1180+
* the codemod shape later cannot quietly re-acquire a wrong page description,
1181+
* and the three headers written above their imports are pinned by name in the
1182+
* `#6145` case above, so neither direction can drift alone.
1183+
*/
1184+
it('never selects a block with a declaration on the far side of the imports', () => {
1185+
const PLUMBING = /^(?:import\b|export\s*(?:\*|\{|type\s*\{))/;
1186+
const offenders: string[] = [];
1187+
for (const file of zodFiles) {
1188+
const source = fs.readFileSync(file, 'utf-8');
1189+
const block = findModuleDocBlock(source);
1190+
if (block === null) continue;
1191+
const lines = source.split('\n');
1192+
const marker = `/**${block}*/`;
1193+
const at = source.indexOf(marker);
1194+
if (at < 0) continue; // already reported by the case above
1195+
const startLine = source.slice(0, at).split('\n').length - 1;
1196+
const endLine = startLine + marker.split('\n').length - 1;
1197+
if (!lines.slice(0, startLine).some(l => PLUMBING.test(l))) continue; // above the imports
1198+
1199+
for (let i = endLine + 1; i < lines.length; i++) {
1200+
const line = lines[i];
1201+
if (line.trim() === '') continue;
1202+
if (line.trimStart().startsWith('/')) break; // a comment ends the preamble
1203+
if (PLUMBING.test(line)) continue;
1204+
if (!/^[A-Za-z_$@]/.test(line)) continue; // continuation / closing punctuation
1205+
offenders.push(`${path.relative(SRC_DIR, file)}${line.slice(0, 60)}`);
1206+
break;
1207+
}
1208+
}
1209+
expect(offenders).toEqual([]);
1210+
});
1211+
1212+
it('publishes no description for the modules whose opening was one schema\'s doc', () => {
1213+
const openingOf = (rel: string) =>
1214+
opening(findModuleDocBlock(fs.readFileSync(path.join(SRC_DIR, rel), 'utf-8')));
1215+
1216+
// Four of the 28, one per shape: the plain single injected import, the run
1217+
// of five, the wrapped import, and the one whose page-wide blast radius the
1218+
// card measured (`gen:skill-refs` shipped `Field Type Enum` as the pointer
1219+
// row for a module of ~40 field schemas).
1220+
expect(openingOf('api/discovery.zod.ts')).toBeNull();
1221+
expect(openingOf('automation/flow.zod.ts')).toBeNull();
1222+
expect(openingOf('kernel/execution-context.zod.ts')).toBeNull();
1223+
expect(openingOf('data/field.zod.ts')).toBeNull();
1224+
});
10261225
});
10271226

10281227
/**

packages/spec/scripts/lib/file-description.ts

Lines changed: 91 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@
3333
* and this repo writes module headers on either side of them); the first
3434
* `const`/`export const`/… does.
3535
* 3. **Documenting nothing** — the block is not immediately followed by a
36-
* declaration.
36+
* declaration, and — when the block sits INSIDE the import block — no
37+
* declaration sits on the far side of that plumbing either.
3738
*
3839
* (3) is the load-bearing one, and it is simply TSDoc's own rule read back: a
3940
* doc block belongs to the declaration it immediately precedes, which is why
@@ -42,8 +43,51 @@
4243
* as the Realtime page's opening paragraph was the generator inventing a second
4344
* meaning for text that already had one. "Immediately" means blank lines only:
4445
* nobody separates a JSDoc from its symbol with a `// ═══` banner, so a banner
45-
* (or another doc block, or an import) between the two marks the end of the
46-
* preamble rather than an attachment.
46+
* (or another doc block) between the two marks the end of the preamble rather
47+
* than an attachment.
48+
*
49+
* ## An import between a block and its symbol is not a separator (#13263)
50+
*
51+
* The one exception to that last sentence, and it is not a judgement call — it
52+
* is a codemod. `scripts/lazify-schemas.ts` injects
53+
* `import { lazySchema } from '…/shared/lazy-schema';` at the END of the file's
54+
* leading run of comments, blank lines and imports, and the regex it uses for
55+
* that run counts a doc block among the comments (its comment alternative is
56+
* slash-star, any run, star-slash — a doc block matches it). So when a module
57+
* was written as an `import { z } from 'zod';`, a blank line, a doc block
58+
* reading `Service Status Enum`, and then `export const ServiceStatus`,
59+
* the run swallowed the doc block and the injected import landed BETWEEN the
60+
* block and the symbol it documents. Nothing about the block changed; only the
61+
* detector's view of it did. Reading that import as "the preamble ends here"
62+
* republished 28 symbol comments as their pages' subjects, `api/discovery`'s
63+
* `Service Status Enum` and `data/field`'s `Field Type Enum` among them — and
64+
* `gen:skill-refs` copied each one's first line into the PUBLISHED skill
65+
* indexes, so the misattribution shipped to customer projects.
66+
*
67+
* Hence condition 3's second half: a declaration reachable across nothing but
68+
* blank lines and plumbing is still the block's subject. Two things keep this a
69+
* tightening rather than a demotion of the spelling `MODULE_PLUMBING` exists to
70+
* permit — measured over all 193 reference sources, and both are load-bearing:
71+
*
72+
* - **It applies only INSIDE the import block.** A header written ABOVE the
73+
* imports is where the codemod cannot have put it: the injection point is
74+
* after the last import, so a block preceding every import was preceding them
75+
* before the codemod ran too. Without that half, `api/error-code-ledger`,
76+
* `cloud/template-manifest` and `system/doc` — three real headers whose
77+
* imports happen to be followed directly by a declaration — lose their
78+
* opening paragraph.
79+
* - **A comment of any kind on the far side still ends the preamble.** A `// ═══`
80+
* banner or a second doc block means the block did NOT sit against the
81+
* declaration before the codemod ran, so the injection tells us nothing.
82+
* `api/analytics` (banner) and `system/cache` (the next schema's own JSDoc)
83+
* keep their headers through exactly this clause.
84+
*
85+
* What it deliberately does NOT decide is the block that sits inside the import
86+
* list with a comment on the far side: `system/cache` and `shared/mapping` are
87+
* genuine headers there and seven others are detached symbol docs, and no
88+
* positional or structural signal separates them — only the prose does. That
89+
* residue needs an explicit `@module` marker or a corpus pass, not a cleverer
90+
* detector; #13263 records the reading, module by module.
4791
*
4892
* When no block qualifies, the module has no description and the page prints
4993
* none. 宁可缺,不要错 — a missing paragraph is a gap the reader can see, while
@@ -255,6 +299,40 @@ function nextNonBlankLine(lines: readonly string[], from: number): number | null
255299
return i < lines.length ? i : null;
256300
}
257301

302+
/**
303+
* Is there a declaration on the far side of the plumbing that starts at `from`
304+
* — i.e. does everything between hold nothing but blank lines and imports?
305+
*
306+
* Only ever asked of a block that sits INSIDE the import block, where the one
307+
* thing known to put an import between a doc block and its symbol is the lazify
308+
* codemod (see the module comment). Answering `true` there restores the verdict
309+
* the block had before that import was injected.
310+
*
311+
* A comment of any kind — a `// ═══` banner, a second doc block, an import's
312+
* own explanatory note — answers `false` instead of being walked over. That is
313+
* the same boundary `nextNonBlankLine` draws and for the same reason: it means
314+
* the block was NOT sitting against the declaration beforehand either, so the
315+
* injected import carries no information about what the block documents. It is
316+
* what keeps `api/analytics` (banner) and `system/cache` (the next schema's own
317+
* JSDoc) opening with their real module headers.
318+
*
319+
* Continuation and closing lines of a multi-line import (` Foo,`,
320+
* `} from './x';`) are plumbing too — they open with neither an identifier
321+
* character nor a comment delimiter, exactly as `findModuleDocBlock`'s own walk
322+
* reads them.
323+
*/
324+
function declarationBeyondPlumbing(lines: readonly string[], from: number): boolean {
325+
for (let i = from; i < lines.length; i++) {
326+
const line = lines[i];
327+
if (line.trim() === '') continue;
328+
if (line.trimStart().startsWith('/')) return false; // a comment ends the preamble
329+
if (MODULE_PLUMBING.test(line)) continue;
330+
if (!startsDeclaration(line)) continue; // continuation / closing punctuation
331+
return true;
332+
}
333+
return false;
334+
}
335+
258336
/**
259337
* The module's own doc block, INNER text only (delimiters stripped, `*` line
260338
* prefixes intact) — or `null` when the module does not have one.
@@ -271,6 +349,12 @@ function nextNonBlankLine(lines: readonly string[], from: number): number | null
271349
export function findModuleDocBlock(source: string): string | null {
272350
const lines = source.split('\n');
273351

352+
// Whether the walk has passed an import / re-export, i.e. whether a block
353+
// found from here on sits INSIDE the import block rather than above it. Only
354+
// there can the lazify codemod have put an import between a block and its
355+
// symbol, so only there does the far-side check below apply (#13263).
356+
let insideImportBlock = false;
357+
274358
let i = 0;
275359
while (i < lines.length) {
276360
const line = lines[i];
@@ -280,13 +364,16 @@ export function findModuleDocBlock(source: string): string | null {
280364
if (end >= lines.length) return null; // unterminated — nothing to trust
281365
const next = nextNonBlankLine(lines, end + 1);
282366
if (next !== null && startsDeclaration(lines[next])) return null; // documents a symbol
367+
// …and the same verdict when only injected plumbing stands between the
368+
// two: the import moved, the attachment did not.
369+
if (insideImportBlock && declarationBeyondPlumbing(lines, end + 1)) return null;
283370
const raw = lines.slice(i, end + 1).join('\n');
284371
return raw.slice(raw.indexOf('/**') + 3, raw.lastIndexOf('*/'));
285372
}
286373

287374
if (line.startsWith('/*')) { i = endOfBlockComment(lines, i) + 1; continue; }
288375
if (line.trim() === '' || line.trim().startsWith('//') || !/^\S/.test(line)) { i++; continue; }
289-
if (MODULE_PLUMBING.test(line)) { i++; continue; }
376+
if (MODULE_PLUMBING.test(line)) { insideImportBlock = true; i++; continue; }
290377
if (startsDeclaration(line)) return null; // header zone closed before any block
291378

292379
i++; // closing punctuation of a multi-line import / re-export

0 commit comments

Comments
 (0)