[PR #395] [MERGED] feat(cursor-approve): make the card say WHY, and split "nothing to judge" out of yellow #398

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/395
Author: @mattmillerai
Created: 10/9/2026
Status: ✅ Merged
Merged: 10/9/2026
Merged by: @mattmillerai

Base: main ← Head: matt/axis-na-and-headline

📝 Commits (2)

5e41a5a feat(cursor-approve): make the card say WHY, and split "nothing to judge" out of yellow
0b45dfe test(cursor-approve): pin that the headline reaches the uploaded artifact

📊 Changes

9 files changed (+460 additions, -54 deletions)

View changed files

📝 .github/cursor-approve/README.md (+32 -4)
📝 .github/cursor-approve/aggregate.py (+48 -12)
📝 .github/cursor-approve/card.py (+79 -8)
📝 .github/cursor-approve/prompt-common.md (+42 -11)
📝 .github/cursor-approve/tests/test_aggregate.py (+111 -2)
📝 .github/cursor-approve/tests/test_card.py (+99 -5)
📝 .github/workflows/cursor-approve.yml (+4 -1)
📝 .github/workflows/cursor-axis-base.yml (+5 -2)
📝 docs/callers/cursor-approve.md (+40 -9)

📄 Description

The problem, from the card itself

This is what a reader got on cloud#12712:

Axis	Verdict	Confidence	Summary
correctness	🟢 green	0.90	Checked the only changed file.
conformance	🟡 yellow	0.60	I read root AGENTS.md and CLAUDE.md.

Neither cell says anything. The yellow is the worse half: the PR is blocked and
the card gives no reason for it.

Two separate causes.

1. The column was fed the wrong field. It rendered the first sentence of
summary, and summary is where a model writes its process log. The prompt
even asked for that ("Say what you checked and what decided the verdict" — the
models answer the first half). So the card reported what the axis did.

2. yellow meant three different things. prompt-common.md defined it as
"you are uncertain, or you found a material concern", and added "When the
evidence is thin, answer yellow with low confidence". So "I found a problem",
"I couldn't tell", and "nothing on my axis applies to this change" all came out
yellow and all blocked identically. The screenshot's conformance yellow is the
third kind — that is why no concern is named: there isn't one.

What this changes

n/a becomes its own verdict. For an axis with no purchase on the change.
It counts as REPORTING (so it is not a missing axis, which withholds) but not
as a yellow (so it does not block), and the prompt requires it to name what the
axis looked for and where — an n/a that doesn't is untrusted.

It is deliberately not a green. green means "I checked and it's clean";
letting "no applicable standard" claim that would quietly approve a conformance
gap the day guidance does appear, and nobody would learn the axis was never
exercised. And a round where every axis reports n/a is still withheld —
nothing was judged, so there is nothing to approve on. That is the one way this
could have become a path to a free approval, and it is closed.

A required headline. One line, ≤100 chars, stating what decided the
verdict. The prompt gives per-verdict examples and bans the openers models
reach for:

red / yellow → the concern itself, and where. Not "reviewed the migration"
but "Backfill rewrites rows the API still reads."

green → what you verified holds. Not "checked the changed file" but "Both
new branches handle an empty payload."

n/a → what you looked for and did not find. Not "no issues" but "No
AGENTS.md governs .github/."

Never begin with "Checked", "Read", "Reviewed", "Looked at", "I read" or
"Verified that I" — those spend the line on process. Begin with the finding.

A missing or over-long headline makes that axis untrusted, not truncated —
truncating would reproduce the reason-free row one step later.

The card becomes three columns, and the detail moves behind the link:

Axis	Verdict	Why
correctness	🟢 green	Both new branches handle an empty payload.
conformance	⚪ n/a	No AGENTS.md governs `.github/`.

Confidence goes with it. 0.60 on its own does not help anyone understand a
verdict, and it was taking a column from the one thing that does.

The run link now leads somewhere. No axis wrote GITHUB_STEP_SUMMARY
before, so "workflow run" led to a job list. The decide step now writes the
full summaries and confidences there. Card = the glance; job summary = the
record.

Scope

No input changes, no caller changes, no permission changes. aggregate.py extract already passed the whole object through, so headline flows end to
end without touching cursor-axis-base.yml beyond its header comment.

The one behavioural tightening: an axis that returns no headline is untrusted,
which withholds the approval. That is the intended failure mode (it is how
every other malformed field already behaves) and it only fires for an axis
running a prompt older than its aggregate.py — impossible here, since both
load from the same pinned ref.

Tests

196 passed in .github/cursor-approve/tests (+22), cursor-review's 1280
unaffected, check_workflow_pins.py and check_agents_md.py clean.

New: the headline contract (required / blank / non-string / the cap / the
whitespace fold / the folded length being what's measured) and the n/a rules
(not counted as yellow, named in the reason, all-n/a withholds, does not rescue
a red, does not excuse a missing axis, exact spelling) in test_aggregate.py;
the three-column card, the ⚪ icon, the rule line, the summary fallback, and the
job summary — including that card.py decide actually writes it to
GITHUB_STEP_SUMMARY — in test_card.py.

Card sanitization moved to the headline, since that is now the only model text
the card renders; the job summary's own sanitization is covered too (a #
heading, a mention and a forged card marker in a summary).

Not in this PR

The axes receive no prior-round context at all — build-ledger.py feeds
the review panel, and nothing feeds the axes — so a yellow the author has
already addressed comes back every round, and the card is a plain issue comment
that the ledger's reviews + reviewThreads query never sees either. Worth
fixing, separately.

🤖 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/395 **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/axis-na-and-headline` --- ### 📝 Commits (2) - [`5e41a5a`](https://github.com/Comfy-Org/github-workflows/commit/5e41a5ab58ffd7834807d580638218e8d57ae86f) feat(cursor-approve): make the card say WHY, and split "nothing to judge" out of yellow - [`0b45dfe`](https://github.com/Comfy-Org/github-workflows/commit/0b45dfe6eb7cf48a87909908cf04c01e85719b9c) test(cursor-approve): pin that the headline reaches the uploaded artifact ### 📊 Changes **9 files changed** (+460 additions, -54 deletions) <details> <summary>View changed files</summary> 📝 `.github/cursor-approve/README.md` (+32 -4) 📝 `.github/cursor-approve/aggregate.py` (+48 -12) 📝 `.github/cursor-approve/card.py` (+79 -8) 📝 `.github/cursor-approve/prompt-common.md` (+42 -11) 📝 `.github/cursor-approve/tests/test_aggregate.py` (+111 -2) 📝 `.github/cursor-approve/tests/test_card.py` (+99 -5) 📝 `.github/workflows/cursor-approve.yml` (+4 -1) 📝 `.github/workflows/cursor-axis-base.yml` (+5 -2) 📝 `docs/callers/cursor-approve.md` (+40 -9) </details> ### 📄 Description ## The problem, from the card itself This is what a reader got on cloud#12712: | Axis | Verdict | Confidence | Summary | |---|---|---|---| | correctness | 🟢 green | 0.90 | Checked the only changed file. | | conformance | 🟡 yellow | 0.60 | I read root AGENTS.md and CLAUDE.md. | Neither cell says anything. The yellow is the worse half: the PR is blocked and the card gives no reason for it. Two separate causes. **1. The column was fed the wrong field.** It rendered the first sentence of `summary`, and `summary` is where a model writes its process log. The prompt even asked for that ("Say what you checked and what decided the verdict" — the models answer the first half). So the card reported what the axis *did*. **2. `yellow` meant three different things.** `prompt-common.md` defined it as "you are uncertain, or you found a material concern", and added "When the evidence is thin, answer `yellow` with low confidence". So "I found a problem", "I couldn't tell", and "nothing on my axis applies to this change" all came out yellow and all blocked identically. The screenshot's conformance yellow is the third kind — that is why no concern is named: there isn't one. ## What this changes **`n/a` becomes its own verdict.** For an axis with no purchase on the change. It counts as REPORTING (so it is not a missing axis, which withholds) but not as a yellow (so it does not block), and the prompt requires it to name what the axis looked for and where — an `n/a` that doesn't is untrusted. It is deliberately **not** a `green`. `green` means "I checked and it's clean"; letting "no applicable standard" claim that would quietly approve a conformance gap the day guidance does appear, and nobody would learn the axis was never exercised. And a round where **every** axis reports `n/a` is still withheld — nothing was judged, so there is nothing to approve on. That is the one way this could have become a path to a free approval, and it is closed. **A required `headline`.** One line, ≤100 chars, stating *what decided the verdict*. The prompt gives per-verdict examples and bans the openers models reach for: > - red / yellow → the concern itself, and where. Not "reviewed the migration" > but "Backfill rewrites rows the API still reads." > - green → what you verified holds. Not "checked the changed file" but "Both > new branches handle an empty payload." > - n/a → what you looked for and did not find. Not "no issues" but "No > AGENTS.md governs .github/." > > Never begin with "Checked", "Read", "Reviewed", "Looked at", "I read" or > "Verified that I" — those spend the line on process. Begin with the finding. A missing or over-long headline makes that axis **untrusted**, not truncated — truncating would reproduce the reason-free row one step later. **The card becomes three columns**, and the detail moves behind the link: | Axis | Verdict | Why | |---|---|---| | correctness | 🟢 green | Both new branches handle an empty payload. | | conformance | ⚪ n/a | No AGENTS.md governs `.github/`. | Confidence goes with it. `0.60` on its own does not help anyone understand a verdict, and it was taking a column from the one thing that does. **The run link now leads somewhere.** No axis wrote `GITHUB_STEP_SUMMARY` before, so "workflow run" led to a job list. The decide step now writes the full summaries and confidences there. Card = the glance; job summary = the record. ## Scope No input changes, no caller changes, no permission changes. `aggregate.py extract` already passed the whole object through, so `headline` flows end to end without touching `cursor-axis-base.yml` beyond its header comment. The one behavioural tightening: an axis that returns no `headline` is untrusted, which withholds the approval. That is the intended failure mode (it is how every other malformed field already behaves) and it only fires for an axis running a prompt older than its `aggregate.py` — impossible here, since both load from the same pinned ref. ## Tests `196 passed` in `.github/cursor-approve/tests` (`+22`), cursor-review's `1280` unaffected, `check_workflow_pins.py` and `check_agents_md.py` clean. New: the headline contract (required / blank / non-string / the cap / the whitespace fold / the folded length being what's measured) and the n/a rules (not counted as yellow, named in the reason, all-n/a withholds, does not rescue a red, does not excuse a missing axis, exact spelling) in `test_aggregate.py`; the three-column card, the ⚪ icon, the rule line, the summary fallback, and the job summary — including that `card.py decide` actually writes it to `GITHUB_STEP_SUMMARY` — in `test_card.py`. Card sanitization moved to the headline, since that is now the only model text the card renders; the job summary's own sanitization is covered too (a `#` heading, a mention and a forged card marker in a `summary`). ## Not in this PR The axes receive **no prior-round context at all** — `build-ledger.py` feeds the review panel, and nothing feeds the axes — so a yellow the author has already addressed comes back every round, and the card is a plain issue comment that the ledger's `reviews` + `reviewThreads` query never sees either. Worth fixing, separately. 🤖 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:22 +00:00