refactor(tooling): round-trip the repinned review JSON

Preserving byte-exact formatting required a hand-rolled JSON scanner: string
escape handling, container nesting, and span splicing, for a file that lives in
gitignored .local/ and is only ever read back through JSON.parse.

repin already parses the artifact to validate it, so assign the two identity
fields and re-serialize. JSON.parse/stringify keeps insertion order, which is
the only formatting property worth holding. Drops four helpers.
This commit is contained in:
Peter Steinberger
2026-07-28 10:34:44 -07:00
parent 2d4c0bc69a
commit c6304eb539
2 changed files with 14 additions and 96 deletions

View File

@@ -534,81 +534,6 @@ function readJson(filePath) {
}
}
function skipJsonWhitespace(source, start) {
let index = start;
while (/\s/u.test(source[index] ?? "")) {
index += 1;
}
return index;
}
function jsonStringEnd(source, start) {
for (let index = start + 1; index < source.length; index += 1) {
if (source[index] === "\\") {
index += 1;
} else if (source[index] === '"') {
return index + 1;
}
}
throw new Error("Invalid JSON string while locating review identity");
}
function jsonValueEnd(source, start) {
const first = source[start];
if (first === '"') {
return jsonStringEnd(source, start);
}
if (first === "{" || first === "[") {
const openings = [first];
for (let index = start + 1; index < source.length; index += 1) {
const character = source[index];
if (character === '"') {
index = jsonStringEnd(source, index) - 1;
} else if (character === "{" || character === "[") {
openings.push(character);
} else if (character === "}" || character === "]") {
openings.pop();
if (openings.length === 0) {
return index + 1;
}
}
}
throw new Error("Invalid JSON container while locating review identity");
}
let index = start;
while (index < source.length && !/[\s,}\]]/u.test(source[index])) {
index += 1;
}
return index;
}
function findJsonPropertyValue(source, objectStart, property) {
let index = skipJsonWhitespace(source, objectStart + 1);
let found;
while (source[index] !== "}") {
const keyStart = index;
const keyEnd = jsonStringEnd(source, keyStart);
const key = JSON.parse(source.slice(keyStart, keyEnd));
index = skipJsonWhitespace(source, keyEnd);
if (source[index] !== ":") {
throw new Error("Invalid JSON object while locating review identity");
}
const valueStart = skipJsonWhitespace(source, index + 1);
const valueEnd = jsonValueEnd(source, valueStart);
if (key === property) {
found = { start: valueStart, end: valueEnd };
}
index = skipJsonWhitespace(source, valueEnd);
if (source[index] === ",") {
index = skipJsonWhitespace(source, index + 1);
}
}
if (found) {
return found;
}
throw new Error(`Missing ${property} while locating review identity`);
}
function repinReviewArtifacts({ expectedPrNumber, reviewPath, reviewMarkdownPath, prMetaPath }) {
if (!existsSync(prMetaPath)) {
throw new Error(`Missing ${prMetaPath}; run scripts/pr review-init <PR>.`);
@@ -631,28 +556,18 @@ function repinReviewArtifacts({ expectedPrNumber, reviewPath, reviewMarkdownPath
);
}
const reviewSource = readFileSync(reviewPath, "utf8");
const review = readJson(reviewPath);
if (!isObject(review) || !isObject(review.pr)) {
throw new Error(
`Invalid PR identity in ${reviewPath}; run scripts/pr review-artifacts-init ${expectedPrNumber}.`,
);
}
const objectStart = skipJsonWhitespace(reviewSource, 0);
const prSpan = findJsonPropertyValue(reviewSource, objectStart, "pr");
const numberSpan = findJsonPropertyValue(reviewSource, prSpan.start, "number");
const headShaSpan = findJsonPropertyValue(reviewSource, prSpan.start, "headSha");
const replacements = [
{ ...numberSpan, value: String(prMeta.number) },
{ ...headShaSpan, value: JSON.stringify(prMeta.headRefOid) },
].toSorted((left, right) => right.start - left.start);
let nextReviewSource = reviewSource;
for (const replacement of replacements) {
nextReviewSource =
nextReviewSource.slice(0, replacement.start) +
replacement.value +
nextReviewSource.slice(replacement.end);
}
// Round-trip rather than splice the source: JSON.parse/stringify keeps insertion
// order, and `.local/` artifacts are gitignored scratch that only ever gets read
// back through JSON.parse, so byte-exact formatting buys nothing.
review.pr.number = prMeta.number;
review.pr.headSha = prMeta.headRefOid;
const nextReviewSource = `${JSON.stringify(review, null, 2)}\n`;
const markdownSource = readFileSync(reviewMarkdownPath, "utf8");
const firstNewline = markdownSource.indexOf("\n");

View File

@@ -437,11 +437,14 @@ describePosix("scripts/pr review artifact validation", () => {
const { result, localDir, reviewSource, markdownBody } = runArtifactsRepin();
expect(result.status, `${result.stdout}\n${result.stderr}`).toBe(0);
const repinnedReview = readFileSync(join(localDir, "review.json"), "utf8");
const expectedReview = reviewSource
.replace(`"number": ${SUPERSEDED_PR}`, `"number": ${REVIEWED_PR}`)
.replace(`"headSha": "${SUPERSEDED_HEAD}"`, `"headSha": "${REVIEWED_HEAD}"`);
expect(repinnedReview).toBe(expectedReview);
const repinned = JSON.parse(readFileSync(join(localDir, "review.json"), "utf8"));
const expected = JSON.parse(reviewSource);
expected.pr.number = REVIEWED_PR;
expected.pr.headSha = REVIEWED_HEAD;
// Deep-equal, so every authored field outside `pr` must survive verbatim.
expect(repinned).toEqual(expected);
// Key order matters for readability of an artifact humans hand-edit.
expect(Object.keys(repinned)).toEqual(Object.keys(expected));
expect(readFileSync(join(localDir, "review.md"), "utf8")).toBe(
`${REVIEWED_IDENTITY_LINE}\n${markdownBody}`,
);