Skip to content

Show bounded inline review bodies without losing Markdown code - #439

Open
avifenesh wants to merge 7 commits into
mainfrom
tower-crane/T452-workers-read-the-full-text-of-open-revie
Open

avifenesh wants to merge 7 commits into
mainfrom
tower-crane/T452-workers-read-the-full-text-of-open-revie

Conversation

@avifenesh

@avifenesh avifenesh commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Worker ci checks, submit refusals and rework prompts show each open revuto inline finding's id, path:line and up to 4000 Unicode body characters with an explicit truncation note. The reader removes only line-start HTML comment blocks outside Markdown code, preserving literal prose markers, code spans, indented code and fenced suggestions. Text prints one note per finding with removed blocks, [n characters of HTML comment removed], where n is the total Unicode characters removed across all its blocks. ci checks --json retains the complete body after block removal and the nonzero total in html_comment_chars_removed. Resolved and outdated findings remain omitted.

The merge with main retains its ci dispatch feedback in ci checks, its CI-gate dispatch filtering helpers, and both sets of test imports. Command help and the state contract describe both finding output and dispatched-run feedback; docs/cli.md was regenerated. GitHub reports the pushed PR head as mergeable.

Validation: all 80 tests passed in test/ci-feedback.test.js, test/submit-ci.test.js and test/gates/ci.test.js. These include worker finding text/JSON, submit refusals, rework/fallback prompts, dispatched-run feedback and exclusion of dispatched jobs from PR checks. The 29-row Markdown table and five-row removal-total probe remain green. npm run check:shared -- --base origin/main passed, and the working tree is clean.

Limits: test/live-git-sandbox.test.js and test/live-sandbox.test.js remain for the host tests gate; cleanup runs through the clean gate. The reviewer's parser-performance, raw-severity and extra-visible-metadata observations remain follow-ups. This revision resolves the prepared merge without adding behavior outside the existing task and main's merged changes.


Note

Low Risk
Worker-facing display and parsing of review comment bodies only; submission blocking rules for P1/P2 findings are unchanged aside from clearer text.

Overview
Revuto inline findings are now normalized before workers see them in ci checks, submit refusals, and rework prompts.

A new findingBody path strips terminal controls and removes only line-start HTML comment blocks outside Markdown (fenced blocks, indented code, and inline code spans stay intact). Text output caps each body at 4000 Unicode characters with explicit truncation and one aggregate [n characters of HTML comment removed] note per finding; ci checks --json keeps the full post-stripped body plus optional html_comment_chars_removed. Resolved and outdated findings are still omitted.

CLI help, docs/cli.md, docs/state.md, and changelog.d/T452.md describe the contract; test/ci-feedback.test.js and test/submit-ci.test.js cover formatting, caps, and end-to-end worker/submit paths.

Reviewed by Cursor Bugbot for commit a371ded. Configure here.

Pre-submit review

  • Head: a371ded
  • Fresh context: no (worker dispatch forbids starting other agents).
  • Acceptance: worker broker ci checks text/JSON and submit-refusal fixtures verify both open bodies, ids, paths, caps, aggregate removal notes and resolved/outdated omissions; all 80 tests in the three affected files passed.
  • Claims and contracts: command description and state docs combine T452 finding output with main's dispatched-run feedback, and generated CLI docs pass shared checks. The reader sums removal counts once; the formatter prints one aggregate note per finding. The removal-total probe passed all five behavior rows and prose copies.
  • Edge cases and scope: the 29-row Markdown table, cap/Unicode rows, resolution/read-error cases, rework/fallback snapshots, dispatched-run feedback and dispatch exclusion passed. Hand-written conflict resolutions combine both sides; the task diff against main remains the same seven feature, test and contract files. Git reports no unmerged paths, and GitHub reports MERGEABLE at this head.
  • Rework coverage: merge conflicts were in bin/tower-crane.js, docs/state.md, lib/gates/ci.js and test/ci-feedback.test.js. Both finding and dispatch contracts, exports and imports are retained; docs were regenerated. The three-file test run passed 80/80, including both sets of CLI behavior. Prior Markdown context-loss blocker remains covered by the 29-row table and worker/submit/rework regressions. Prior per-block prose blocker remains covered by the exact two-block/21-character row and five-row aggregate-note/prose probe. Earlier before/after evidence remains recorded in the task. Host-only files remain pending with the host gate.
  • Revuto at head: capped at two rounds and nonblocking under ci.capped_review; worker ci checks reported no open inline findings.
  • CI at head: pending (shared files and CodeQL); no blocking failing checks or open scanning alerts at the worker check read. Local shared checks passed.
  • Left open: no blocking findings. Parser performance, raw severity and extra visible metadata remain reviewer follow-ups; host-only tests and cleanup remain for host gates.
  • Result: pass

@cursor

