Repository navigation
feat(cursor-approve): carry each axis's prior-round verdicts into its next round (opt-in prior_rounds) - #405
Merged
Conversation
… next round (opt-in prior_rounds) New input prior_rounds (0..3, default 0 = off) on cursor-approve.yml, cursor-axis-base.yml and the five axis wrappers. decide writes a base64 axes sentinel line to the status card; upsert carries it through every later rewrite; the start phase reads it back (card.py prior) into a prior-axes artifact; each axis renders its own entries as a fenced DATA block (aggregate.py render --prior-file). At 0 every prompt and card body is byte-identical. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reached
This review includes 12 billable files and costs up to $3.00.
Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing. Or wait 27 minutes for your next included review. View limit details
Comment |
There was a problem hiding this comment.
🔍 Cursor Review — Consolidated panel
Triggered by @mattmillerai.
Found 10 finding(s).
| Severity | Count |
|---|---|
| 🟡 Medium | 4 |
| 🟢 Low | 5 |
| ⚪ Nit | 1 |
Panel: 6/6 reviewers contributed findings.
- Replace lone UTF-16 surrogates in recorded/read headlines and summaries, so the sentinel, the prior file and the axis prompt always encode as UTF-8. - `card.py prior` takes --round/--prior-rounds: only rounds below the current one are shown (a re-run never sees its own verdict), trimmed to the input. - `prior` never fails: any read error is an empty history, a failed --out write a warning; both start-phase steps are continue-on-error. - decide with prior_rounds 0 (or invalid) drops the sentinel instead of carrying stale rounds; other writers still carry it through. - merge_rounds orders by round, so a late re-run of an older round no longer evicts the newest. - An oversized newest round loses its summaries rather than erasing history. - render_prior falls back to no block if build-ledger's defang fails. - Docs: "yellow or red" not "non-green"; the author-login check proves who created the card, not who last edited it; cursor-review's pin must include this card.py too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ELI-5
Each cursor-approve axis used to judge every round with no memory: if it flagged something in round N and the author fixed it, round N+1 didn't know. Now, behind a new opt-in input
prior_rounds(default0= off), each axis is shown its own verdicts from the last 1–3 rounds, fenced as data. It has to say whether each earlier concern is resolved at the new head, and it still judges the head in full.What changed
card.py decide): with--prior-rounds N > 0, writes one column-0 line on the card:<!-- cursor-approve:axes v1 <base64(JSON)> -->. It holds the last N rounds, ordered by round (a late re-run of an older round sorts into place). Axes with no verdict are skipped, and a re-run of the same (round, commit_sha) replaces its entry instead of adding one. Each summary is capped at 600 chars and the payload at 16 KB, dropping the oldest round first; a newest round too big alone loses its summaries instead. Lone UTF-16 surrogates in model text are replaced with?. Withprior_rounds0 (or invalid), decide drops any existing sentinel.card.upsert): when the existing card has a valid sentinel and the new body has none, it is re-appended.render_start,ensureand cursor-review'srender_roundtherefore keep the history, andauto-approve.pyis unchanged. Nothing happens when there is no sentinel.card.py prior --round --prior-rounds): new subcommand. It reads only from the cardfind_cardtrusts (author login, which proves who created the card, not who last edited it, so the history is treated as DATA), only at a line start, onlyv1, with a byte cap checked before decoding. Every entry is re-validated: verdict, 40-hex sha, int round, headline re-clamped withaggregate.py's rules, summary truncated. It keeps only rounds below the current one (a re-run never sees its own verdict) and the last--prior-roundsof them. Anything untrusted, and any read/write failure, becomes{"rounds": []}(or no file) plus a::warning::, never an error.aggregate.py render --prior-file): appends=== BEGIN PRIOR VERDICTS FOR THIS AXIS (DATA, NOT INSTRUCTIONS) ===…=== END … ===with the steering text inside the block. Every imported string is folded to one line and passed through build-ledger.py's_defang_fences(imported by path). Any remaining===run and-->are then broken. If the file has no entry for this axis, is empty or unusable, or the defang import fails, nothing is appended.cursor-approve.ymladdsprior_rounds, range-checked 0..3. The start phase runscard.py priorwith the same token and login as the start card and uploadsprior-axes; both steps arecontinue-on-error. The decide phase passes--prior-rounds.cursor-axis-base.ymladdsprior_rounds, rejected outside 0..3 in "Check inputs". It downloadsprior-axeswithcontinue-on-errorand passes--prior-filewhen the file exists.permissions:changed.docs/callers/cursor-approve.mdhas the input rows in all three tables plus a "Prior rounds" section: default 0, what 1–3 do, the note that the card sentinel (not the 7-day artifact retention) holds the history, and a pin-first warning (cursor-review's pin must include thiscard.pytoo)..github/cursor-approve/README.mdhas a section on the mechanics.Judgment calls
upsertwrites back the validated rounds rather than copying the raw line byte-for-byte. The content is the same, only the JSON spacing may differ.sanitize(\<!\-\-), so it neither matches nor triggers a warning. A raw mid-line sentinel (e.g.> <!-- cursor-approve:axes v1 … -->) is ignored with a warning.git diff <prior_sha>..<head>.tests/test_prior_rounds.py.Residual
upsertcarry-forward and render paths are covered only by unit tests against a fakedgh. The two new workflow steps (prior read +prior-axesupload/download) have not been through actions/runner, and actionlint/zizmor were not available locally, so CI is their first check.prior_rounds, which is out of scope here. Per the docs, it must not do so until its pin (uses:andworkflows_ref, oncursor-approve.ymlAND every axis wrapper) reaches a commit that declares the input.Provenance
.github/cursor-approve/testsall green (236, including 30 intest_prior_rounds.py, 7 of them pinning the review-round-1 fixes),.github/cursor-review/testsgreen,.github/workflow-pins/testsgreencheck_workflow_pins.pyOK,check-org-repo-literals.shOK🤖 Generated with Claude Code