Prove the engine pin, and fix the drift job the pin turned red - #77
Merged
Conversation
PART 9 section 4b's withdrawal of the quote attribution (markup-carve/carve#1213) is already implemented here: the engine pin names a carve-js commit that postdates the withdrawal, `convert.ts` maps a `figure` whose target is a `block_quote` to a pandoc `Figure` around the `BlockQuote` with the caption, and `reverse.ts` reads that shape back. What was missing is the part that says so when it stops being true. THE PIN IS CHECKED AGAINST THE INSTALLED TREE, NOT THE MANIFEST. Three files name the engine - the manifest, the lockfile, and `node_modules/.package-lock.json`, which records what npm actually wrote to disk. Nothing read the third, so a checkout whose `node_modules` predates a pin bump ran every engine-facing test against a different engine and reported the same green. Measured on a long-lived checkout of this repo, which held: ``` package-lock.json git+...carve-js.git#3f5dd8cb node_modules/.package-lock.json registry.npmjs.org/...carve-0.1.3.tgz ``` 0.1.3 is the last released tag and predates the withdrawal, so that tree still carried the `block_quote` `attribution` field. The new check in `test/participants.test.mjs` compares the commit named by all three and fails with "run npm ci" when they disagree. Mutated by bumping the manifest and leaving the lockfile behind: exactly one test goes red, and it is this one. Nothing else in the suite can see that drift. THE ENGINE'S OWN SERIALIZED AST IS ASSERTED, not only the converted pandoc tree. Every other assertion about a captioned quote reads what `convert.ts` produced, which cannot tell a correct engine from a stale one the converter compensated for - the failure class markup-carve/carve#755 collects. The new check in `test/ast-json.test.mjs` reads `carveToCarveAst` directly: a `figure` whose target is a `block_quote`, carrying the caption, with no `attribution` anywhere, and an uncaptioned quote as the control that keeps the caption load-bearing. Mutated by installing a pre-withdrawal engine: 20 tests red, this one among them. THE DRIFT JOB WENT RED THE FIRST MORNING AFTER THE PIN LANDED, and said the opposite of the truth. It resolved the dependency spec with `npm view`, which cannot read a git spec at all - it answers "GitFetcher requires an Arborist constructor to pack a tarball" - so an empty resolution reached the branch that reports "resolves to nothing on the registry, an install here is broken" while `npm ci` was installing the pin perfectly well. It did not even get that far: under `set -e -o pipefail` the failing `npm view` killed the step before its own error message ran, so the job failed with an empty log. The spec's shape now decides which comparison is meaningful. A range is compared against the registry as before; a commit is compared against carve-js `main` directly, which is the same "how far behind the language is this repo" question with the registry taken out of the middle. The one failure the git path owns is a commit carve-js does not have, which is a dependency nobody can install and the real version of the error the range path was reporting falsely. Both `npm view` reads tolerate a non-zero exit so the explaining message is what the log shows. Every path was run locally against the extracted step scripts: the current git pin (warns, exit 0), a semver range (resolves, exit 0), a git spec naming a branch rather than a commit (errors), an unresolvable range (errors, with its message), a pinned commit present upstream (reports 8 commits behind), and a pinned commit that is not (errors). Two comments left over from the section 4a era are corrected. The one in `convert.ts` was not merely stale but false about the code directly beneath it: it told a reader that outside a group a captioned quote never reaches the generic figure arm, which is the arm it has taken since the reroute came out. The one in `reverse.ts` pointed at a "section 4a quote branch above" that no longer exists, and the table collapse it was comparing itself to is now the only collapse of its kind. No CHANGELOG entry: nothing a consumer installs behaves differently. The behavior these checks pin was described when it landed.
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
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.
Three rows on the
markup-carve/carve#1210tracker asked for the quote attribution's withdrawal (markup-carve/carve#1213) to be carried through this bridge: bump the engine pin past it, mapfigure{target: block_quote}to a pandocFigure[BlockQuote]with the caption, and delete the attribution handling.All three are already implemented on
main. The pin moved in #70 to a carve-js commit that postdates the withdrawal, and #72 removed the reroute,attributionInlines,attributionFromBlocks, the numbering exception and the serializer probe. The reverse mapping #72 widened is the canonical one and is untouched here. So this PR does not re-do that work; it supplies the part that was missing, which is the part that says so when any of it stops being true.The pin is checked against the installed tree, not the manifest
Three files name the engine:
package.json,package-lock.json, andnode_modules/.package-lock.json, which records what npm actually wrote to disk. Nothing read the third. A checkout whosenode_modulespredates a pin bump therefore ran every engine-facing test against a different engine and reported the same green.That is not hypothetical. A long-lived checkout of this repo held:
0.1.3 is the last released tag and predates the withdrawal, so that tree still carried the
block_quoteattributionfield.test/participants.test.mjsnow compares the commit all three name and fails with "runnpm ci" when they disagree.The engine's own serialized AST is asserted, not only the converted tree
Every other assertion about a captioned quote reads the pandoc tree
convert.tsproduced, which cannot tell a correct engine from a stale one the converter compensated for. That is the failure classmarkup-carve/carve#755collects.test/ast-json.test.mjsnow readscarveToCarveAstdirectly: afigurewhose target is ablock_quote, carrying the caption, with noattributionanywhere, plus an uncaptioned quote as the control that keeps the caption load-bearing rather than incidental.The Carve to pandoc to Carve invariant in
test/equivalence.test.mjsalready covers this case twice, as> quote\n^ Authorand as{#fig .fancy}\n> quote\n^ Author. Both are among the tests that redden when the pin regresses.The drift job went red the morning after the pin landed, and said the opposite of the truth
engine-drift.ymlresolved the dependency spec withnpm view, which cannot read a git spec at all. It answersGitFetcher requires an Arborist constructor to pack a tarball, so the empty resolution reached the branch reporting "resolves to nothing on the registry, an install here is broken" whilenpm ciwas installing the pin perfectly well. It did not even reach that message: underset -e -o pipefailthe failingnpm viewkilled the step first, so run 31929226732 failed with an empty log.The spec's shape now decides which comparison is meaningful. A range is compared against the registry as before. A commit is compared against carve-js
maindirectly, which is the same "how far behind the language is this repo" question with the registry taken out of the middle. The one failure the git path owns is a commit carve-js does not have, which is a dependency nobody can install and the honest version of the error the range path was reporting falsely. Both registry reads now tolerate a non-zero exit so the explaining message is what the log shows.Two comments from the section 4a era
The one in
convert.tswas not merely stale but false about the code directly beneath it: it told a reader that outside a group a captioned quote never reaches the generic figure arm, which is the arm it has taken since the reroute came out. The one inreverse.tspointed at a "section 4a quote branch above" that no longer exists.Not done, deliberately
The engine pin is not moved forward. It already sits past the withdrawal, and carve-js
mainhas since gained table cell padding and escaping changes that would land unrelated output churn in this diff. The drift job now warns about the distance rather than hiding it.No CHANGELOG entry: nothing a consumer installs behaves differently, and the behavior these checks pin was described when it landed.