cursor Bot commented Oct 20, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: abe38942-0247-485b-ac2e-d66d673e5366)

@revuto-review revuto-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an auto review done by revuto.


Revuto completed the review and found no evidence-backed concerns.

@avifenesh

Copy link
Copy Markdown
Contributor Author

Review (Tower Crane, clean context)
SHA: 13411d9

Acceptance: worker ci checks (text, through the broker) and the submit refusal list every open finding with its id, path:line and a body capped at 4000 code points with a truncation note. Outdated (line: null) and resolved findings are left out. test/ci-feedback.test.js and test/submit-ci.test.js cover this, and the tests gate shows them failing before the change and passing after it. The cap and the omissions are correct. The new HTML-comment stripping has the defect below.

Blocking findings

  1. lib/gates/ci.js:711-713 (findingBody) - the comment regex /<!--[\s\S]*?(?:-->|$)/g ignores Markdown code spans and fences, and an unclosed <!-- deletes everything to the end of the body. A finding that quotes <!-- in backticks loses the rest of its text in worker ci checks text and JSON, in submit reports and in rework prompts. Nothing tells the worker that text was removed. A ```suggestion block that contains a closed marker also loses that marker line. This repo's docs carry such markers (<!-- commands:Gates:start --> in docs/cli.md), so a worker who applies the suggestion by hand deletes the marker. This also makes the new contract text false: docs/cli.md / the ci checks description in bin/tower-crane.js:159 and changelog.d/T452.md say JSON keeps "complete visible bodies", but GitHub shows these characters and the JSON drops them. Reproduction, from the worktree:
    node -e 'const CI=require("./lib/gates/ci");
    console.log(JSON.stringify(CI.revutoLines([{id:1,path:"a.md",line:3,body:"Use the `<!--` opener check here.\nSecond paragraph with the actual fix."}])));
    console.log(JSON.stringify(CI.revutoLines([{id:1,path:"a.md",line:3,body:"```suggestion\n<!-- commands:Gates:start -->\n| a | b |\n```"}])))'
    ["  a.md:3 (comment 1)","    Use the `"]
    ["  a.md:3 (comment 1)","    ```suggestion","    ","    | a | b |","    ```"]
    
    revutoFindings uses the same findingBody, so findings[].body in ci checks --json is cut the same way. Suggested fix: leave fenced blocks and inline code spans alone, and remove an unclosed <!-- only where it starts an HTML block (at the start of a line), as CommonMark does. Add both inputs to the variant table.

Blocking count: 1

Follow-ups

  • lib/gates/ci.js:706 and :720 - findingBody runs once in revutoFindings and again in revutoLines, and it is not idempotent. For <!<!-- a -->-- b -->Visible, JSON keeps <!-- b -->Visible while the text shows Visible. Strip once, in the reader, and have revutoLines only cap.
  • lib/gates/ci.js:707 - severity is still read from the raw body. A [P1] tag that appears only inside an HTML comment blocks submit, but the body the worker sees no longer shows it. The refusal still names the comment id, so this is minor.

@cursor

cursor Bot commented Oct 20, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: 9b1b11ec-b960-419a-b1cb-3dc61bc96438)

@avifenesh avifenesh changed the title Strip and cap inline review bodies in worker output Show bounded inline review bodies without losing Markdown code Oct 20, 2026

@revuto-review revuto-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an auto review done by revuto.


Revuto completed the review and found no evidence-backed concerns.

@avifenesh

Copy link
Copy Markdown
Contributor Author

Review (Tower Crane, clean context)
SHA: b90a35c

Full review from base ea10d58 (revision 2). The tests, clean and CI gates are green at this sha, and the tests gate shows test/ci-feedback.test.js and test/submit-ci.test.js failing without the change.

Acceptance: met. Worker ci checks (text through the broker, and JSON) and the submit refusal list every open finding with its id, path:line and a body capped at 4000 code points, with a truncation note. Outdated (line: null) and resolved findings are left out. Rework prompts use the same revutoLines.

