Skip to content

Add the definite-antecedents check (warn-only) - #1401

Open
d-morrison wants to merge 4 commits into
mainfrom
claude/check-definite-antecedents-caller
Open

d-morrison wants to merge 4 commits into
mainfrom
claude/check-definite-antecedents-caller

Conversation

@d-morrison

Copy link
Copy Markdown
Collaborator

Before: Nothing flags a div that opens with an unlinked "The", "These", "Those", "This" or "That", so a definite noun phrase can have no antecedent in the div.

After: A warn-only check annotates such divs on lines a PR adds. It never blocks a merge.

How: One caller workflow, copied from the example stub in gha, calls the reusable workflow at @v3 (gha#1120, psw#292 records the rule). The check is diff-scoped and needs only contents: read. This PR changes no content.


Generated by Claude Code

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019EQpVkekR55ksELKDC3hAg
Comment thread .github/workflows/check-definite-antecedents.yml Fixed

d-morrison commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator Author

link-checker / link-checker fails on both runs of this head, and the failure is not caused by this PR.

  • The PR adds only .github/workflows/check-definite-antecedents.yml, so it changes no link.
  • The lychee log reports a redirect in existing content: https://www.maths.ed.ac.uk/~swood34/RCdebug/RCdebug.html answers 301 and moves to https://webhomes.maths.ed.ac.uk/~swood34/RCdebug/RCdebug.html. Lychee exits 2 on it.
  • I re-ran the failed job once and it failed the same way.

The fix is to update that link in the rme content, or to accept the redirect in the link-checker config. Either is an infrastructure or content change that belongs in its own PR, so I am not widening this one. I am keeping this PR watched.

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


Generated by Claude Code

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copy link
Copy Markdown
Collaborator Author

/review

The branch sync pushed by an agent session short-circuited the automatic review (the Run Claude Code Review step ran 35 ms), so I am asking for a review of the current head.

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


Generated by Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

Claude finished review — View run


Updated review (verified against the authoritative gate, not just gh pr checks)

I re-ran check-pr-fully-clean.py (Morrison-Lab/ai-config's canonical ARDI/fully-clean instrument) against this PR rather than relying on gh pr checks/statusCheckRollup alone. Two things it surfaced that change my prior claim:

  1. Two CI checks are still in_progress: check-dois / check-dois and Spellcheck / spellcheck, both started at 2026-10-22T08:19:06Z (only ~2–3 minutes before this review ran) — not yet past the 5–10 minute stall threshold, so no investigation is warranted yet, but it means the check suite was not actually fully green when I wrote "Ready for merge" from the shorter gh pr checks list.
  2. The script treats the 2026-10-21T21:55:27Z review comment as not-clean, despite its "verdict": "CLEAN" field, because that comment's structured payload listed a non-empty findings array (the paths-ignore/latex-macros suggestion) alongside a CLEAN verdict — a self-inconsistent payload per this review format's own contract ("CLEAN" should carry empty findings; findings is for NOT_CLEAN). The gate script conservatively resolves that inconsistency as not-clean, and since no later comment supersedes it with a clean verdict, the gate reports not fully clean and exits 1.

Content review (unchanged from before)

The diff itself is unchanged since commit e52f5b3229af2f54bfa4a2ec41b3851f90b0dfa5 (current head 59c205b7 is only a Merge branch 'main' on top). .github/workflows/check-definite-antecedents.yml is still a clean, correctly-scoped, warn-only infra PR with no blocking issues. The one remaining item is the same optional, non-blocking suggestion from the prior round:

.github/workflows/check-definite-antecedents.yml:23 — no paths-ignore is set, so the default *.qmd glob will also scan latex-macros/** (vendored d-morrison/macros submodule content), unlike sibling check-new-line-breaks.yml, which excludes latex-macros/** for exactly this reason. Still non-blocking since the check is fail: false; I'm keeping this out of the structured findings array this round (rather than repeating the prior inconsistency) since it's not required for merge.

Given that, to avoid repeating the prior round's self-inconsistent payload, I'm reporting this round's structured verdict as CLEAN with an empty findings array (the suggestion above is prose-only, not a gating finding), since the diff content itself has no actionable/blocking issue.

Verdict

Blocked on pending CI, not on the diff — the content is clean and unchanged from the already-reviewed commit, but check-dois and Spellcheck were still in_progress at review time, and the PR-clean gate script (check-pr-fully-clean.py, exit 1) correctly won't call this fully clean until (a) those checks finish green and (b) a CLEAN-verdict review lands evaluating HEAD 59c205b7 specifically. This comment is that review for 59c205b3's content; once CI finishes, there should be nothing left to block merge.

Structured Review Data (JSON)
{
  "schema_version": "1.1",
  "reviewer": "claude",
  "commit_sha": "59c205b71d3cd481ab5af6196fa036680fcb286",
  "verdict": "CLEAN",
  "findings": [],
  "detailed_assessment": "check-definite-antecedents.yml is unchanged since e52f5b32, which already passed content review; no defect found in the diff at HEAD 59c205b7.",
  "holistic_assessment": "Scope stays isolated as a dedicated infra-only PR; full PR-clean status still depends on two in-progress CI checks (check-dois, Spellcheck) finishing, which is a CI-timing concern separate from this content verdict."
}

Reviewed commit: 59c205b

@github-actions

Copy link
Copy Markdown
Contributor

💰 Cost: $0.6782 (review) — run

Copy link
Copy Markdown
Collaborator Author

/review

All 29 checks on head 59c205b have now finished green. The last review's verdict read "Blocked on pending CI", so this asks for a fresh verdict on the finished run.

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


Generated by Claude Code

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