docs(tests): correct the seam guard's prose, and the false absolutes each fix introduced (#4940)

* docs(tests): correct the seam guard's header and cut what argues rather than informs

The header claimed "you cannot call the function without importing the module it
lives in". A consumer reached through a re-exporting barrel matches neither half
of the detector; what saves the ledger is that the BARREL matches the `from`
clause and joins it, one file away from the consumer that gates. Stated, with
the limit, because a reader trusting the absolute would stop looking.

Cuts the dated "85/85 green" count, the change-log narration of what the ledger
used to be, the summary of the assertions below it, and a clause arguing the
guard is correct. The mutants are the proof now; the header does not need to
make the case.

Comment-only: the diff contains no non-comment lines.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs(tests): restore the truncated clause on the corpus-loop control

The comment above `expectedCorpus` ended mid-sentence, dropping the half that
explains why scoping the corpus loop is green: no detector fixture can observe
that loop at all, because `verdictFor` seeds `SOURCE` directly and runs past it;
and every corpus member already carries the token such a filter would scope by,
so the filter excludes nothing. Green there means inert, not caught, and the
re-derivation below is what would actually disagree.

Comment-only: the diff contains no non-comment lines.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs(tests): correct the detector limit and move it onto the code it constrains

The header said a consumer reached through a re-exporting barrel matches neither
half of the detector. It matches the symbol half: a name-preserving re-export
still writes `classifyGatedImageForViewer(`, so the consumer joins the ledger at
its own path. Only a barrel hop that also renames escapes both. That mattered
more than wording, because the barrel case is the symbol half's ONLY unique
contribution - an alias, a namespace import and a re-export from the logic module
all carry the `from` clause - so the header handed a future tidier an argument
for deleting it.

The corrected statement lives on `isCallSite` rather than in the header, where it
sits beside the expression it describes instead of drifting from it. The same
docblock justified the symbol half as catching a namespace import or a re-export,
both of which the import half already catches.

The header keeps the rule and drops what restated it: the detection mechanism
(stated twice more, on `LOGIC_MODULE_IMPORT` and `isCallSite`), the paragraph
arguing a per-file suite could not catch this, and a summary of two sibling
suites' assertions - which also over-claimed, since the grid withholds an unrated
image's url from its author too when the image is flagged or scan-refused.

The corpus-loop comment now states the fact rather than the mutation-testing
note: every corpus member's path contains the token, so narrowing the loop by it
excludes nothing.

Comment-only: the diff contains no non-comment lines.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs(tests): state the detector's escape shapes as a property, not a list

The previous round's docblock said only a renaming barrel hop escapes both
halves. It does not. A renaming dynamic import escapes both with no barrel
anywhere: `await import('…logic')` carries no `from` clause for the import half,
and a renamed destructure puts a `:` where the symbol half needs `(`. The file
still enters the corpus, is scanned, and comes back not-a-call-site. A helper
handed the function as a value escapes the same way.

That distinction is the safety-relevant part and it is now stated: a renaming
barrel hop reddens this suite at the barrel, while a renaming `import()` or a
helper reddens nothing at all. Written as a property of what escapes rather than
an enumeration of shapes, so finding a fourth shape does not make it false again.

Restores the clause saying each file type-checks and each file's own suite passes.
It was cut as self-justification, but it is the only statement of why the two
per-consumer suites cannot substitute for this one, it lives nowhere else, and
being about the nature of cross-file defects rather than about any code, it cannot
drift.

Comment-only: the diff contains no non-comment lines.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs(tests): state both halves as spellings, and stop claiming the per-file suites are blind

Two false statements, both introduced by the previous two rounds of this branch.

The restored clause said each file type-checks and each file's own suite passes,
so this is the class of defect no per-file suite can see. Not true of the tree as
it stands: `block-post.service.test.ts` has an `it.each` whose first two rows are
`{ ingestion: 'Pending' }` and `{ nsfwLevel: 0 }`, both reaching the gate at
`block-post.service.ts:592`, so rewriting that gate as `=== 'hidden'` fails them.
It was true when the seam was created and stopped being true when those cases
were written. The narrower statement is the one that does not rot: neither file is
wrong on its own, so nothing fails until someone writes a per-consumer case for
the new state - which is exactly what a third consumer would not have.

The escape condition said a file escapes both halves by naming neither the module
nor the symbol. A renaming `import()` names the module in full and escapes anyway,
because the import half keys on a `from` clause rather than on the module's name -
so the condition excluded a case the same paragraph listed two clauses later.

Both halves are now stated as what they are, spellings, with the `from` forms left
to the regex's own docblock instead of restated fifty lines away. Every false
absolute on this file has been a claim about that regex written far from it.

Comment-only: the diff contains no non-comment lines.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs(tests): cut the detector's consequence claims, and make its referent exact

The docblock claimed that when an escape shape lands in an already-ledgered file,
the `status !== 'visible'` containment still holds the line. False for half the
ledger: `targets` is `EXPECTED_CALL_SITES` minus `MAY_BRANCH_ON_HIDDEN`, pinned by
name to `block-post.service.ts` alone, so the grid projection is ledgered with no
content assertion against it at all. The claim read as a backstop that does not
exist for the one consumer allowed to branch on `=== 'hidden'`.

Its companion - that a barrel hop still reddens at the barrel - was unconditional
in the same way: a barrel re-exporting via an extension the regex does not list
matches neither half itself, so that hop reddens nowhere either.

Both are deleted rather than qualified. This is the fifth false statement in this
docblock in four rounds, every one of them a consequence claim about a text
matcher; a deletion is the only edit here that cannot produce a sixth. What
remains is the part that has survived every round: each half pins a spelling, and
a file writing neither is not a call site.