Prior blocker (reviewer-T452-1 at 13411d9)

  1. lib/gates/ci.js findingBody dropped Markdown code and cut the body at an unclosed <!--: fixed. The prior reproduction now keeps Use the `<!--` opener check here. with its second paragraph, and the ```suggestion block keeps <!-- commands:Gates:start -->. Only line-start blocks outside fences, spans and indented code are removed. An unclosed line-start block runs to the end of the body, as a CommonMark type-2 HTML block does, and it is now counted. The prior idempotence follow-up (<!<!-- a -->-- b -->Visible) is also fixed: the reader strips once and revutoLines only caps.

Blocking findings

  1. PR body, docs/cli.md:467 and docs/state.md:407 - these say "Every removal prints" / "Each removal prints" / "Each removal is reported as" [n characters of HTML comment removed]. The code prints one note per finding with the total. revutoLines (lib/gates/ci.js:787) appends a single line from html_comment_chars_removed, which findingBody sums across blocks. The multiple blocks row in test/ci-feedback.test.js asserts exactly that. docs/cli.md and docs/state.md are the contract, so a reader who sees [20 characters ...] concludes one 20-character block was removed. Reproduction:
    node -e 'const CI=require("./lib/gates/ci");console.log(CI.revutoLines([{id:1,path:"a",line:1,...CI.findingBody("<!-- a -->\nX\n<!-- bb -->\nY")}]).join("\n"))'
      a:1 (comment 1)
        X
    
        Y
        [21 characters of HTML comment removed]
    
    Fix the prose to say one note per finding with the total removed characters (the bin/tower-crane.js description and changelog.d/T452.md already read that way), or print one note per block.

Blocking count: 1

Follow-ups

  • lib/gates/ci.js:748-760 - the backtick branch slices the rest of the text, finds the paragraph end and spreads every backtick run in it for each run it meets, so a long single paragraph is quadratic. A 64 KiB body of ` pairs takes 12.4 s and `` takes 6.4 s; a realistic 60 KB single paragraph with code spans takes 0.67 s, and 4 KB takes 6 ms. Any login matching^revuto(?:-[\w-]+)?counts, so a commenter on the public repo can stallsubmit, ci checks` and rework dispatch for seconds per comment. Scan within the paragraph once, or index backtick runs by length.
  • lib/gates/ci.js:707 - severity is still read from the raw body. A [P1] inside a removed HTML comment blocks submit while the shown body lacks it. This was carried over from the prior review; the PR body names it as out of scope.
  • lib/gates/ci.js findingBody - a line-start <!-- inside a blockquote (> <!-- x -->), or inside an apparent multi-line code span that an HTML block would interrupt under CommonMark, is kept although GitHub hides it. This errs toward showing more text and loses nothing; noted only for completeness.

@cursor

cursor Bot commented Oct 20, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: 8ca5bc1e-aa23-48b5-a110-5f21786a5095)

@avifenesh

Copy link
Copy Markdown
Contributor Author

Review (Tower Crane, clean context)
SHA: ac0f2ca

Full review from base 26f0b38 (revision 3). The base moved since the prior review at b90a35c. Tests, clean and CI gates are green at this sha. The tests gate shows test/ci-feedback.test.js and test/submit-ci.test.js failing with lib/gates/ci.js and bin/tower-crane.js reverted.

Acceptance: met. Worker ci checks lists each open revuto finding with its id, path:line and a body capped at 4000 code points plus a truncation note. That covers text through the broker, and JSON. The submit refusal does the same through the shared revutoLines. Outdated (line: null) and resolved findings are omitted. Covered by test/ci-feedback.test.js (broker text/JSON, dry and real rework prompts) and test/submit-ci.test.js (two open bodies, one outdated, one resolved).

Prior blockers (reviewer-T452-2 at b90a35c)

  1. PR body: "every removal prints" a note: fixed. The body now says "Text prints one note per finding with removed blocks ... where n is the total Unicode characters removed across all its blocks."
  2. docs/cli.md submit paragraph: fixed. It now says "Text prints one note per finding with removed blocks ... the total removed characters across all its blocks". The ci checks command row and the bin/tower-crane.js description match.
  3. docs/state.md ci checks paragraph: fixed. It uses the same one-note-per-finding, total-count wording and adds "Each block contributes the characters from its opener through its closer or the end of the body."

The prior reproduction <!-- a -->\nX\n<!-- bb -->\nY prints X, ``, Y and one `[21 characters of HTML comment removed]` note. That matches the corrected prose and the new `reported multiple blocks around prose` row in `test/ci-feedback.test.js`. The rework commit 63d3f5d changes only prose and that table row; `lib/gates/ci.js` is unchanged since b90a35c.

Blocking findings

None

Blocking count: 0

Follow-ups

  • lib/gates/ci.js:748-760: the backtick branch rescans the rest of the paragraph for every backtick run, so it is quadratic in a long single paragraph. reviewer-T452-2 measured 12.4 s on a 64 KiB body of backtick pairs. Any revuto* login counts, so one comment can stall submit, ci checks and rework dispatch. Carried over.
  • lib/gates/ci.js:707: severity is still read from the raw body, so a [P1] inside a removed HTML comment blocks submit while the shown body lacks it. Carried over; the PR body names it as out of scope.
  • lib/gates/ci.js findingBody: a line-start <!-- inside a blockquote or list item (> <!-- x -->, - <!-- x -->) is kept although GitHub hides it. This errs toward showing more text. Carried over.

@cursor

cursor Bot commented Oct 21, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: 01ae4b12-226f-40fb-98b9-139699551cab)

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.

1 participant