diff --git a/.changelog/next/fixed-issue-4175.md b/.changelog/next/fixed-issue-4175.md new file mode 100644 index 0000000000..db0da3b767 --- /dev/null +++ b/.changelog/next/fixed-issue-4175.md @@ -0,0 +1 @@ +- Server and client mirror contracts now detect missing parity coverage before drift reaches users. diff --git a/server/lib/appIdentity.mirror.test.js b/server/lib/appIdentity.mirror.test.js new file mode 100644 index 0000000000..53af99262a --- /dev/null +++ b/server/lib/appIdentity.mirror.test.js @@ -0,0 +1,9 @@ +import { describe, expect, it } from 'vitest'; +import { PORTOS_APP_ID as serverAppId } from './appIdentity.js'; +import { PORTOS_APP_ID as clientAppId } from '../../client/src/lib/appIdentity.js'; + +describe('appIdentity — server/client mirror parity', () => { + it('keeps the baseline app id identical', () => { + expect(clientAppId).toBe(serverAppId); + }); +}); diff --git a/server/lib/issueLength.mirror.test.js b/server/lib/issueLength.mirror.test.js new file mode 100644 index 0000000000..b49ae390b9 --- /dev/null +++ b/server/lib/issueLength.mirror.test.js @@ -0,0 +1,55 @@ +import { describe, expect, it } from 'vitest'; +import { + CUSTOM_MINUTE_MAX as serverMinuteMax, + CUSTOM_MINUTE_MIN as serverMinuteMin, + CUSTOM_PAGE_MAX as serverPageMax, + CUSTOM_PAGE_MIN as serverPageMin, + DEFAULT_LENGTH_PROFILE as serverDefaultProfile, + LENGTH_PROFILES as serverProfiles, +} from './issueLength.js'; +import { + CUSTOM_MINUTE_MAX as clientMinuteMax, + CUSTOM_MINUTE_MIN as clientMinuteMin, + CUSTOM_PAGE_MAX as clientPageMax, + CUSTOM_PAGE_MIN as clientPageMin, + DEFAULT_LENGTH_PROFILE as clientDefaultProfile, + LENGTH_PROFILES as clientProfiles, +} from '../../client/src/lib/issueLength.js'; + +describe('issueLength — server/client picker parity', () => { + it('keeps the profiles the client displays aligned with server targets', () => { + const serverPickerProfiles = Object.fromEntries(Object.entries(serverProfiles).map(([id, profile]) => [ + id, + { + label: profile.label, + pageTarget: profile.pageTarget, + minutesTarget: profile.minutesTarget, + }, + ])); + const clientPickerProfiles = Object.fromEntries(Object.entries(clientProfiles).map(([id, profile]) => [ + id, + { + label: profile.label, + pageTarget: profile.pageTarget, + minutesTarget: profile.minutesTarget, + }, + ])); + + expect(clientPickerProfiles).toEqual(serverPickerProfiles); + expect(clientDefaultProfile).toBe(serverDefaultProfile); + }); + + it('keeps every custom-override bound identical', () => { + expect({ + pageMin: clientPageMin, + pageMax: clientPageMax, + minuteMin: clientMinuteMin, + minuteMax: clientMinuteMax, + }).toEqual({ + pageMin: serverPageMin, + pageMax: serverPageMax, + minuteMin: serverMinuteMin, + minuteMax: serverMinuteMax, + }); + }); +}); diff --git a/server/lib/mirrorCoverage.test.js b/server/lib/mirrorCoverage.test.js new file mode 100644 index 0000000000..5f2891bbd8 --- /dev/null +++ b/server/lib/mirrorCoverage.test.js @@ -0,0 +1,187 @@ +import { describe, expect, it } from 'vitest'; +import { readFileSync, readdirSync } from 'fs'; +import { basename, dirname, join } from 'path'; +import { fileURLToPath } from 'url'; + +const here = dirname(fileURLToPath(import.meta.url)); +const CLIENT_LIB = join(here, '../../client/src/lib'); +const CLIENT_README = join(CLIENT_LIB, 'README.md'); +const SERVER_README = join(here, 'README.md'); + +function listTestFiles(dir) { + return readdirSync(dir, { withFileTypes: true }).flatMap((entry) => { + const path = join(dir, entry.name); + if (entry.isDirectory()) return listTestFiles(path); + return entry.name.endsWith('.test.js') ? [path] : []; + }); +} + +// Both catalogs declare mirrors the same way — a backtick-fenced filename in +// column 1, a description in column 2 naming the counterpart path — differing +// only in which side is "this file" vs. "the other file it mirrors". One +// parameterized walker keeps that row-parsing logic from drifting between the +// two catalogs the way the mirrored declarations it guards must not drift. +function listedPairsFor(readme, otherPathRe) { + const rows = [...readme.matchAll(/^\|\s+`([^`]+\.js)`\s+\|\s+(.+)\|$/gm)]; + return rows.flatMap(([, thisFile, description]) => { + if (!/\bmirror/i.test(description)) return []; + const otherMatch = description.match(otherPathRe); + if (!otherMatch || otherMatch[1].includes('/') || thisFile !== basename(otherMatch[1])) return []; + return [{ thisFile, otherFile: otherMatch[1] }]; + }); +} + +function listedMirrorPairs(readme) { + return listedPairsFor(readme, /server\/lib\/([\w/-]+\.js)/) + .map(({ thisFile, otherFile }) => ({ clientFile: thisFile, serverFile: otherFile })); +} + +function listedServerMirrorPairs(readme) { + return listedPairsFor(readme, /client\/src\/lib\/([\w/-]+\.js)/) + .map(({ thisFile, otherFile }) => ({ clientFile: otherFile, serverFile: thisFile })); +} + +function uniquePairs(pairs) { + return [...new Map(pairs.map((pair) => [`${pair.serverFile}:${pair.clientFile}`, pair])).values()]; +} + +// Matches `ref` only when it appears as (part of) a quoted string — i.e. an +// actual import/require specifier — not a bare substring. A prose comment +// mentioning a filename, or an unrelated same-prefix fixture, must not count +// as "this test imports that file". +function importsRef(source, ref) { + const escaped = ref.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + // Quote characters only — no backtick. Backtick-fenced prose is this + // codebase's dominant style for referencing a file path in a comment or + // JSDoc header (see every existing parity-test docstring), so treating it + // as an import specifier would reopen the exact prose-mention bypass this + // helper exists to close. + return new RegExp(`['"][^'"]*${escaped}['"]`).test(source); +} + +function missingParityPins(pairs, testSources) { + return pairs.filter(({ clientFile, serverFile }) => { + const serverName = basename(serverFile); + // For a direct mirror, clientFile and serverName are the identical + // string — so "reads the client copy" and "reads the server copy" can't + // be proven by a plain check on each independently, or the SAME + // occurrence satisfies both (a client-only test that only imports its + // own module via `'./example.js'` would otherwise pass as a valid + // parity pin, and symmetrically for a server-only test — see the + // bypass-probe tests below). Strip whichever string just proved "reads + // the client copy" before checking for server-copy evidence, so the two + // proofs must come from genuinely different occurrences. + const clientPathRef = `client/src/lib/${clientFile}`; + return !testSources.some(({ path, source }) => { + if (path === fileURLToPath(import.meta.url)) return false; + const inClientLib = path.startsWith(CLIENT_LIB); + const readsClient = importsRef(source, clientPathRef) || (inClientLib && importsRef(source, clientFile)); + if (!readsClient) return false; + let remainder = source.split(clientPathRef).join(''); + // Only strip the bare-import specifier when it was actually used as + // client-copy evidence above (inClientLib) — outside client/src/lib + // that same specifier is exactly what proves the SERVER copy was read + // (see catalogTypes.parity.test.js: `from './catalogTypes.js'` + // alongside the full client path), so stripping it unconditionally + // would erase legitimate server-copy evidence. + if (inClientLib) remainder = remainder.split(`'./${clientFile}'`).join('').split(`"./${clientFile}"`).join(''); + return importsRef(remainder, serverName); + }); + }); +} + +describe('declared server/client mirror coverage', () => { + const readme = readFileSync(CLIENT_README, 'utf8'); + const pairs = uniquePairs([ + ...listedMirrorPairs(readme), + ...listedServerMirrorPairs(readFileSync(SERVER_README, 'utf8')), + ]); + const testSources = [...listTestFiles(here), ...listTestFiles(CLIENT_LIB)].map((path) => ({ + path, + source: readFileSync(path, 'utf8'), + })); + + it('finds direct same-name mirror declarations in the client catalog', () => { + expect(pairs).toContainEqual({ clientFile: 'seasonStructure.js', serverFile: 'seasonStructure.js' }); + expect(pairs).toContainEqual({ clientFile: 'shotGrammar.js', serverFile: 'shotGrammar.js' }); + expect(pairs).toContainEqual({ clientFile: 'appIdentity.js', serverFile: 'appIdentity.js' }); + expect(pairs).toContainEqual({ clientFile: 'issueLength.js', serverFile: 'issueLength.js' }); + }); + + it('also includes direct same-name declarations from the server catalog', () => { + expect(pairs).toContainEqual({ clientFile: 'catalogTypes.js', serverFile: 'catalogTypes.js' }); + }); + + it('requires every declared direct mirror to have a test that reads both copies', () => { + const missing = missingParityPins(pairs, testSources); + expect(missing, `missing parity pins: ${missing.map(({ clientFile }) => clientFile).join(', ')}`).toEqual([]); + }); + + it('reports a synthetic declared mirror when no test reads both copies', () => { + const synthetic = listedMirrorPairs('| `example.js` | Mirror of `server/lib/example.js`. |'); + expect(missingParityPins(synthetic, [])).toEqual([ + { clientFile: 'example.js', serverFile: 'example.js' }, + ]); + }); + + it('does not accept a same-name server-only unit test as a parity pin (bypass probe)', () => { + // A direct mirror's clientFile and serverFile are the same string, so a + // plain server-side unit test importing its own module via a bare + // relative path (e.g. `bareUrl.test.js` doing `from './bareUrl.js'`) + // trivially contains both `'./example.js'` and the server filename + // without ever touching the client copy. Pin that this does NOT count. + const synthetic = listedMirrorPairs('| `example.js` | Mirror of `server/lib/example.js`. |'); + const serverOnlyUnitTest = { + path: join(here, 'example.test.js'), + source: "import { thing } from './example.js';\n", + }; + expect(missingParityPins(synthetic, [serverOnlyUnitTest])).toEqual([ + { clientFile: 'example.js', serverFile: 'example.js' }, + ]); + }); + + it('does not accept a same-name client-only unit test as a parity pin (bypass probe)', () => { + // The mirror image of the probe above: a plain client-side unit test + // importing its own module via a bare relative path (e.g. + // client/src/lib/catalogTypes.test.js doing `from './catalogTypes.js'`) + // never touches the server copy. Pin that this does NOT count either. + const synthetic = listedMirrorPairs('| `example.js` | Mirror of `server/lib/example.js`. |'); + const clientOnlyUnitTest = { + path: join(CLIENT_LIB, 'example.test.js'), + source: "import { thing } from './example.js';\n", + }; + expect(missingParityPins(synthetic, [clientOnlyUnitTest])).toEqual([ + { clientFile: 'example.js', serverFile: 'example.js' }, + ]); + }); + + it('does not accept a bare prose mention of the filename as a parity pin (bypass probe)', () => { + // A comment mentioning the server path in passing — without an actual + // import of it — must not satisfy the guard either, or a stray comment + // surviving the deletion of the real parity-pinning test would keep this + // suite silently green. + const synthetic = listedMirrorPairs('| `example.js` | Mirror of `server/lib/example.js`. |'); + const commentOnlyMention = { + path: join(CLIENT_LIB, 'example.test.js'), + source: "import { thing } from './example.js';\n// keep this in sync with server/lib/example.js\n", + }; + expect(missingParityPins(synthetic, [commentOnlyMention])).toEqual([ + { clientFile: 'example.js', serverFile: 'example.js' }, + ]); + }); + + it('does not accept a backtick-fenced prose mention as a parity pin (bypass probe)', () => { + // Backtick-fenced file references are this codebase's dominant docstring + // style (see every existing parity-test header) — a JSDoc comment that + // mentions both paths in backticks, with no real import backing it, must + // not count either. + const synthetic = listedMirrorPairs('| `example.js` | Mirror of `server/lib/example.js`. |'); + const backtickOnlyMention = { + path: join(CLIENT_LIB, 'example.test.js'), + source: 'import { thing } from \'./example.js\';\n// mirrors `server/lib/example.js` in spirit only, no import here.\n', + }; + expect(missingParityPins(synthetic, [backtickOnlyMention])).toEqual([ + { clientFile: 'example.js', serverFile: 'example.js' }, + ]); + }); +}); diff --git a/server/lib/scenePrompt.test.js b/server/lib/scenePrompt.test.js index f05b4d1076..37c3f4c59e 100644 --- a/server/lib/scenePrompt.test.js +++ b/server/lib/scenePrompt.test.js @@ -1,4 +1,7 @@ import { describe, it, expect } from 'vitest'; +import { readFileSync } from 'fs'; +import { dirname, join } from 'path'; +import { fileURLToPath } from 'url'; import { normalizeSlugline, normCharKey, @@ -9,6 +12,11 @@ import { buildScenePrompt, __testing, } from './scenePrompt.js'; +import { compareDeclaration } from './mirrorParity.js'; + +const here = dirname(fileURLToPath(import.meta.url)); +const SERVER_COPY = join(here, 'scenePrompt.js'); +const CLIENT_COPY = join(here, '../../client/src/lib/scenePrompt.js'); describe('scenePrompt — normalizeSlugline', () => { it('collapses em/en/hyphen + punctuation + spaces so equivalent sluglines match', () => { @@ -307,3 +315,31 @@ describe('scenePrompt — buildScenePrompt wardrobe appearances', () => { expect(out).not.toContain('Wearing:'); }); }); + +describe('scenePrompt — server/client mirror parity', () => { + const server = readFileSync(SERVER_COPY, 'utf8'); + const client = readFileSync(CLIENT_COPY, 'utf8'); + const mirroredDeclarations = [ + 'PROMPT_MAX', + 'normalizeSlugline', + 'normCharKey', + 'buildCharByKey', + 'matchSceneCharacters', + 'matchCharactersInText', + 'buildPlaceByKey', + 'matchScenePlace', + 'matchEntriesByCandidates', + 'matchPlacesInText', + 'matchObjectsInText', + 'appendWardrobe', + 'buildScenePrompt', + ]; + + for (const name of mirroredDeclarations) { + it(`keeps ${name} identical`, () => { + const { clientDecl, serverNorm, clientNorm } = compareDeclaration(server, client, name); + expect(clientDecl, `client/src/lib/scenePrompt.js is missing ${name}`).not.toBeNull(); + expect(clientNorm).toBe(serverNorm); + }); + } +}); diff --git a/server/lib/seasonStructure.mirror.test.js b/server/lib/seasonStructure.mirror.test.js new file mode 100644 index 0000000000..17f34d4612 --- /dev/null +++ b/server/lib/seasonStructure.mirror.test.js @@ -0,0 +1,23 @@ +import { describe, expect, it } from 'vitest'; +import { readFileSync } from 'fs'; +import { dirname, join } from 'path'; +import { fileURLToPath } from 'url'; +import { compareDeclaration } from './mirrorParity.js'; + +const here = dirname(fileURLToPath(import.meta.url)); +const SERVER_COPY = join(here, 'seasonStructure.js'); +const CLIENT_COPY = join(here, '../../client/src/lib/seasonStructure.js'); +const MIRRORED_DECLARATIONS = ['pickSeasonCount', 'recommendStructure', 'describeStructure']; + +describe('seasonStructure — server/client mirror parity', () => { + const server = readFileSync(SERVER_COPY, 'utf8'); + const client = readFileSync(CLIENT_COPY, 'utf8'); + + for (const name of MIRRORED_DECLARATIONS) { + it(`keeps ${name} identical`, () => { + const { clientDecl, serverNorm, clientNorm } = compareDeclaration(server, client, name); + expect(clientDecl, `client/src/lib/seasonStructure.js is missing ${name}`).not.toBeNull(); + expect(clientNorm).toBe(serverNorm); + }); + } +}); diff --git a/server/lib/shotGrammar.mirror.test.js b/server/lib/shotGrammar.mirror.test.js new file mode 100644 index 0000000000..11bf69b819 --- /dev/null +++ b/server/lib/shotGrammar.mirror.test.js @@ -0,0 +1,22 @@ +import { describe, expect, it } from 'vitest'; +import { readFileSync } from 'fs'; +import { dirname, join } from 'path'; +import { fileURLToPath } from 'url'; +import { compareDeclaration } from './mirrorParity.js'; + +const here = dirname(fileURLToPath(import.meta.url)); +const SERVER_COPY = join(here, 'shotGrammar.js'); +const CLIENT_COPY = join(here, '../../client/src/lib/shotGrammar.js'); + +describe('shotGrammar — server/client vocabulary parity', () => { + const server = readFileSync(SERVER_COPY, 'utf8'); + const client = readFileSync(CLIENT_COPY, 'utf8'); + + for (const name of ['SHOT_TYPES', 'SCREEN_DIRECTIONS']) { + it(`keeps ${name} identical`, () => { + const { clientDecl, serverNorm, clientNorm } = compareDeclaration(server, client, name); + expect(clientDecl, `client/src/lib/shotGrammar.js is missing ${name}`).not.toBeNull(); + expect(clientNorm).toBe(serverNorm); + }); + } +});