check-new-line-breaks: add clause-break check (SemBr rule 5), on by default

.github/workflows/_selftest.yml

@@ -28,7 +28,7 @@
       contents: read
     steps:
       - name: Check out repository
         uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
       - name: Lint YAML
         uses: ./lint-yaml
   lint-markdown:
@@ -38,7 +38,7 @@
       contents: read
     steps:
       - name: Check out repository
         uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
       - name: Lint Markdown
         uses: ./lint-markdown
   lint-qmd:
@@ -48,7 +48,7 @@
       contents: read
     steps:
       - name: Check out repository
         uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
       - name: Lint QMD prose
         uses: ./lint-qmd
         with:
@@ -66,7 +66,7 @@
       contents: read
     steps:
       - name: Check out repository
         uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
       - name: Set up R
         uses: r-lib/actions/setup-r@d3c5be51b12e724e68f33216ca3c148b66d5f0b6 # v2.12.1
         with:
@@ -157,6 +157,24 @@
         uses: ./check-new-line-breaks
         with:
           base-ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || '' }}
+      # The clause check (#336) is on by default, so the step above already
+      # covers it. This one covers the opt-out, and covers exactly two things:
+      # that `action.yml` parses, and that the clause_breaks=False path runs
+      # to completion. It does NOT pin that the input is declared: an
+      # undeclared composite input is only an Actions warning, so this step
+      # stays green either way. Declaration is pinned by the
+      # defaults-agreement test, which fails outright when the input has no
+      # `default:`. It cannot FAIL on
+      # findings -- neither step sets `fail:`, and main() returns 0 on every
+      # path unless NLB_FAIL is set -- so it says nothing about whether the
+      # value reaches the script. That is pinned instead by the env-var ->
+      # main() -> exit-code cases in check-new-line-breaks/tests/, which make
+      # the exit code depend on it (gha#337 review).
+      - name: Check again with the clause check opted out
+        uses: ./check-new-line-breaks
+        with:
+          base-ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || '' }}
+          clause-breaks: 'false'
   # The new-line-breaks job above proves the action runs against this repo's
   # own tree; this job unit-tests the sentence-splitter/block-detector
   # functions and the diff-scoping behavior directly (small throwaway git

.github/workflows/check-new-line-breaks.yml

@@ -22,6 +22,21 @@ on:
         description: Fail the workflow when a violation is found (else warn only).
         type: boolean
         default: false
+      clause-breaks:
+        description: >-
+          Also flag a long line carrying a mid-line semicolon (a narrow slice
+          of SemBr rule 5), on top of the rule 4 sentence check that always
+          runs. On by default; set false to check sentences only.
+        type: boolean
+        default: true
+      clause-min-length:
+        description: >-
+          Minimum visible line length before `clause-breaks` applies;
+          inclusive. Measured after stripping inline markup such as code
+          spans, link targets, bare URLs, and HTML entities. Ignored when
+          `clause-breaks` is false.
+        type: string
+        default: '80'
 
 jobs:
   check-new-line-breaks:
@@ -43,6 +58,8 @@ jobs:
           globs: ${{ inputs.globs }}
           paths-ignore: ${{ inputs.paths-ignore }}
           fail: ${{ inputs.fail }}
+          clause-breaks: ${{ inputs.clause-breaks }}
+          clause-min-length: ${{ inputs.clause-min-length }}
           # On PRs, check only lines added relative to the base; otherwise
           # (push) there's no base to diff against, so the check is skipped.
           base-ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || '' }}

CLAUDE.md

@@ -388,6 +388,66 @@
 repo's own tree, the same "local composite, not yet the `@v1`-pinned
 reusable-workflow chain" precedent `phi` uses above.
 
