feat(action): first-class review controls (effort, max_tokens_budget, llm_reasoning_effort) and live progress

[thxCode]

Description

This PR makes the review action tunable from workflow inputs, in three commits:

• effort / max_tokens_budget inputs (closes feat(action): expose --effort and --max-tokens-budget as action inputs #1147) — validated and normalized in the Validate inputs step, appended as --effort / --max-tokens-budget to the review command; both join the checkpoint config fingerprint.
• Opt-in live progress via stream_progress (closes feat(action): stream review progress to the workflow log instead of a long silent step #1148) — default behavior is unchanged (--audience agent, stderr captured to the log file, nothing streams). Setting stream_progress: 'true' drops --audience agent so [ocr] progress routes to stderr while the result JSON stays on stdout; a FIFO feeds a background tee that streams progress live into the workflow log and is awaited before the log file is read, so artifacts and the posting step always see a fully flushed capture. OCR_EXIT_CODE handling is unaffected in either mode.
• llm_reasoning_effort input (closes feat(action): first-class llm_reasoning_effort input for always-on-thinking models (e.g. GLM-5.3) #1149) — injected into the effective llm.extra_body via node in the Configure step (node, not jq: the Actions runtime guarantees node on PATH even inside container jobs), riding the existing extra_body merge so any published CLI supports it; an explicit reasoning_effort key in llm_extra_body wins. The llm_extra_body default is unchanged ({"thinking": {"type": "disabled"}}) — thinking stays disabled unless explicitly opted in. Malformed llm_extra_body JSON now fails the step with an actionable ::error:: naming the input.

Testing

• make check
• make test
• npm run test:github-actions — 36/36 action contract tests pass
• End-to-end exercised through a reusable workflow consuming this fork action, with max-tokens-budget and llm-reasoning-effort wired from caller workflows

Upgrade note: the three new fingerprint axes (effort, max_tokens_budget, llm_reasoning_effort) invalidate every existing checkpoint once on upgrade — the first run after upgrading re-reviews the full PR, then incremental resumes.

[CLAassistant]

All committers have signed the CLA.

[github-actions]

🔍 OpenCodeReview found 4 issue(s) in this PR.

• ✅ Successfully posted inline: 4 comment(s)

[thxCode]

@lizhengfeng101 , PTAL. If duplicated in the implementation or not in the milestone, it should be good to close. Thanks.

[wu21-web]

I appreciate your work. This can solve some important issues.

[wu21-web]

@thxCode Can you update the documentation, currently deployed at https://open-codereview.ai/docs/cicd

[thxCode]

@wu21-web Done — added an "Action inputs" section to the CI/CD docs page (pages/src/content/docs/{en,zh,ja,ko,ru}/integrations/ci.md) covering effort, max_tokens_budget, llm_reasoning_effort, and stream_progress, plus a copyable with: example and a link to the full input list in action.yml. Also corrected the stale Anthropic wording for llm_reasoning_effort (it now fails fast instead of being ignored). See 0b3f0b8.

[lizhengfeng101]

Four things to fix before this lands.

1. --effort needs CLI v1.10.0, but the action's floor is v1.9.6. The flag came in with a662400, first tagged v1.10.0; action.yml:357/366 still accepts 1.9.6+. So ocr_version: '1.9.8' + effort: high dies with unknown flag: --effort and the whole review fails. Gate it the way the auth_token_cmd floor is already gated, or at minimum state the minimum version in the input description.

2. stream_progress: 'true' has the same version problem, but silently. Progress only moved to stderr for human+json in 66d71b2 (v1.9.8). On 1.9.6/1.9.7 dropping --audience agent puts progress lines on stdout, interleaved with the result document — /tmp/ocr-result.json stops parsing and the failure surfaces later in the posting step with nothing pointing back at stream_progress. This one needs a floor more than the previous point does, since it degrades instead of failing.

3. The node merge assumes the parsed body is an object. llm_extra_body: 'null' throws an uncaught TypeError — the step fails, but with a stack trace instead of the ::error:: this block exists to produce. '[]' or '5' are worse: the assignment is silently lost and reasoning_effort never reaches the request. A type check right after JSON.parse covers both:

if (body === null || typeof body !== "object" || Array.isArray(body)) {
  console.error("::error::llm_extra_body must be a JSON object");
  process.exit(1);
}

4. stream_progress: '' is rejected, and that will bite callers. The common wiring is stream_progress: ${{ inputs.stream_progress }} from a reusable workflow, which evaluates to an empty string whenever the caller leaves their own input unset — so "not set" becomes a hard failure. effort and llm_reasoning_effort both treat empty as "use the default"; this should too.

Non-blocking, but worth a look: the fingerprint reads ${{ inputs.* }} rather than the normalized values already exported to $GITHUB_ENV, so HIGH vs high and 010000 vs 10000 produce different digests for identical runs; and max_tokens_budget doesn't need to be in the fingerprint at all — budgetExceeded always leaves a group undispatched, so the manifest is never complete and no checkpoint is recorded. #1157 has the reasoning written up.

[thxCode]

Thanks — all four addressed in ca54207:

1. effort floor: already gated since c8d2498 (rejects < v1.10.0 with ::error::); the input description now also states the v1.10.0 requirement.
2. stream_progress floor: gated on v1.9.8 — verified 66d71b2 first shipped in v1.9.8 via git tag --contains. Older versions now fail fast with ::error::The stream_progress input requires OpenCodeReview v1.9.8 or newer.
3. non-object extra_body: type check right after JSON.parse; null / [] / scalars now fail with ::error::llm_extra_body must be a JSON object.
4. empty stream_progress: normalized to the default (false), same as effort and llm_reasoning_effort.

Each is locked in by a contract test. On the fingerprint notes: agreed they belong with #1157 — keeping this PR to the input plumbing.

[lizhengfeng101]

Two things on this — the rest looks solid, and I checked the version floors (--effort first lands in v1.10.0, the stderr-progress commit in v1.9.8, --max-tokens-budget predates v1.9.6), those are all correct.

---

stream_progress: 'true' can silently eat the error banner

post-review-comments.js:227 dumps the whole stderr file into the summary comment when ocr-result.json fails to parse, and fencedBlock (:1802) doesn't truncate. That path was safe before, because --audience agent kept stderr down to actual error text. With the human audience it's the full progress stream, so on a decent-sized PR the body blows past GitHub's 65536-char comment limit, the post 422s, and you lose the error report — at exactly the moment you needed it.

Keeping the tail is enough (the thing that killed the run is the last thing written):

const MAX_COMMENT_STDERR_CHARS = 20000;

function tailForComment(text, limit = MAX_COMMENT_STDERR_CHARS) {
  const s = String(text || "");
  if (s.length <= limit) return s;
  return `[... ${s.length - limit} earlier characters truncated; see the ocr-stderr.log artifact ...]\n${s.slice(-limit)}`;
}

-    const stderr = safeRead(fs, stderrPath).trim();
+    const stderr = tailForComment(safeRead(fs, stderrPath).trim());

Strictly this is a pre-existing bug in a file you didn't touch, so a follow-up issue is fair — but it's ~10 lines and this PR is what makes it reachable, so I'd rather see it here. A test asserting the tail survives (not the head) would be worth adding.

---

Fingerprint uses the raw inputs instead of the normalized ones

OCR_FP_EFFORT / OCR_FP_MAX_TOKENS_BUDGET / OCR_FP_LLM_REASONING_EFFORT read inputs.*, but Validate inputs already normalized all three into $GITHUB_ENV. So effort: HIGH vs high, or max_tokens_budget: '0' vs '' vs '00', hash differently despite meaning the same thing — the checkpoint gets thrown away and the whole PR is re-reviewed. Slightly ironic for the token-budget feature.

-        OCR_FP_EFFORT: ${{ inputs.effort }}
-        OCR_FP_MAX_TOKENS_BUDGET: ${{ inputs.max_tokens_budget }}
+        OCR_FP_EFFORT: ${{ env.EFFORT }}
+        OCR_FP_MAX_TOKENS_BUDGET: ${{ env.MAX_TOKENS_BUDGET }}

Same for OCR_FP_LLM_REASONING_EFFORT. OCR_HEAD_SHA: ${{ env.HEAD_SHA }} a few lines up already does exactly this, and Validate inputs has no if: and runs before this step, so the ordering is fine. Happy for this one to be a follow-up if you'd rather keep the PR tight.

(One leftover either way: an inline reasoning_effort in llm_extra_body plus an empty llm_reasoning_effort produces the same effective body as the reverse, but two fingerprints. Could collapse both axes onto the merged EFFECTIVE_EXTRA_BODY since Configure runs first — probably not worth it now.)

Also worth a line in the release notes that the three new fingerprint axes invalidate every existing checkpoint once on upgrade.

[thxCode]

Both addressed in 08a6c27 (branch rebased onto main @ 7f8fa44):

1. stderr tail: the unparseable-result summary now posts only the last 20000 chars of stderr, with a truncation notice pointing at the ocr-stderr.log artifact — a test asserts the tail (the killing error) survives, not the head.
2. fingerprint normalization: OCR_FP_EFFORT / OCR_FP_MAX_TOKENS_BUDGET / OCR_FP_LLM_REASONING_EFFORT now read the normalized env values from Validate inputs, so HIGH vs high and '0' vs '' hash identically. Locked in by a contract test asserting all three axes read env.*.

Left the merged-extra_body axis collapse out as you suggested — not worth it now. On the release note: the PR body's release-note block mentions the one-time checkpoint invalidation from the new fingerprint axes.

[lizhengfeng101]

LGTM