That sentence defers to `LOGIC_MODULE_IMPORT`'s own docblock for the forms, which
makes it load-bearing, and it under-described them - it named the rooted, relative
and extensionless spellings while the regex also accepts `.ts`, `.tsx`, `.js` and
`.jsx`. A reader following the pointer to check a `.js` specifier was told by
implication it was not covered. Now stated exactly, with its closed end.

Comment-only: the diff contains no non-comment lines.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Justin Maier
2026-09-18 11:41:10 -06:00
committed by GitHub
parent e50d122cb2
commit 0340f692bf
@@ -12,36 +12,12 @@ import { stripSourceComments } from '~/components/AppBlocks/stripSourceComments'
* for everyone else. Every OTHER consumer must keep treating anything that is not
* `visible` as a refusal, and must do so by spelling the test `!== 'visible'`: a
* gate written `=== 'hidden'` was correct while the verdict had two members and
* silently ADMITS a `pending` image now. That is not a bug in either file on its
* own — each one type-checks, each one's unit tests pass — so it is exactly the
* class of defect a per-file suite cannot see.
*
* This asserts the RELATIONSHIP, not a component:
* 1. the exact SET of call sites (fails when it GROWS *or* SHRINKS, so a new
* consumer cannot join without a human deciding what it does with `pending`),
* 2. that no call site outside the projection gates on `=== 'hidden'`.
*
* 🔴 (1) IS DETECTED BY THE IMPORT SPECIFIER, NOT BY THE SYMBOL'S SPELLING, AND
* THAT IS THE WHOLE POINT. It used to be `source.includes('classifyGatedImageForViewer(')`
* — a SPELLED check, walkable by writing the thing a different way. A third
* consumer added as
*
* import { classifyGatedImageForViewer as classify } from '…block-gated-images.logic';
* if (classify(row, level).status === 'hidden') { … } // ADMITS `pending`
*
* is a real bypass of both assertions below, and this suite reported 85/85 green
* over it. Binding the ledger to `from '…block-gated-images.logic'` pins the
* thing that cannot be renamed away: you cannot call the function without
* importing the module it lives in. The symbol-spelling test is KEPT as a second,
* differently-failing route (a namespace import, a re-export) rather than
* replaced — a file matching EITHER is a call site.
* silently ADMITS a `pending` image now. Neither file is wrong on its own, so nothing
* fails until someone writes a per-consumer case for the new state: the two ledgered
* consumers have one, and a third would not.
*
* The BEHAVIOURAL half lives with each consumer and is deliberately not duplicated
* here: `block-post.service.test.ts` proves the public-Post adoption gate refuses
* a `Pending`-ingestion and an unrated (`nsfwLevel: 0`) image, and
* `block-gated-images.service.test.ts` proves the grid withholds the url from
* every viewer but the image's own author. A structural check alone would
* type-check past a wrong argument; those two are what make it mean something.
* here — see `block-post.service.test.ts` and `block-gated-images.service.test.ts`.
*/
const SRC = resolve(__dirname, '../../../..'); // …/src
@@ -52,10 +28,11 @@ const DEFINITION = 'server/services/blocks/block-gated-images.logic.ts';
/**
* An `import … from '<anything>/block-gated-images.logic'` — the alias-proof half
* of the detection. Matches the `~/`-rooted, relative and extensionless spellings
* alike, and (because the source is comment-stripped first) cannot be satisfied by
* prose that merely names the module. `export … from` is matched by the same
* `from` clause, so a re-export is a call site too.
* of the detection. Matches any prefix, `~/`-rooted or relative, with no extension
* or a `.ts`/`.tsx`/`.js`/`.jsx` one and nothing else, and (because the source is
* comment-stripped first) cannot be satisfied by prose that merely names the module.
* `export … from` is matched by the same `from` clause, so a re-export is a call
* site too.
*/
const LOGIC_MODULE_IMPORT = /from\s*['"][^'"]*block-gated-images\.logic(?:\.[jt]sx?)?['"]/;
@@ -123,9 +100,11 @@ for (const full of PRODUCTION_FILES) {
}
/**
* A file is a call site if it IMPORTS the logic module (alias-proof) OR names the
* symbol in call position (namespace import / re-export). Either alone is
* walkable; the union is what the ledger asserts.
* A file is a call site if it IMPORTS the logic module or names the symbol in call
* position. Each half pins a SPELLING: a `from` clause in the forms listed on
* `LOGIC_MODULE_IMPORT`, and the literal `SYMBOL(`. Escaping both takes a file that
* writes neither — reaching the module some way that regex does not list (a barrel, an
* `import()`, an unlisted extension) AND reaching the function under another name.
*/
function isCallSite(rel: string): boolean {
if (rel === DEFINITION) return false;
@@ -159,9 +138,10 @@ describe(`${SYMBOL} seam`, () => {
it('the file walk actually reaches the module under test', () => {
const rels = [...SOURCE.keys()];
expect(rels).toContain(DEFINITION);
// `verdictFor` writes straight into `SOURCE`, so no detector fixture can observe the corpus
// loop narrowing by path — and every real corpus member carries the token `blocks`, so no
// Re-derive from the same walk instead: a filter added to the LOOP makes the two disagree.
// `verdictFor` seeds `SOURCE` directly, so no detector fixture runs the corpus loop and none
// can observe it narrowing by path. Every corpus member's path also contains the token
// `blocks`, so narrowing the loop by that token excludes nothing and stays green. Re-derive
// from the same walk instead: a filter added to the LOOP makes the two disagree.
// Both shared inputs are pinned elsewhere — `PRODUCTION_FILES` by the enumeration equality
// above, `couldBeCallSite` by its own case — so deleting either makes this one vacuous.
const expectedCorpus = PRODUCTION_FILES.filter((full) =>