+The suite also covers the gha#336 clause check (a long line carrying a
+mid-line semicolon, as a proxy for SemBr rule 5), including that it is
+**on by default** -- and pins the two defaults that are declared in three
+places at once.
+`_DEFAULT_CLAUSE_BREAKS`/`_DEFAULT_CLAUSE_MIN_LENGTH` in the script are the
+single source, but `action.yml` and
+`.github/workflows/check-new-line-breaks.yml` each re-declare them for their
+own inputs, so a parametrized test reads both YAML files and asserts they
+agree with the script -- the same gha#303 precedent that pinned
+`generate-altdoc-landing-page`'s `site-root` default rather than leaving it
+to a comment.
+The first draft of #336 proved why: `find_violations()` kept a stale `False`
+default while `classify_line()` and `main()` had moved to `True`, and only
+the test caught the drift.
+That test parses the YAML with a line scan rather than a YAML library,
+because the `new-line-breaks-tests` job installs only pytest.
+
+**A selftest step that sets no `fail:` cannot prove the input reached the
+script, however it is worded.**
+`_selftest.yml`'s `new-line-breaks` job does call the composite a second time
+with `clause-breaks: 'false'`, and that is worth having as a real `uses:`
+exercise -- but `main()` returns 0 on every path unless `NLB_FAIL` is set, so
+the step stays green whether the input arrives, is dropped, or was never
+declared at all (an undeclared composite input is only an Actions warning).
+What actually pins the `env var -> main() -> exit code` path is a set of
+pytest cases that set `NLB_FAIL=true` around a real `main()` call on a
+throwaway git repo, asserting exit 1 with the clause check on and exit 0 both
+with `NLB_CLAUSE_BREAKS=false` and with the length gate raised past the line.
+Each was confirmed to fail when the corresponding env read is stubbed out.
+(gha#337 review round 2: the step's original comment, and this paragraph,
+both claimed the step proved the plumbing; neither could.)
+Round 3 added the converse caveat, since "cannot prove the input arrived" is
+not "proves nothing": the step still pins that `action.yml` parses and that
+the opt-out code path runs to completion, which is why it stayed rather than
+being deleted as dead weight.
+Round 5 narrowed that caveat in turn -- it had also claimed the step pins
+that the input is *declared*, contradicting this paragraph's own point two
+sentences earlier that an undeclared input is only a warning.
+Declaration is pinned by the defaults-agreement test instead, which reads
+each YAML file for the input's `default:` and fails outright when there is
+none (gha#337 review round 5).
+
+**Markup stripping is where this check's false verdicts come from, in both
+directions.**
+The clause check keys on a semicolon in the *stripped* line, so every pattern
+in `strip_inline_markup` decides two things at once: whether a `;` is prose,
+and whether the line is long enough to look at.
+Both of gha#337's round-3 findings were one pattern each.
+A code-span pattern of `` `[^`]*` `` matches the empty span between the two
+opening backticks of a ```` ``...`` ```` span, so an N-backtick span kept its
+contents and a `;`-separated shell command read as prose -- the exact case
+the stripping exists to remove.
+And a bare-URL pattern of `https?://\S+` runs to the next whitespace, so a
+`;` immediately after a URL was deleted along with it, silencing a genuine
+break.
+The rule that catches both: a pattern must remove the construct and nothing
+adjacent to it, so backreference a delimiter's opening run rather than
+matching to the next one, and stop a URL before trailing sentence
+punctuation.
+
 **Generate selftest fixtures at runtime; don't commit them.** A fixture
 committed under a composite's `tests/` dir (e.g. a minimal R package for
 `test-coverage`) gets swept into OTHER selftest jobs' repo-wide scans: the
