test: pin declared mirror parity (#4175) - #4279
Merged
Merged
Conversation
Owner
Author
|
Review gate remains unsatisfied: three headless Antigravity attempts returned no usable code-review verdict (one unrelated flag explanation, then two empty responses). The implementation and server suite are green; leaving this PR open for a reviewer retry rather than merging without the configured review. |
atomantic
force-pushed
the
claim/issue-4175
branch
3 times, most recently
from
August 15, 2026 14:48
c993f22 to
29108dc
Compare
…lper Both functions parsed README mirror-declaration table rows with identical row-matching and mirror-description-filter logic, differing only in which side (client vs server) was "this file" vs. "the other file it mirrors" — exactly the duplication mirrorParity.js exists to prevent one file over.
The bare './<file>' fallback matched any test file's source containing that literal substring, without checking WHERE the test lived. For a direct mirror, clientFile and serverFile are the same string, so a plain server-only unit test (e.g. bareUrl.test.js importing './bareUrl.js') trivially satisfied both substring checks and was wrongly counted as a parity pin — verified by simulating removal of bareUrl.mirror.test.js, which left missingParityPins reporting no gap. Restrict the './<file>' form to test files that actually live under client/src/lib, and add a bypass-probe test pinning the fix.
…tyPins clientFile and serverName are the identical string for a direct mirror, so the unconditional `source.includes(serverName)` check was implied by the SAME occurrence that already proved "reads the client copy" via the bare `'./<file>'` branch. A plain client-only unit test (e.g. catalogTypes.test.js importing only './catalogTypes.js') was therefore wrongly counted as a parity pin — verified in isolation. Fix strips whichever string proved client-copy evidence before checking for independent server-copy evidence, taking care not to strip the bare import when it's server-side (that's the only evidence catalogTypes.parity.test.js has for the server copy). Verified against every declared mirror pair in the real repo and all 41 mirror/parity test files still pass; added a matching bypass-probe test.
…arityPins
Every prior check was a bare substring test, so a stray comment
mentioning a mirrored filename (or an unrelated same-prefix fixture)
could satisfy "reads the client/server copy" without any real import
existing — verified in isolation: a client-only test whose only server
reference was a trailing comment ("// keep this in sync with
server/lib/example.js") was wrongly accepted as a parity pin. Replace
every substring check with importsRef(), which requires the filename to
appear inside an actual quoted specifier. This also let the server-side
bare-import branch collapse into the same check, since a quoted
'./file.js' specifier now matches directly. Verified against every
declared pair in the real repo and all 41 mirror/parity test files
(600 tests) still pass; added a third bypass-probe test for the
comment-only case.
Backtick-fenced file paths are this codebase's dominant docstring style
for referencing a mirror's counterpart (every existing parity-test
header does this, e.g. personaTraitBlend.parity.test.js), so accepting
backtick as an import-specifier delimiter reopened the exact
prose-mention bypass the prior fix closed, just via backticks instead
of bare text — verified: a JSDoc comment merely mentioning both paths
in backticks, with no real import, was wrongly accepted. Restrict the
quote class to real string-literal delimiters ('/") only, which is
what every actual import/require/readFileSync path in this codebase
uses. Verified against all declared pairs in the real repo; added a
matching bypass-probe test.
atomantic
force-pushed
the
claim/issue-4175
branch
from
August 15, 2026 15:00
3093aaf to
36bd97d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Testing
NODE_ENV=test npm test(server)Closes #4175