mirror of
https://github.com/civitai/civitai.git
synced 2026-09-20 22:08:18 +08:00
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:
@@ -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) =>
|
||||
|
||||
Reference in New Issue
Block a user