@@ -588,12 +648,12 @@
 | `gh issue list` / `gh issue view <n>` | `mcp__github__list_issues` / `mcp__github__issue_read` |
 | read a file / repo contents | `mcp__github__get_file_contents` |
 | create/edit a file (no local checkout) | `mcp__github__create_or_update_file` — needs the target branch, full new file content, and the file's current blob `sha` (from `get_file_contents`) if it already exists |
 | create a branch (no local checkout) | `mcp__github__create_branch` |
 | CI runs & job logs | `mcp__github__actions_list`, `mcp__github__actions_get`, `mcp__github__get_job_logs` |
 | watch / stop watching PR activity | `mcp__github__subscribe_pr_activity` / `mcp__github__unsubscribe_pr_activity` |
 | `glab mr ...` (GitLab) | N/A — this repo is on GitHub; use the tools above |
 
 Posting inline comments requires a **pending review to already exist** before
 `mcp__github__add_comment_to_pending_review`; create the pending review first, add
 each comment, then submit once at the end. Watch and respond to PR activity with
 `mcp__github__subscribe_pr_activity` / `mcp__github__unsubscribe_pr_activity` (not
@@ -731,7 +791,7 @@
 ! [remote rejected] ... (refusing to allow a GitHub App to create or
   update workflow `.github/workflows/<file>` without `workflows` permission)
 ```
 
 this means the `WORKFLOW_TOKEN` **repository secret** is unset or lacks
 `contents:write` + `workflows:write` scope for this repo — it is not a bug in
 `claude.yml`'s own token-resolution code. `claude.yml` already resolves

README.md

@@ -49,7 +49,7 @@ not reference `@main` from consumers.
 | `check-links.yml` | lychee link check with bundled config, PR skip-label, and auto-issue on `main` | `lychee-config`, `lychee-args`, `fail`, `fail-if-empty`, `create-issue-on-main`, `skip-label` |
 | `lint-yaml.yml` | yamllint over tracked YAML with a bundled config, plus a check that flags long `run:` script blocks as decomposition candidates | `python-version`, `config-file`, `paths-ignore`, `fail`, `max-script-lines`, `fail-on-long-scripts` |
 | `lint-markdown.yml` | markdownlint-cli2 over tracked Markdown with a bundled config, plus a check that flags long fenced code blocks as decomposition candidates | `config-file`, `globs`, `paths-ignore`, `fail`, `max-code-block-lines`, `fail-on-long-code-blocks` |
-| `check-new-line-breaks.yml` | Advisory, diff-scoped check that flags newly-added Markdown lines packing more than one sentence/clause onto one source line | `python-version`, `globs`, `paths-ignore`, `fail` |
+| `check-new-line-breaks.yml` | Advisory, diff-scoped check that flags newly-added Markdown lines packing more than one sentence/clause onto one source line | `python-version`, `globs`, `paths-ignore`, `fail`, `clause-breaks`, `clause-min-length` |
 | `lint-qmd.yml` | markdownlint over the prose sections of tracked `.qmd` Quarto files (code chunks stripped, YAML front matter skipped natively) with a bundled default config; default 80-char line-length ceiling encourages semantic line breaks | `config-file`, `globs`, `paths-ignore`, `fail`, `max-line-length` |
 | `lint-changed-lines.yml` | lintr over only the lines a PR adds or modifies (not whole changed files), so lint rules can be adopted or tightened incrementally | `path`, `install-quarto`, `extra-packages`, `install-package`, `fail` |
 | `summary.yml` | AI summary comment on newly opened issues | — |

changelog.d/clause-breaks-check.changed.md

@@ -0,0 +1,23 @@
+- **`check-new-line-breaks` now also flags a long line carrying a mid-line
+  semicolon** (#336), on top of the sentence check it already ran.
+  This is a proxy for the [SemBr spec](https://sembr.org)'s rule 5
+  ("a semantic line break SHOULD occur after an independent clause"),
+  alongside the rule 4 MUST the check already enforced.
+  It is a proxy rather than a test of the rule, since deciding whether a mark
+  ends an *independent* clause needs a parser.
+
+  The new check is **on by default**, so existing callers get the extra
+  annotations without changing anything.
+  That is safe because the whole check stays warn-only unless `fail: true` is
+  set: it adds annotations, not build failures.
+  Set the new `clause-breaks: false` input to check sentences only, and
+  `clause-min-length` (default `80`, the spec's own rule 12) to move the
+  length gate.
+
+  Of the four marks rule 5 names, only the semicolon is used, and only past
+  that gate: see #336 for the
+  hit rates behind that choice.
+  The gate measures a line's visible length, after stripping inline markup
+  such as code spans, link targets, bare URLs, and HTML entities, so a line
+  that is long only because of a URL does not qualify -- rule 13's own
+  exemption.

check-new-line-breaks/action.yml

@@ -38,6 +38,24 @@ inputs:
       to warn-only: a long line can legitimately be un-splittable (a URL, a
       citation, a single long clause), so this is a nudge, not a gate.
     default: 'false'
+  clause-breaks:
+    description: >-
+      Also flag a long line carrying a mid-line semicolon -- a narrow slice of
+      the SemBr spec's rule 5 ("a semantic line break SHOULD occur after an
+      independent clause"), on top of the rule 4 sentence check that always
+      runs. On by default; set 'false' to check sentences only. See
+      d-morrison/gha#336 for why only the semicolon is used, and the
+      measurements behind it.
+    default: 'true'
+  clause-min-length:
+    description: >-
+      Minimum line length, in characters, before `clause-breaks` applies;
+      inclusive, so a line of exactly this length is checked. Measured on the
+      line's visible prose, after stripping inline markup such as code spans,
+      link targets, bare URLs, and HTML entities, so a line that is long only
+      because of a URL does not qualify. Ignored when `clause-breaks` is
+      'false'.
+    default: '80'
 runs:
   using: composite
   steps:
@@ -53,4 +71,6 @@ runs:
         NLB_GLOBS: ${{ inputs.globs }}
         NLB_PATHS_IGNORE: ${{ inputs.paths-ignore }}
         NLB_FAIL: ${{ inputs.fail }}
+        NLB_CLAUSE_BREAKS: ${{ inputs.clause-breaks }}
+        NLB_CLAUSE_MIN_LENGTH: ${{ inputs.clause-min-length }}
       run: python3 -u "$GITHUB_ACTION_PATH/check-new-line-breaks.py"