Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 58 additions & 23 deletions .github/scripts/issue-quality-core.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -66,45 +66,78 @@ function isPlaceholderOnlyValue(raw) {
*/
function stripMediaTokens(text) {
if (typeof text !== "string") return "";
// Indented code lines render as literal code in GitHub Markdown. Protect
// them first so neither the HTML nor the Markdown media stripper can
// remove example syntax; restore the lines afterwards.
// Fenced and indented code render literally in GitHub Markdown. Protect
// them first so neither media stripper can remove example syntax. The
// protector deliberately leaves indented children of an unindented HTML
// media block visible: those lines are HTML children, not Markdown code.
const protectedText = protectIndentedCodeLines(text);
const markdownStripped = stripMarkdownImages(stripHtmlMedia(protectedText.text));
const referenceStripped = stripReferenceImages(markdownStripped);
return restoreIndentedCodeLines(referenceStripped, protectedText.lines);
return restoreIndentedCodeLines(referenceStripped, protectedText);
}

/**
* Replace every indented code line (4+ leading spaces or a tab) with a
* placeholder of equal length so media stripping cannot touch it. Returns the
* masked text plus the original lines for restoration.
* Replace fenced code and indented code outside HTML media blocks with opaque
* tokens. Restoration is token-based rather than line-position-based because
* stripping a multiline media block may collapse or remove lines.
*/
function protectIndentedCodeLines(text) {
const lines = [];
let markerPrefix = "\u0000OCX_ISSUE_CODE_";
while (text.includes(markerPrefix)) markerPrefix += "_";
let mediaDepth = 0;
let fence = null;

const mask = (line) => {
const index = lines.push(line) - 1;
return `${markerPrefix}${index}\u0000`;
};

const masked = text.split("\n").map((line) => {
if (/^(?: {4,}|\t)/.test(line)) {
lines.push(line);
return "\u0000" + line.replace(/[^\n]/g, " ").slice(1);
if (fence) {
const closing = new RegExp(`^ {0,3}${fence.char}{${fence.length},}[ \\t]*$`);
if (closing.test(line)) fence = null;
return mask(line);
}

const fenceStart = line.match(/^ {0,3}(`{3,}|~{3,})/);
if (fenceStart) {
fence = { char: fenceStart[1][0], length: fenceStart[1].length };
return mask(line);
}

// Four-space/tab lines inside an active unindented HTML media block are
// child markup or fallback text. Treating them as code would keep an
// otherwise media-only <picture>/<video> block alive.
if (mediaDepth === 0 && /^(?: {4,}|\t)/.test(line)) {
return mask(line);
}
lines.push(null);

mediaDepth = Math.max(0, mediaDepth + htmlMediaDepthDelta(line));
return line;
});
return { text: masked.join("\n"), lines };
return { text: masked.join("\n"), lines, markerPrefix };
}

/**
* Restore masked indented-code lines from their original content. Placeholder
* lines are identified by the leading \u0000 marker and matched positionally.
* Count opening/closing block-media tags on one line. Self-closing tags do not
* create a block. This small scanner is only for deciding whether indentation
* belongs to HTML; stripHtmlMedia remains the authority for removing media.
*/
function restoreIndentedCodeLines(text, lines) {
const out = text.split("\n").map((line, i) => {
if (lines[i] !== null && line.startsWith("\u0000")) {
return lines[i];
}
return line;
});
return out.join("\n");
function htmlMediaDepthDelta(line) {
let delta = 0;
for (const match of line.matchAll(/<(\/)?(picture|video|audio)\b[^>]*>/gi)) {
if (match[1]) delta -= 1;
else if (!/\/\s*>$/.test(match[0])) delta += 1;
}
return delta;
Comment on lines +127 to +133

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Handle HTML media start tags that span multiple lines.

htmlMediaDepthDelta only detects a media tag when its closing > is on the same line. A valid block with a multiline opening tag leaves mediaDepth at zero.

<video
    src="clip.mp4">
    <source src="clip.mp4">
</video>

The indented attribute and child lines are then masked as Markdown code. stripHtmlMedia cannot remove the resulting tokenized block. isMediaOnly returns false, so media-only issue content bypasses normalization.

Track media-tag state across lines. Set media depth when the scanner sees an opening <picture>, <video>, or <audio> tag. Add a regression test for a multiline opening tag with indented children.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/scripts/issue-quality-core.cjs around lines 127 - 133, Update
htmlMediaDepthDelta and its caller to preserve media-tag scanning state across
lines, counting a picture, video, or audio opening tag once its eventual closing
“>” is encountered even when attributes span lines, while retaining correct
closing-tag handling. Add a regression test covering a multiline opening tag
with indented children and verify media-only normalization still succeeds.

}

/** Restore protected code from ordered tokens, independent of line count. */
function restoreIndentedCodeLines(text, protection) {
const escapedPrefix = protection.markerPrefix.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
const token = new RegExp(`${escapedPrefix}(\\d+)\\u0000`, "g");
return text.replace(token, (_match, rawIndex) => protection.lines[Number(rawIndex)] ?? "");
}

/**
Expand All @@ -131,7 +164,9 @@ function stripHtmlMedia(text) {
.replace(/<[^>]+>/g, " ")
.replace(/[\s_*~`]+/g, " ")
.trim();
return innerStripped.length === 0 ? " " : match;
// GitHub's generated placeholders remain empty even when wrapped as
// fallback text inside a media element. Real captions remain intact.
return innerStripped.length === 0 || isPlaceholderOnlyValue(innerStripped) ? " " : match;
},
);
return s;
Expand Down
33 changes: 33 additions & 0 deletions .github/scripts/issue-quality.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -396,12 +396,45 @@ describe("validateIssue - feature", () => {
false,
);
assert.equal(isMediaOnly('<picture><source srcset="x.webp"><img src="x.png"></picture>'), true);
assert.equal(isMediaOnly('<video>No response</video>'), true);
assert.equal(isMediaOnly('<audio> _No response_ </audio>'), true);
assert.equal(clean('<video>No response</video>'), "");
assert.equal(
isMediaOnly('<picture>\n <source srcset="x.webp">\n <img src="x.png">\n</picture>'),
true,
);
assert.equal(
isMediaOnly('<video>\n <source src="clip.mp4">\n Real fallback caption\n</video>'),
false,
);
assert.equal(isMediaOnly('<video src="clip.mp4"></video>'), true);
assert.equal(isMediaOnly('<img src="x.png" />\nCaption text'), false);
assert.equal(isMediaOnly("Some real description."), false);
assert.equal(stripMediaTokens('<img src="x.png" />').trim(), "");
assert.equal(stripMediaTokens('![alt](url "title")').trim(), "");
assert.equal(stripMediaTokens('before ![alt](url) after').replace(/\s+/g, " ").trim(), "before after");

const fencedMediaExample = [
"```html",
"<video>No response</video>",
"```",
].join("\n");
assert.equal(stripMediaTokens(fencedMediaExample), fencedMediaExample);
assert.equal(isMediaOnly(fencedMediaExample), false);

const protectedAroundMedia = [
" ![before](url)",
"<video>",
' <source src="clip.mp4">',
"</video>",
" ![after](url)",
].join("\n");
const strippedAroundMedia = stripMediaTokens(protectedAroundMedia);
assert.ok(strippedAroundMedia.includes(" ![before](url)"));
assert.ok(strippedAroundMedia.includes(" ![after](url)"));
assert.equal(strippedAroundMedia.includes("<video>"), false);
assert.equal(strippedAroundMedia.includes("<source"), false);
assert.equal(strippedAroundMedia.includes("\u0000"), false);
});

it("accepts a concise but actionable feature", () => {
Expand Down
Loading