Skip to content

fix(ci): replace every em dash in tracked source with ASCII --- - #139

Open
d-morrison wants to merge 1 commit into
mainfrom
fix/138-em-dashes-md
Open

fix(ci): replace every em dash in tracked source with ASCII ---#139
d-morrison wants to merge 1 commit into
mainfrom
fix/138-em-dashes-md

Conversation

@d-morrison

@d-morrison d-morrison commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

Fixes the Check Non-Standard Characters check, red on main since 2026-08-24 (run 32768070308). Every PR opened since inherits the failure, which is how this surfaced: the dependabot submodule bump #137 failed (run 33280843342) over files its own diff never touches.

Cause

.github/workflows/check-non-standard-chars.yaml passes no extensions input, so it takes the reusable workflow's default in Morrison-Lab/gha. That default is now .qmd, .R, .md. The .md entry was not there when the caller was written -- its comment still described the pre-migration scope, "curly quotes / en/em dashes in .qmd and .R files" -- so broadening the scope pulled in pre-existing Markdown that had never been scanned.

Approach

Fix the flagged source rather than relax the check, per this repo's own CLAUDE.md ("Don't bypass CI failures ... fix the underlying issue"). Scanning .md is wanted: the lab convention bans non-ASCII punctuation in every tracked source file.

That rationale does not stop at what CI can see, so the sweep covers three groups:

Group Files
Flagged by CI CLAUDE.md, .github/copilot-instructions.md, .github/prompts/quarto-review.prompt.md, .github/rulesets/README.md
Tracked .md the checker skips (its ignored_dirs hard-codes .claude) .claude/commands/new-chapter.md, .claude/commands/render.md, .claude/skills/reprexes/SKILL.md
Outside the checker's extension set entirely .github/scripts/clear-changed-freezer.py, .github/workflows/copilot-setup-steps.yml, .github/workflows/preview.yml, _quarto-website.yml, qwt-reveal-toggle.html

Leaving the last two groups would make a green check read as a clean repository.

All 62 replacements are U+2014 to a spaced ---. That is the lab-wide convention rather than this repo's established practice -- origin/main has no prior spaced triple-hyphen, so this PR is what establishes it here. Every one was a spaced prose dash, so no compound hyphenation changed and no executable code changed. The one in clear-changed-freezer.py is in that module's docstring, which nothing reads programmatically; the one in qwt-reveal-toggle.html sits inside an HTML comment body where it cannot form -->.

The caller's stale scope comment is corrected in the same commit. The extensions input stays unset so the scope keeps tracking gha's default.

Verification

  • gha's own checker at @v2, run against this tree: clean over 17 files. The same script against origin/main reproduces the reported 28 non-standard character(s) in 4 file(s).
  • A scan of every tracked file for all seven banned glyphs (em and en dashes, the four curly quotes, U+00D7): 0 hits over the 79 of 82 tracked entries that are UTF-8 text, the other three being two binaries and the macros submodule. The file count is reported so a zero is distinguishable from a scan that never ran.
  • The edited Python, YAML and HTML files still parse.

Follow-up

The repo has no instrument covering the files outside the checker's extension set, so the five in the third group above were found by hand and nothing stops them recurring. Filed as #140 rather than widened here, since both the extension default and the hard-coded .claude ignore live upstream in Morrison-Lab/gha.

Closes #138

🤖 Generated with Claude Code

`Check Non-Standard Characters` has been red on `main` since 2026-08-24,
and every PR opened since inherits the failure (e.g. #137, whose own diff
touches none of the flagged files).

The caller passes no `extensions` input, so it takes the reusable
workflow's default in Morrison-Lab/gha. That default is now
`.qmd, .R, .md`; `.md` was not in it when the caller was written, so
broadening the scope pulled in pre-existing Markdown that had never been
scanned. Per this repo's own rule, fix the flagged source rather than
relax the check -- scanning `.md` is wanted, since the lab convention
bans non-ASCII punctuation in every tracked source file.

That rationale does not stop at what CI can see, so this replaces U+2014
with the house `---` form in three groups:

- the 4 files CI flagged: CLAUDE.md, .github/copilot-instructions.md,
  .github/prompts/quarto-review.prompt.md, .github/rulesets/README.md;
- 3 tracked Markdown files the checker skips because its `ignored_dirs`
  hard-codes `.claude`: .claude/commands/new-chapter.md,
  .claude/commands/render.md, .claude/skills/reprexes/SKILL.md;
- 5 files outside the checker's extension set entirely:
  .github/scripts/clear-changed-freezer.py,
  .github/workflows/copilot-setup-steps.yml,
  .github/workflows/preview.yml, _quarto-website.yml and
  qwt-reveal-toggle.html.

All 62 were spaced prose dashes, so no compound hyphenation changed and
no executable code changed. Every one sits in a comment or in prose,
except the one in clear-changed-freezer.py, which is in that module's
docstring -- a string literal, but one nothing reads programmatically.
The `---` in qwt-reveal-toggle.html sits inside an HTML comment body
where it cannot form `-->`.

