Comfy-Org/github-workflows
1
0
Fork 0
mirror of https://github.com/Comfy-Org/github-workflows.git synced 2026-11-05 00:17:37 +00:00
Code Issues 20 Projects Packages Wiki Activity

[PR #413] [MERGED] feat(cursor-approve): show each axis's full first sentence and its full reasoning on the card (BE-20249) #413

New issue
Closed
opened 2026-10-26 22:39:31 +00:00 by bsakaguchi · 0 comments
bsakaguchi commented 2026-10-26 22:39:31 +00:00
Owner
Copy link

📋 Pull Request Information

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

Base: main ← Head: matt/be-20249-card-why-full-sentence

📝 Commits (2)

  • ba424d6 feat(cursor-approve): show each axis's full first sentence and its full reasoning on the card (BE-20249)
  • 3a49fdd fix(cursor-approve): tighten the headline sentence cut and align the Why fallbacks (BE-20249)

📊 Changes

9 files changed (+493 additions, -72 deletions)

View changed files

📝 .github/cursor-approve/README.md (+15 -8)
📝 .github/cursor-approve/aggregate.py (+65 -6)
📝 .github/cursor-approve/card.py (+79 -20)
📝 .github/cursor-approve/prompt-common.md (+7 -6)
📝 .github/cursor-approve/tests/test_aggregate.py (+111 -10)
📝 .github/cursor-approve/tests/test_card.py (+194 -6)
📝 .github/workflows/cursor-approve.yml (+4 -4)
📝 .github/workflows/cursor-axis-base.yml (+4 -3)
📝 docs/callers/cursor-approve.md (+14 -9)

📄 Description

ELI-5

The approve bot keeps one status card on each PR. The card has a table with a row per reviewer axis (correctness, conformance, …) and a short reason for each verdict. That reason was chopped at 100 characters, so it usually stopped mid-sentence ("… the cache is still not wired into…"). The full explanation was only in the Actions run.

Now the row shows the whole sentence. Under the result there is a collapsed "full reasoning" section for each axis, which you can open right on the PR.

What changed

  • aggregate.py
    • The headline cap goes from 100 to 240 characters.
    • Past the cap, the clamp keeps every whole sentence that fits, with no "…".
      • ., ! or ? followed by whitespace ends a sentence, including after a closing quote or bracket.
      • Common abbreviations (e.g., i.e., vs., …), initialisms (U.S.), one-character initials and list numbers (J., 2.) and ellipses do not. Neither does a decimal or a mid-word dot, since no space follows it. etc. ends a sentence only before a capital.
      • A sentence cut that keeps fewer than four words ("OK.") is not taken.
    • Otherwise (a first sentence longer than 240 characters, or too short a lead), the headline is word-cut with "…". The old hard-cut fallback is unchanged.
    • A headline with no word character is rejected at any length, not only past the cap.
  • card.py
    • The Why cell renders the whole clamped headline. It is sanitised as before, with a cell limit WHY_LIMIT that a test pins equal to HEADLINE_MAX. A headline that is missing or that the clamp rejects falls back to the summary's first sentence.
    • Under the result, there is one collapsed <details> block per axis that reported a summary. It holds the sanitised full summary and the confidence.
    • The decide card now links "workflow run" on the reviewed-commit line, like every other card. Before, the rule line carried the link and the line under it printed an unlinked "workflow run".
    • The job summary's headline is the card's Why text (why_text), so the two always match.
  • prompt-common.md: the headline is ONE sentence, aiming for about 120 characters and never more than 240. The summary is now described as shown collapsed on the card.
  • Docs and comments: docs/callers/cursor-approve.md (the headline/card passage and the 240 cap), the .github/cursor-approve/README.md field table, and the header comments in cursor-approve.yml and cursor-axis-base.yml.

Why

BE-20249.

  • Clipped reasons. On a real round-5 card in a private consumer repo, both rows (correctness and conformance) were clipped mid-sentence. The models write one-sentence reasons of 110–140 characters, and the 100-character cap cut every one of them.
  • Buried detail. The only way to the reasoning was the Actions run.

Design notes

  • Whole headline, not "first sentence only". A headline that fits is shown whole. Cutting one that fits down to its first sentence would drop information, and a first-sentence splitter stops at "e.g.". The sentence rule only matters past the cap, and there it guarantees at least the full first sentence.
  • The agent contract is unchanged.
    • The first three lines and the | Axis | Verdict | Why | rows are as before. Only the cell text can be longer.
    • The blocks sit below the table and the result.
    • Model text goes through sanitize, which folds it to one line and escapes every | and <.
    • A test ports the agent fleet's awk -F'|' row parser line for line. It proves a summary that carries a forged row, a forged state marker, </details>, a heading, a mention or the card marker changes nothing an agent reads.
  • No new exposure.
    • The summary already reached the run's job summary. That has the same audience as the PR comment, on public and private repos alike.
    • With prior_rounds on, the summary was also already in the card's base64 sentinel.
    • The context axes' Linear/Notion/Slack sources stay private-repo only.

Coordination

This touches .github/cursor-approve/card.py alongside #412 (BE-20246).

  • _reviewed_line keeps its signature, which #412's render_pending uses.
  • #412 has merged. A trial merge of this head with current main (ebd6a64, which includes #412) is clean, and the cursor-approve and workflow-pins suites and check_workflow_pins.py pass on the merged tree.

Testing

  • python3 -m unittest discover -s .github/cursor-approve/tests -p 'test_*.py': 267 OK on the current head. New cases:
    • the sentence-preferring clamp (whole sentences, ?, abbreviations, initialisms, initials and list numbers, ellipses, closing quotes and brackets, etc. before a capital, decimals and file names, a short lead, a cut that keeps no word, a word-less headline within the cap, and decide end to end);
    • the Why cell's summary fallback for a rejected headline, and the job summary showing the card's headline;
    • the full-sentence cell, and the pin between WHY_LIMIT and HEADLINE_MAX;
    • the blocks: one per axis, placement, no summary or verdict, no confidence, none on superseded or start cards;
    • inert model text, checked with the fleet's parser port;
    • the run link on every card state.
  • .github/cursor-review/tests: 1527 OK.
  • .github/workflow-pins/tests: 551 OK (43 skipped). check_workflow_pins.py OK.
  • .github/agents-md-integrity/tests: 62 OK. check_agents_md.py passed, with the same 2 warnings as main (line count, no CODEOWNERS).
  • .github/public-repo-hygiene/tests: 204 OK. check-org-repo-literals.sh OK. shellcheck OK.
Sample: a two-axis pass card as rendered (markdown) 
<!-- cursor-approve-card -->
<!-- cursor-approve-state: pass -->
<!-- cursor-approve-next: none -->
### Cursor approve · round 5 of 5 · `e9ece9c`

| Axis | Verdict | Why |
|---|---|---|
| correctness | 🟢 green | Round\-4 changes hold up and each new test fails without its fix; the cache is still not wired into the request path. |
| conformance | 🟢 green | Flag bound via envconfig and documented in this PR; the service stays build\-isolated, with tests beside the code. |

**Result: ✅ Approved.**

<details>
<summary>correctness — full reasoning (confidence 0.86)</summary>

Rebuild now keeps a charge recorded after its cut\-off, rejects a cut\-off later than the clock, and resets a key on every tier change. Each new test fails with its fix reverted. Nothing calls the cache yet; the flag is read only in config.

</details>

<details>
<summary>conformance — full reasoning (confidence 0.91)</summary>

Standard: the root AGENTS.md and the service's own AGENTS.md. Logging uses the context\-first form, there are no new cross\-module imports, and the flag has a default tag.

</details>

_Rule: no red, at most 0 yellow; every axis must report (⚪ n/a counts as reporting and does not block, but all-n/a withholds)._

_Reviewed commit: `e9ece9ca341ba9ed1d7c1ae302152d7aef895418` · [workflow run](https://github.com/o/r/actions/runs/1)_

Rollout

Callers get this on their next SHA bump. There are no new inputs. The 240 cap and the prompt wording apply from the next round.

Provenance

  • Authored by: agent-work loop
  • Verified:
    • .github/cursor-approve/tests: 267 OK on the current head.
    • .github/workflow-pins/tests OK; check_workflow_pins.py, check_agents_md.py (same 2 warnings as main) and check-org-repo-literals.sh pass.
    • The cursor-review panel's findings were addressed: 6 fixed in code with tests, 2 answered on their threads with no change.
  • Deviations:
    • The review asked for a half-cap floor on the sentence cut. A four-word floor is used instead, so a short first sentence is still kept whole, which is the point of this PR.

🤖 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/413 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 10/12/2026 **Status:** ✅ Merged **Merged:** 10/12/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/be-20249-card-why-full-sentence` --- ### 📝 Commits (2) - [`ba424d6`](https://github.com/Comfy-Org/github-workflows/commit/ba424d67c4c839383dd7f2978493060dde8172e0) feat(cursor-approve): show each axis's full first sentence and its full reasoning on the card (BE-20249) - [`3a49fdd`](https://github.com/Comfy-Org/github-workflows/commit/3a49fdd9711ca7684bfcc561e94b7eee685c3a77) fix(cursor-approve): tighten the headline sentence cut and align the Why fallbacks (BE-20249) ### 📊 Changes **9 files changed** (+493 additions, -72 deletions) <details> <summary>View changed files</summary> 📝 `.github/cursor-approve/README.md` (+15 -8) 📝 `.github/cursor-approve/aggregate.py` (+65 -6) 📝 `.github/cursor-approve/card.py` (+79 -20) 📝 `.github/cursor-approve/prompt-common.md` (+7 -6) 📝 `.github/cursor-approve/tests/test_aggregate.py` (+111 -10) 📝 `.github/cursor-approve/tests/test_card.py` (+194 -6) 📝 `.github/workflows/cursor-approve.yml` (+4 -4) 📝 `.github/workflows/cursor-axis-base.yml` (+4 -3) 📝 `docs/callers/cursor-approve.md` (+14 -9) </details> ### 📄 Description ## ELI-5 The approve bot keeps one status card on each PR. The card has a table with a row per reviewer axis (correctness, conformance, …) and a short reason for each verdict. That reason was chopped at 100 characters, so it usually stopped mid-sentence ("… the cache is still not wired into…"). The full explanation was only in the Actions run. Now the row shows the whole sentence. Under the result there is a collapsed **"full reasoning"** section for each axis, which you can open right on the PR. ## What changed - **`aggregate.py`** - The headline cap goes from 100 to **240** characters. - Past the cap, the clamp keeps every whole sentence that fits, with no "…". - `.`, `!` or `?` followed by whitespace ends a sentence, including after a closing quote or bracket. - Common abbreviations (`e.g.`, `i.e.`, `vs.`, …), initialisms (`U.S.`), one-character initials and list numbers (`J.`, `2.`) and ellipses do not. Neither does a decimal or a mid-word dot, since no space follows it. `etc.` ends a sentence only before a capital. - A sentence cut that keeps fewer than four words ("OK.") is not taken. - Otherwise (a first sentence longer than 240 characters, or too short a lead), the headline is word-cut with "…". The old hard-cut fallback is unchanged. - A headline with no word character is rejected at any length, not only past the cap. - **`card.py`** - The Why cell renders the whole clamped headline. It is sanitised as before, with a cell limit `WHY_LIMIT` that a test pins equal to `HEADLINE_MAX`. A headline that is missing or that the clamp rejects falls back to the summary's first sentence. - Under the result, there is one collapsed `<details>` block per axis that reported a summary. It holds the sanitised full `summary` and the confidence. - The decide card now links "workflow run" on the reviewed-commit line, like every other card. Before, the rule line carried the link and the line under it printed an unlinked "workflow run". - The job summary's headline is the card's Why text (`why_text`), so the two always match. - **`prompt-common.md`**: the headline is ONE sentence, aiming for about 120 characters and never more than 240. The summary is now described as shown collapsed on the card. - **Docs and comments**: `docs/callers/cursor-approve.md` (the headline/card passage and the 240 cap), the `.github/cursor-approve/README.md` field table, and the header comments in `cursor-approve.yml` and `cursor-axis-base.yml`. ## Why BE-20249. - **Clipped reasons.** On a real round-5 card in a private consumer repo, both rows (correctness and conformance) were clipped mid-sentence. The models write one-sentence reasons of 110–140 characters, and the 100-character cap cut every one of them. - **Buried detail.** The only way to the reasoning was the Actions run. ## Design notes - **Whole headline, not "first sentence only".** A headline that fits is shown whole. Cutting one that fits down to its first sentence would drop information, and a first-sentence splitter stops at "e.g.". The sentence rule only matters past the cap, and there it guarantees at least the full first sentence. - **The agent contract is unchanged.** - The first three lines and the `| Axis | Verdict | Why |` rows are as before. Only the cell text can be longer. - The blocks sit below the table and the result. - Model text goes through `sanitize`, which folds it to one line and escapes every `|` and `<`. - A test ports the agent fleet's `awk -F'|'` row parser line for line. It proves a summary that carries a forged row, a forged state marker, `</details>`, a heading, a mention or the card marker changes nothing an agent reads. - **No new exposure.** - The summary already reached the run's job summary. That has the same audience as the PR comment, on public and private repos alike. - With `prior_rounds` on, the summary was also already in the card's base64 sentinel. - The context axes' Linear/Notion/Slack sources stay private-repo only. ## Coordination This touches `.github/cursor-approve/card.py` alongside #412 (BE-20246). - `_reviewed_line` keeps its signature, which #412's `render_pending` uses. - #412 has merged. A trial merge of this head with current `main` (`ebd6a64`, which includes #412) is clean, and the cursor-approve and workflow-pins suites and `check_workflow_pins.py` pass on the merged tree. ## Testing - `python3 -m unittest discover -s .github/cursor-approve/tests -p 'test_*.py'`: 267 OK on the current head. New cases: - the sentence-preferring clamp (whole sentences, `?`, abbreviations, initialisms, initials and list numbers, ellipses, closing quotes and brackets, `etc.` before a capital, decimals and file names, a short lead, a cut that keeps no word, a word-less headline within the cap, and decide end to end); - the Why cell's summary fallback for a rejected headline, and the job summary showing the card's headline; - the full-sentence cell, and the pin between `WHY_LIMIT` and `HEADLINE_MAX`; - the blocks: one per axis, placement, no summary or verdict, no confidence, none on superseded or start cards; - inert model text, checked with the fleet's parser port; - the run link on every card state. - `.github/cursor-review/tests`: 1527 OK. - `.github/workflow-pins/tests`: 551 OK (43 skipped). `check_workflow_pins.py` OK. - `.github/agents-md-integrity/tests`: 62 OK. `check_agents_md.py` passed, with the same 2 warnings as main (line count, no CODEOWNERS). - `.github/public-repo-hygiene/tests`: 204 OK. `check-org-repo-literals.sh` OK. `shellcheck` OK. <details> <summary>Sample: a two-axis pass card as rendered (markdown)</summary> ```markdown <!-- cursor-approve-card --> <!-- cursor-approve-state: pass --> <!-- cursor-approve-next: none --> ### Cursor approve · round 5 of 5 · `e9ece9c` | Axis | Verdict | Why | |---|---|---| | correctness | 🟢 green | Round\-4 changes hold up and each new test fails without its fix; the cache is still not wired into the request path. | | conformance | 🟢 green | Flag bound via envconfig and documented in this PR; the service stays build\-isolated, with tests beside the code. | **Result: ✅ Approved.** <details> <summary>correctness — full reasoning (confidence 0.86)</summary> Rebuild now keeps a charge recorded after its cut\-off, rejects a cut\-off later than the clock, and resets a key on every tier change. Each new test fails with its fix reverted. Nothing calls the cache yet; the flag is read only in config. </details> <details> <summary>conformance — full reasoning (confidence 0.91)</summary> Standard: the root AGENTS.md and the service's own AGENTS.md. Logging uses the context\-first form, there are no new cross\-module imports, and the flag has a default tag. </details> _Rule: no red, at most 0 yellow; every axis must report (⚪ n/a counts as reporting and does not block, but all-n/a withholds)._ _Reviewed commit: `e9ece9ca341ba9ed1d7c1ae302152d7aef895418` · [workflow run](https://github.com/o/r/actions/runs/1)_ ``` </details> ## Rollout Callers get this on their next SHA bump. There are no new inputs. The 240 cap and the prompt wording apply from the next round. ## Provenance - **Authored by:** agent-work loop - **Verified:** - `.github/cursor-approve/tests`: 267 OK on the current head. - `.github/workflow-pins/tests` OK; `check_workflow_pins.py`, `check_agents_md.py` (same 2 warnings as main) and `check-org-repo-literals.sh` pass. - The cursor-review panel's findings were addressed: 6 fixed in code with tests, 2 answered on their threads with no change. - **Deviations:** - The review asked for a half-cap floor on the sentence cut. A four-word floor is used instead, so a short first sentence is still kept whole, which is the point of this PR. 🤖 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 2026-10-26 22:39:31 +00:00
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
fix/cursor-review-post-retry-5xx
ci/bump-cursor-review
ci/refresh-coderabbit-schema
ci/bump-cursor-cli-pin
dependabot/github_actions/github-actions-43d9b7ee8a
dependabot/pip/dot-github/coderabbit-config/rpds-py-2026.9.1
codex/cursor-review-fields
codex/cursor-review-history-shape
codex/cursor-label-shape
codex/cursor-immediate-handoff
codex/cursor-preserve-rounds
codex/cursor-count-superseded
codex/cursor-three-review
codex/cursor-fallback-finding-identity
codex/readiness-human-identity
codex/readiness-reviewer-permissions
codex/readiness-approval-incomplete
codex/cursor-review-evaluation
codex/readiness-check-failures
codex/readiness-shadow-report
codex/readiness-review-inventory
codex/readiness-from-risk
codex/pr-risk-report-snapshot
codex/pr-risk-report-only
codex/risk-report-base-revision
fix/cursor-review-account-outage-guard
groom/github-workflows-whole-repo-github-cursor-review-39021195499-0
matt/direct-lane-error-visibility
recovery/worktrees/20261008T012922Z-github-workflows-pr178-dirty-3c1b68f78aaf
matt/cursor-review-auto-approve
fix/detect-unreviewed-human-approvals
anupreet-comfy/be-17523-runs-on-input
christian-byrne/cifix-16923-groom-finder-bounds
christian-byrne/fix-linear-ticket-post-merge
chore/comfy-cli-codeowners-team-allowlist
matt/be-4405-groom-extra-denied-paths
cx/risk-dispute-command
cx/risk-label-map-variable
cx/named-risk-tiers
luke/area-label-bump-doc
matt/be-5508-pr-risk-shadow
matt/be-4005-bump-cursor-review-auto-label-callers
agent/cursor-review-xl
matt/be-3160-judge-parse-hardening
matt/auto-label-runs-on
matt/cursor-review-self-hostable
matt/be-3046-cursor-review-callers-var
matt/reviewers-team-slug-expansion
matt/cursor-review-judge-reformat-retry
matt/fix-bump-trailing-newline
matt/bump-kimi-k2.7-code
matt/be-1916-harden-judge-parse
matt/be-1891-cursor-review-blocking
matt/bump-callers-workflow-dispatch
matt/fix-bump-callers-yaml-parse
matt/be-1812-story-11-cursor-review-reusable-workflow-optional-dedicated
fix/cursor-review-fork-pr-403
ci/auto-bump-cursor-review-callers
chore/cursor-review-opus-4-8
matt/auto-label-open-ready
chore/ignore-pycache-cursor-review
docs/cursor-review-readme
feat/cursor-review-auto-label-reusable
chore/cursor-review-deroster
v1
Labels
Clear labels   
cursor-review-catalog-drift

   
groom

   
groom

   
pull-request
Mirrored from GitHub Pull Request

No labels
cursor-review-catalog-drift
groom
groom
pull-request
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Comfy-Org/github-workflows#413
No description provided.