[PR #398] [MERGED] fix(cursor-approve): clamp an over-long axis headline instead of failing the axis #399

New issue

Closed

opened 2026-10-26 22:39:22 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:39:22 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/github-workflows/pull/398
Author: @mattmillerai
Created: 10/9/2026
Status: ✅ Merged
Merged: 10/9/2026
Merged by: @mattmillerai

Base: main ← Head: matt/cursor-axis-clamp-headline

📝 Commits (2)

727cc12 fix(cursor-approve): clamp an over-long axis headline instead of failing the axis
fc94283 fix(cursor-approve): fail closed on a separator-only clamp, keep the reason on an early space

📊 Changes

4 files changed (+102 additions, -19 deletions)

View changed files

📝 .github/cursor-approve/README.md (+7 -3)
📝 .github/cursor-approve/aggregate.py (+30 -8)
📝 .github/cursor-approve/tests/test_aggregate.py (+61 -5)
📝 docs/callers/cursor-approve.md (+4 -3)

📄 Description

ELI-5

Each cursor-approve reviewer writes a one-line "headline" explaining its verdict, capped at 100 characters. When a reviewer wrote a slightly longer line (106 or 113 characters), the whole check went red, even though the verdict was fine. The model can't reliably count characters, so this PR shortens an over-long headline to fit, cutting at a word and adding "…", instead of failing.

What changed

aggregate.py: validate_output now clamps the whitespace-folded headline to HEADLINE_MAX (100) with a new clamp_headline. It cuts at the last space that fits and appends …, and trims trailing ,;:-. It falls back to a hard cut when that space is in the first half, for example one long token or Fix: <long path>. If nothing but separators survives, it raises, so the axis stays untrusted instead of showing a bare …. It used to raise headline is N characters, over the 100 limit.
cmd_extract writes the clamped headline into the uploaded artifact, so the artifact is always within the cap.
A missing or empty headline is still untrusted. The "card says WHY" guarantee from #395 holds, because the first 100 characters carry the reason.
README and docs/callers/cursor-approve.md: the rule now reads clamp, not reject.

Why

extract runs in each axis job's Extract the verdict step. When a headline was over the cap, it exited 1 there and the axis job (axis: conformance and axis: correctness) failed on length alone. A prompt instruction or a retry can't guarantee a length limit on model output. Only enforcing it in code makes it deterministic.

Testing

python3 -m unittest discover -s .github/cursor-approve/tests: 205 tests, OK.
Replaced test_an_overlong_headline_is_rejected_not_truncated with tests that cover:
- clamping in decide
- the word-boundary cut, including a word that fits exactly at 100
- the hard cut for one long token or an early-only space
- trimming trailing punctuation
- rejecting a separator-only headline, both directly and via decide
- the clamped value appearing in the extract CLI artifact

Callers that pin a SHA, like cloud, pick this up through the bump-callers flow once it merges.

Provenance

Authored by: agent-work loop
Verified:
- python3 -m unittest discover -s .github/cursor-approve/tests: 205 tests, OK
- python3 -m unittest discover -s .github/cursor-review/tests: OK
- check_workflow_pins.py, check_agents_md.py --root . and check-org-repo-literals.sh: pass
Deviations:
- The cursor-review panel's 4 findings are addressed in fc94283: separator-only fail-closed, early-space fallback, the off-by-one boundary, and the caller-guide docs.

🤖 Generated with Claude Code

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/github-workflows/pull/398 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 10/9/2026 **Status:** ✅ Merged **Merged:** 10/9/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/cursor-axis-clamp-headline` --- ### 📝 Commits (2) - [`727cc12`](https://github.com/Comfy-Org/github-workflows/commit/727cc128a68ea29f7b3d6c0900fe20dd9c6bf5f7) fix(cursor-approve): clamp an over-long axis headline instead of failing the axis - [`fc94283`](https://github.com/Comfy-Org/github-workflows/commit/fc942831426c325fff5c26ae9ba5bda18b39f6f7) fix(cursor-approve): fail closed on a separator-only clamp, keep the reason on an early space ### 📊 Changes **4 files changed** (+102 additions, -19 deletions) <details> <summary>View changed files</summary> 📝 `.github/cursor-approve/README.md` (+7 -3) 📝 `.github/cursor-approve/aggregate.py` (+30 -8) 📝 `.github/cursor-approve/tests/test_aggregate.py` (+61 -5) 📝 `docs/callers/cursor-approve.md` (+4 -3) </details> ### 📄 Description ## ELI-5 Each cursor-approve reviewer writes a one-line "headline" explaining its verdict, capped at 100 characters. When a reviewer wrote a slightly longer line (106 or 113 characters), the whole check went red, even though the verdict was fine. The model can't reliably count characters, so this PR shortens an over-long headline to fit, cutting at a word and adding "…", instead of failing. ## What changed - `aggregate.py`: `validate_output` now clamps the whitespace-folded headline to `HEADLINE_MAX` (100) with a new `clamp_headline`. It cuts at the last space that fits and appends `…`, and trims trailing `,;:-`. It falls back to a hard cut when that space is in the first half, for example one long token or `Fix: <long path>`. If nothing but separators survives, it raises, so the axis stays untrusted instead of showing a bare `…`. It used to raise `headline is N characters, over the 100 limit`. - `cmd_extract` writes the clamped headline into the uploaded artifact, so the artifact is always within the cap. - A missing or empty headline is still untrusted. The "card says WHY" guarantee from #395 holds, because the first 100 characters carry the reason. - README and `docs/callers/cursor-approve.md`: the rule now reads clamp, not reject. ## Why `extract` runs in each axis job's **Extract the verdict** step. When a headline was over the cap, it exited 1 there and the axis job (`axis: conformance` and `axis: correctness`) failed on length alone. A prompt instruction or a retry can't guarantee a length limit on model output. Only enforcing it in code makes it deterministic. ## Testing - `python3 -m unittest discover -s .github/cursor-approve/tests`: 205 tests, OK. - Replaced `test_an_overlong_headline_is_rejected_not_truncated` with tests that cover: - clamping in `decide` - the word-boundary cut, including a word that fits exactly at 100 - the hard cut for one long token or an early-only space - trimming trailing punctuation - rejecting a separator-only headline, both directly and via `decide` - the clamped value appearing in the `extract` CLI artifact Callers that pin a SHA, like cloud, pick this up through the bump-callers flow once it merges. ## Provenance - **Authored by:** agent-work loop - **Verified:** - `python3 -m unittest discover -s .github/cursor-approve/tests`: 205 tests, OK - `python3 -m unittest discover -s .github/cursor-review/tests`: OK - `check_workflow_pins.py`, `check_agents_md.py --root .` and `check-org-repo-literals.sh`: pass - **Deviations:** - The cursor-review panel's 4 findings are addressed in fc94283: separator-only fail-closed, early-space fallback, the off-by-one boundary, and the caller-guide docs. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-10-26 22:39:22 +00:00

bsakaguchi referenced this issue

2026-10-26 22:39:22 +00:00

[PR #399] [MERGED] feat(cursor-review): optional direct-Anthropic judge via the pinned Claude Code CLI #400

bsakaguchi closed this issue

2026-10-26 22:39:22 +00:00

bsakaguchi referenced this issue

2026-10-26 22:39:25 +00:00

[PR #401] [MERGED] feat(cursor-review): record the direct judge's token usage #402