Verified three ways: gha's own checker at @v2 is clean over 17 files; a
scan of every tracked file for all seven banned glyphs (em and en
dashes, the four curly quotes, U+00D7) reports 0 hits over the 79 of 82
tracked files that are UTF-8 text, the other 3 being two binaries and
the `macros` submodule; and the edited Python and YAML files still
parse.

Also corrects the caller's scope comment, which still described the
pre-migration `.qmd`/`.R` behavior.

Closes #138

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings September 10, 2026 18:26

Copilot AI left a comment

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@d-morrison

Copy link
Copy Markdown
Collaborator Author

Claimed by this Claude Code session. Will drive it to a clean verdict.

Posted by Claude Code (AI agent) --- not written by a human.

@d-morrison
d-morrison requested a lite review from Copilot September 10, 2026 18:29

Copilot AI left a comment

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@d-morrison
d-morrison requested a lite review from Copilot September 10, 2026 18:30

Copilot AI left a comment

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://Morrison-Lab.github.io/qwt/pr-preview/pr-139/

Built to branch gh-pages at 2026-09-10 11:30 PDT.
Preview will be ready when the GitHub Pages deployment is complete.

@github-actions

Copy link
Copy Markdown
Contributor

Claude finished reviewView run


Reasonable line lengths, consistent with the rest of the file. This is a thoroughly clean, well-scoped, and precisely verified PR. Every factual claim in the description checked out against the actual code.

Review summary

This PR fixes the Check Non-Standard Characters CI failure by replacing all 62 em dashes (U+2014) in tracked source files with the ASCII --- form, and corrects a stale comment in .github/workflows/check-non-standard-chars.yaml explaining the reusable workflow's current default extension scope.

Verification performed (all claims in the PR description independently checked against the actual PR HEAD commit, not the working tree, since this checkout has CLAUDE.md/.claude/** reverted to the pre-PR state alongside the documented .github/workflows/ fallback):

  • Ran the actual vendored Morrison-Lab/gha@v2 checker (check-non-standard-chars.py) against the PR's HEAD tree: clean, 0 issues over 17 scanned files — matches the PR's claim.
  • Confirmed the checker's real defaults (.qmd, .R, .md) and its hard-coded ignored_dirs (which does include .claude) — both match the PR's stated rationale exactly.
  • Full scan of every tracked file in the PR tree for all seven banned glyphs (curly quotes ×4, en/em dash, multiplication sign): 0 hits, confirming the "clean beyond what CI can see" claim.
  • Programmatically diffed every removed em-dash line against its hunk's added lines: all 61 lines / 62 individual em-dash occurrences map to an exact --- substitution with no other content drift — no accidental edits smuggled in.
  • Verified .github/scripts/clear-changed-freezer.py still compiles (py_compile), and all touched YAML files (including the YAML frontmatter in .github/prompts/quarto-review.prompt.md, where --- appears mid-line in a scalar value) still parse correctly — the mid-string --- is not misinterpreted as a YAML document separator.
  • Confirmed the HTML comment in qwt-reveal-toggle.html still closes correctly (--> intact, no accidental --> formed by the substitution).
  • Cross-checked referenced issues Check Non-Standard Characters red on main: gha default now scans .md, flagging 28 pre-existing em dashes #138, Non-standard-char check has two blind spots: extension set and hard-coded .claude ignore #140, and PR chore(submodule): bump macros from 8ce5d0c to a63f1a0 #137 via gh: all exist, and their descriptions accurately match what's cited in this PR's body (no hallucinated links/citations).
  • The diff for .github/workflows/check-non-standard-chars.yaml, copilot-setup-steps.yml, and preview.yml in the saved PR diff matches what gh pr diff returns, independent of the on-disk fallback copies.

Notes (non-blocking):

  • The PR description calls --- "the house --- form," but a check of origin/main shows no prior file in this repo used a spaced triple-hyphen for em dashes before this PR — there wasn't an established "house style" yet; this PR is effectively the one establishing it. This doesn't affect correctness (the substitution is 1:1 and lossless), and --- is a defensible, common ASCII em-dash convention, so this is just a minor wording overstatement in the description, not a code issue.
  • Scope is appropriately tight: only em-dash characters were touched; no unrelated reformatting, no new dependencies, no build artifacts committed.

No bugs, no CLAUDE.md violations, no lab-manual violations, and no hallucinated content found.

Verdict

Ready for merge

Structured Review Data (JSON)
{
  "schema_version": "1.1",
  "reviewer": "claude",
  "commit_sha": "b0b28bf4a1aea8aa5089063464b29584e4033e6d",
  "verdict": "CLEAN",
  "findings": [],
  "detailed_assessment": "Verified all 62 em-dash-to-triple-hyphen substitutions across thirteen changed files map exactly with no unintended content drift, and confirmed the checker script, YAML parsers, and HTML comment structure all remain valid post-edit.",
  "holistic_assessment": "Requirement to fix the CI check at its source rather than relax it is satisfied, scope stays tightly bound to punctuation-only changes, and referenced issues 138 and 140 accurately document the regression and follow-up work."
}

Reviewed commit: b0b28bf

@github-actions

Copy link
Copy Markdown
Contributor

💰 Cost: $0.8117 (review) — run

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.

Check Non-Standard Characters red on main: gha default now scans .md, flagging 28 pre-existing em dashes

2 participants