Skip to content

fix(ci): scope PR evidence to visible sections - #318

Open
aaroncoville wants to merge 1 commit into
HarnessMD:mainfrom
aaroncoville:fix/pr-evidence-section-capture
Open

aaroncoville wants to merge 1 commit into
HarnessMD:mainfrom
aaroncoville:fix/pr-evidence-section-capture

Conversation

@aaroncoville

@aaroncoville aaroncoville commented Aug 25, 2026 •

Copy link
Copy Markdown

What & why

A pull request that follows this repository's own template is rejected by the evidence check.

PULL_REQUEST_TEMPLATE.md puts an instructional comment and a blank line under ### Before and ### After. The check strips comments before matching, which leaves the blank line, and the section regex ends its lazy capture at $, which under the m flag matches the first line end it reaches.

In practice the check only reads the single line directly beneath each heading.

Two corrections. Terminate the capture at the next heading line start or at true end of input, so an immediately adjacent heading ends a section at width zero and an empty heading cannot be satisfied by the media under a later one. And strip fenced code blocks alongside HTML comments: test output pasted under a heading otherwise ends the section at its first # line and hides an image below it, and Markdown that looks like media inside a fence otherwise counts as evidence though it never renders.

Evidence still has to sit under its own heading, a section stops where the next one starts, and a heading with nothing under it still fails.

Type of change

  • Bug fix
  • New feature
  • Refactor / cleanup
  • Docs
  • Build / CI

Evidence

The check's own section extraction, run over a body in the shape the template produces — heading, instructional comment, blank line, then the image — alongside the cases that must keep failing.

Before

CleanShot 2026-08-25 at 15 02 28@2x

Both sections come back empty, so a description that follows the template is reported as having no evidence.

After

CleanShot 2026-08-25 at 15 03 04@2x

The same body is read correctly, while an empty heading beside an adjacent one, media in a later section, and media inside a code fence are all still refused.

How I tested it

  • OS: macOS
  • Ran the workflow's extraction over the template shape, a heading immediately followed by another heading with the media under the second, media present only in a later section, media inside a fenced block, test output fenced above a real image, a section ending at end of input, and a heading with nothing under it. Only the template shape and the fenced-output-plus-image case are accepted; the rest are refused.
  • npm run typecheck, npm run test:focused (552/552), npm run build.

The evidence check reads each section with a lazy capture that ends at `$`
under the `m` flag, so it stops at the first line end it reaches. A heading
followed by a blank line yields an empty section, and in practice only the
single line directly beneath a heading is ever read. The pull request template
puts an instructional comment and a blank line under `Before` and `After`, and
comments are stripped before matching — so a description that follows the
template exactly is reported as having no evidence, and the failure comment
asks the author to do what they already did.

Stop the capture at the next heading line start or at true end of input. An
immediately adjacent heading then ends a section at width zero, so an empty
heading cannot be satisfied by the media under a later one.

Strip fenced code blocks alongside HTML comments. Test output pasted under a
heading otherwise ends the section at its first `#` line, hiding an image
below it, and Markdown that looks like media inside a fence otherwise counts
as evidence though it never renders as media.

Evidence still has to sit under its own heading: a section stops where the
next one starts, and a heading with nothing under it still fails.
@github-actions

github-actions Bot commented Aug 25, 2026 •

Copy link
Copy Markdown
Contributor

✅ Evidence received. Before and after are both attached. Thanks — this is what makes a PR reviewable in one pass.

@aaroncoville
aaroncoville marked this pull request as ready for review August 25, 2026 19:04
@chaitanyagiri

Copy link
Copy Markdown
Collaborator

Leaving this open on purpose, with an explanation.

Your diagnosis was right and it shipped in v0.4.6, but your patch did not, and the difference matters.

96c64456 fixed the section terminator in .github/workflows/pr-evidence.yml, changing $ to (?![\s\S]) the way you identified. It was re-typed rather than taken from here, so it kept \n#{1,6} where you had moved to ^#{1,6}.

What it did not take is your first hunk, the one that strips fenced code blocks before the visible body is scanned. The release still reads:

const visible = body.replace(/<!--[\s\S]*?-->/g, '');

So a fenced block in a PR description can still be read as evidence. That half of your fix is still outstanding, which is why this stays open rather than being closed as shipped.

The branch no longer applies cleanly against the release, so it needs a rebase before it can go anywhere.

https://github.com/chaitanyagiri/munder-difflin/releases/tag/v0.4.6

Thank you for the diagnosis, and sorry we took the idea without taking the patch.

mrlfarano added a commit to mrlfarano/munder-difflin that referenced this pull request Aug 31, 2026
# Conflicts:
#	.github/workflows/pr-evidence.yml
@chaitanyagiri

Copy link
Copy Markdown
Collaborator

Reviewed by an agent on the Munder Difflin hive floor, posted from @chaitanyagiri.
First person statements below are the agent's, including the scope limits.
Replies here are read. Questions in this comment are real questions.
Base: origin/main at 956bfb4.


The $ to (?![\s\S]) change is the real fix and the PR could say so louder. The old lookahead
was (?=\n#{1,6}\\s|$) compiled with 'im', and under m the $ matches at every line end,
so a section stopped at its own first line break. Every evidence section was being read one line
deep. (?![\s\S]) is true end of input and fixes it.

The fenced block strip is the one I would ask about. This line is new:

.replace(/^(```|~~~)[\s\S]*?^\1\s*$/gm, '')

The line above it carries three lines of comment explaining why HTML comments are stripped, and
this one carries none. In this file the comments are the reason, and a reader six months from
now cannot tell whether fenced blocks are stripped to stop a template's example from passing the
check, or by accident. Worth one sentence.

Two behaviours worth confirming while you are in there: a fence opened with four backticks does
not match the three backtick alternation, and an unclosed fence matches nothing at all, so in
both cases the block stays visible and still counts. If the point is that a code block is never
evidence, both of those are holes.

Also, this conflicts. The API reports mergeable: false on it today.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants