Skip to content

feat(action): opt-in completeness-gated checkpoint ranges (#476) #26

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
chethanuk wants to merge 3 commits into
base: main
Choose a base branch
from
+1,286 −5
Open

feat(action): opt-in completeness-gated checkpoint ranges (#476) #26

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dd13584
feat(action): opt-in completeness-gated checkpoint ranges (#476)
chethanuk Aug 8, 2026
f91a7f6
fix(action): fingerprint custom rule contents, not just the path
chethanuk Aug 8, 2026
0d3b893
docs(action): list rule_unreadable in the checkpoint reason table
chethanuk Aug 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
162 changes: 161 additions & 1 deletion action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,25 @@ inputs:
with route_severity_below to route on either condition.
required: false
default: ''
checkpoint_range:
description: >-
Cross-push checkpoints. true = a run that reviewed everything it selected
records the head it covered in its sticky summary comment, and the next
run reviews only <checkpoint>..<new head> instead of
<merge-base>..<new head>. Fail-closed: if anything is in doubt — the
summary is missing or was not posted by this token, the marker is
unreadable, the base moved, the configuration changed, or git cannot prove
the checkpoint is an ancestor of the new head — the full range is reviewed
exactly as it is today. Requires sticky_summary; ignored without it.
required: false
default: 'false'
full_review:
description: >-
Force one full review even when checkpoint_range is enabled (reason
'manual_full_review'). Use it to re-review a PR from the merge-base
without turning checkpointing off; the run still records a new checkpoint.
required: false
default: 'false'
base_ref:
description: >-
Override the base ref. Provide this (and head_sha) when invoking from a
Expand Down Expand Up @@ -154,6 +173,24 @@ outputs:
summary_comment_url:
description: URL of the posted/updated summary comment, if any.
value: ${{ steps.post.outputs.summary_comment_url }}
range_mode:
description: >-
'checkpoint' when this run reviewed only the range since the previous
checkpoint, 'full' when it reviewed from the merge-base. Empty when
checkpoint_range is not enabled.
value: ${{ steps.range.outputs.range_mode }}
range_summary:
description: >-
The reviewed range plus the reason it was chosen, e.g.
"full (base_changed)" or "checkpoint (ok): <from>..<to>". Empty when
checkpoint_range is not enabled.
value: ${{ steps.range.outputs.range_summary }}
checkpoint_after:
description: >-
The head this run recorded as the new checkpoint, or empty when it did not
advance one (incomplete run, a finding failed to post, or the summary did
not publish).
value: ${{ steps.post.outputs.checkpoint_after }}

runs:
using: composite
Expand Down Expand Up @@ -252,6 +289,11 @@ runs:
npm install -g "@alibaba-group/open-code-review@${OCR_VERSION}"
echo "OpenCodeReview installed:"
ocr version || true
# Resolved version (not the spec, which is usually "latest"). It feeds
# the checkpoint fingerprint so an OCR upgrade invalidates checkpoints
# taken by the previous version.
VERSION_ACTUAL=$(ocr version 2>/dev/null | head -n 1 | tr -d '\r' || true)
echo "OCR_VERSION_ACTUAL=${VERSION_ACTUAL}" >> "$GITHUB_ENV"
Comment on lines +292 to +296

@coderabbitai coderabbitai Bot Aug 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Handle an empty resolved OCR version.

If ocr version writes to stderr only, or prints nothing, VERSION_ACTUAL becomes empty. The fingerprint then folds in '' for every run, so an OCR upgrade no longer invalidates a checkpoint. The documented guarantee "the resolved OCR version changed → config_changed" silently stops holding. Consider falling back to the version spec, or forcing a full review when the resolved version is empty.

🛡️ Proposed fallback 
-        VERSION_ACTUAL=$(ocr version 2>/dev/null | head -n 1 | tr -d '\r' || true)
+        VERSION_ACTUAL=$(ocr version 2>/dev/null | head -n 1 | tr -d '\r' || true)
+        # Never fingerprint an empty version: it would make every OCR upgrade
+        # look identical. Fall back to the requested spec.
+        VERSION_ACTUAL="${VERSION_ACTUAL:-spec:${OCR_VERSION}}"
         echo "OCR_VERSION_ACTUAL=${VERSION_ACTUAL}" >> "$GITHUB_ENV"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Resolved version (not the spec, which is usually "latest"). It feeds
# the checkpoint fingerprint so an OCR upgrade invalidates checkpoints
# taken by the previous version.
VERSION_ACTUAL=$(ocr version 2>/dev/null | head -n 1 | tr -d '\r' || true)
echo "OCR_VERSION_ACTUAL=${VERSION_ACTUAL}" >> "$GITHUB_ENV"
# Resolved version (not the spec, which is usually "latest"). It feeds
# the checkpoint fingerprint so an OCR upgrade invalidates checkpoints
# taken by the previous version.
VERSION_ACTUAL=$(ocr version 2>/dev/null | head -n 1 | tr -d '\r' || true)
# Never fingerprint an empty version: it would make every OCR upgrade
# look identical. Fall back to the requested spec.
VERSION_ACTUAL="${VERSION_ACTUAL:-spec:${OCR_VERSION}}"
echo "OCR_VERSION_ACTUAL=${VERSION_ACTUAL}" >> "$GITHUB_ENV"
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@action.yml` around lines 292 - 296, Update the VERSION_ACTUAL assignment in
the OCR version resolution block to handle an empty result explicitly: fall back
to the configured OCR version spec, or otherwise force checkpoint
invalidation/full review. Preserve the guarantee that a successfully changed
resolved OCR version changes the fingerprint, while avoiding an empty value
being written to GITHUB_ENV.


- name: Configure OCR
env:
Expand All @@ -262,6 +304,114 @@ runs:
ocr config set llm.extra_body "$OCR_EXTRA_BODY"
ocr config set language "$OCR_LANGUAGE"

- name: Resolve review range
if: inputs.checkpoint_range == 'true'
id: range
uses: actions/github-script@v9
env:
OCR_FULL_REVIEW: ${{ inputs.full_review }}
OCR_STICKY_SUMMARY: ${{ inputs.sticky_summary }}
# Everything that changes what a review would say. Any difference
# invalidates the checkpoint, because findings from the previous run are
# no longer comparable to what this configuration would produce.
OCR_FINGERPRINT_INPUTS: >-
${{ inputs.llm_url }}|${{ inputs.llm_model }}|${{ inputs.llm_use_anthropic }}|${{
inputs.language }}|${{ inputs.llm_extra_body }}|${{ inputs.rule }}|${{
inputs.route_severity_below }}|${{ inputs.route_categories }}
OCR_RULE_PATH: ${{ inputs.rule }}
with:
github-token: ${{ inputs.github_token }}
script: |
const fs = require('fs');
const path = require('path');
const crypto = require('crypto');
const { spawnSync } = require('child_process');
// Same helper lookup as the posting step below.
const REL = 'scripts/github-actions/post-review-comments.js';
const roots = [process.env.GITHUB_ACTION_PATH, process.env.GITHUB_WORKSPACE].filter(Boolean);
const helper = roots.map(r => path.resolve(r, REL)).find(p => fs.existsSync(p));
if (!helper) throw new Error(`Could not locate ${REL}; searched roots: ${roots.join(', ')}`);
const { resolveCheckpointRange, readCheckpointComment } = require(helper);

// `rule` names a JSON file that OCR reads off the workspace at review
// time (rules.NewResolver only touches disk when the path is non-empty;
// the default rule set is embedded in the binary and so already moves
// with OCR_VERSION_ACTUAL). Fingerprinting the *path* alone would let an
// edit to that file narrow the next range under rules the earlier
// commits were never reviewed against, so hash the contents too.
let ruleDigest = 'none';
let ruleUnverified = false;
const rulePath = process.env.OCR_RULE_PATH || '';
if (rulePath) {
try {
const abs = path.resolve(process.env.GITHUB_WORKSPACE || '.', rulePath);
ruleDigest = crypto.createHash('sha256').update(fs.readFileSync(abs)).digest('hex');
} catch (e) {
// Cannot prove the rules are unchanged -> do not narrow. OCR itself
// would normally have failed on an unreadable rule file before this
// step runs, so this is a belt-and-braces path.
ruleUnverified = true;
core.warning(`checkpoint: cannot read rule file ${rulePath} (${e.message}); forcing a full review.`);
}
}

const fingerprint = crypto.createHash('sha256')
.update(`${process.env.OCR_FINGERPRINT_INPUTS || ''}|${process.env.OCR_VERSION_ACTUAL || ''}|${ruleDigest}`)
.digest('hex')
.slice(0, 16);

// git's own ancestry verdict: 0 = ancestor, 1 = not, 128 = the object
// is not in this clone (shallow fetch, force-push, head_sha override).
// Anything else (git missing, signal) is a resolver error. Never
// treated as "ancestor" except on a literal 0.
const isAncestor = (a, b) =>
spawnSync('git', ['merge-base', '--is-ancestor', a, b], { cwd: process.env.GITHUB_WORKSPACE }).status;

const common = {
github,
owner: context.repo.owner,
repo: context.repo.repo,
prNumber: context.issue.number,
log: (m) => core.info(m),
};
// One read serves both purposes: the range decision, and the verbatim
// marker the posting step re-emits on a run that does not advance the
// checkpoint (the summary body is rewritten wholesale, which would
// otherwise erase it). Passing it into the resolver as `read` is what
// keeps this to a single listComments pagination per run.
const existing = await readCheckpointComment(common);
const range = await resolveCheckpointRange(Object.assign({}, common, {
read: existing,
enabled: true,
sticky: process.env.OCR_STICKY_SUMMARY === 'true',
fullReview: process.env.OCR_FULL_REVIEW === 'true',
headSha: process.env.HEAD_SHA || '',
baseRef: process.env.BASE_REF || '',
mergeBase: process.env.MERGE_BASE || '',
fingerprint,
isAncestor,
}));

// Last gate, applied after the ordered twelve: the rules this run will
// apply could not be read, so no stored fingerprint can be trusted to
// mean "same rules". Widening is always safe; narrowing is not.
if (ruleUnverified && range.mode === 'checkpoint') {
range.mode = 'full';
range.reason = 'rule_unreadable';
}

// Empty RANGE_FROM means "review the full range": the review step
// expands ${RANGE_FROM:-$MERGE_BASE}, so unset and empty behave alike.
core.exportVariable('RANGE_FROM', range.mode === 'checkpoint' ? range.from : '');
core.exportVariable('OCR_CONFIG_FINGERPRINT', fingerprint);
core.exportVariable('OCR_CHECKPOINT_CARRY', existing.raw || '');
const summary = range.mode === 'checkpoint'
? `checkpoint (${range.reason}): ${range.from}..${range.to}`
: `full (${range.reason})`;
core.setOutput('range_mode', range.mode);
core.setOutput('range_summary', summary);
core.info(`[checkpoint] reviewing ${summary}`);
Comment on lines +324 to +413

@coderabbitai coderabbitai Bot Aug 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Wrap the resolver in a try/catch so an unexpected error still reviews the full range.

The script has no top-level error handling. resolveCheckpointRange catches its own known failures, but any other throw here fails the whole step, and therefore the job, before ocr review runs. Examples: require(helper) fails, context.issue.number is undefined for a trigger without an issue payload, or core.exportVariable receives an unexpected value. Today, without checkpoint_range, none of these paths exist and the review always runs.

The documented contract is "when in doubt, review the full range". Catching here makes the step match that contract.

🛡️ Proposed fix 
-          const existing = await readCheckpointComment(common);
-          const range = await resolveCheckpointRange(Object.assign({}, common, {
-            read: existing,
-            enabled: true,
-            sticky: process.env.OCR_STICKY_SUMMARY === 'true',
-            fullReview: process.env.OCR_FULL_REVIEW === 'true',
-            headSha: process.env.HEAD_SHA || '',
-            baseRef: process.env.BASE_REF || '',
-            mergeBase: process.env.MERGE_BASE || '',
-            fingerprint,
-            isAncestor,
-          }));
+          let existing = { reason: 'resolver_error', payload: null, raw: '' };
+          let range = { mode: 'full', reason: 'resolver_error', from: '', to: process.env.HEAD_SHA || '' };
+          try {
+            existing = await readCheckpointComment(common);
+            range = await resolveCheckpointRange(Object.assign({}, common, {
+              read: existing,
+              enabled: true,
+              sticky: process.env.OCR_STICKY_SUMMARY === 'true',
+              fullReview: process.env.OCR_FULL_REVIEW === 'true',
+              headSha: process.env.HEAD_SHA || '',
+              baseRef: process.env.BASE_REF || '',
+              mergeBase: process.env.MERGE_BASE || '',
+              fingerprint,
+              isAncestor,
+            }));
+          } catch (e) {
+            // Widening is always safe; a resolver crash must not stop the review.
+            core.warning(`checkpoint: range resolution failed (${e.message}); reviewing the full range.`);
+          }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
script: |
const fs = require('fs');
const path = require('path');
const crypto = require('crypto');
const { spawnSync } = require('child_process');
// Same helper lookup as the posting step below.
const REL = 'scripts/github-actions/post-review-comments.js';
const roots = [process.env.GITHUB_ACTION_PATH, process.env.GITHUB_WORKSPACE].filter(Boolean);
const helper = roots.map(r => path.resolve(r, REL)).find(p => fs.existsSync(p));
if (!helper) throw new Error(`Could not locate ${REL}; searched roots: ${roots.join(', ')}`);
const { resolveCheckpointRange, readCheckpointComment } = require(helper);
// `rule` names a JSON file that OCR reads off the workspace at review
// time (rules.NewResolver only touches disk when the path is non-empty;
// the default rule set is embedded in the binary and so already moves
// with OCR_VERSION_ACTUAL). Fingerprinting the *path* alone would let an
// edit to that file narrow the next range under rules the earlier
// commits were never reviewed against, so hash the contents too.
let ruleDigest = 'none';
let ruleUnverified = false;
const rulePath = process.env.OCR_RULE_PATH || '';
if (rulePath) {
try {
const abs = path.resolve(process.env.GITHUB_WORKSPACE || '.', rulePath);
ruleDigest = crypto.createHash('sha256').update(fs.readFileSync(abs)).digest('hex');
} catch (e) {
// Cannot prove the rules are unchanged -> do not narrow. OCR itself
// would normally have failed on an unreadable rule file before this
// step runs, so this is a belt-and-braces path.
ruleUnverified = true;
core.warning(`checkpoint: cannot read rule file ${rulePath} (${e.message}); forcing a full review.`);
}
}
const fingerprint = crypto.createHash('sha256')
.update(`${process.env.OCR_FINGERPRINT_INPUTS || ''}|${process.env.OCR_VERSION_ACTUAL || ''}|${ruleDigest}`)
.digest('hex')
.slice(0, 16);
// git's own ancestry verdict: 0 = ancestor, 1 = not, 128 = the object
// is not in this clone (shallow fetch, force-push, head_sha override).
// Anything else (git missing, signal) is a resolver error. Never
// treated as "ancestor" except on a literal 0.
const isAncestor = (a, b) =>
spawnSync('git', ['merge-base', '--is-ancestor', a, b], { cwd: process.env.GITHUB_WORKSPACE }).status;
const common = {
github,
owner: context.repo.owner,
repo: context.repo.repo,
prNumber: context.issue.number,
log: (m) => core.info(m),
};
// One read serves both purposes: the range decision, and the verbatim
// marker the posting step re-emits on a run that does not advance the
// checkpoint (the summary body is rewritten wholesale, which would
// otherwise erase it). Passing it into the resolver as `read` is what
// keeps this to a single listComments pagination per run.
const existing = await readCheckpointComment(common);
const range = await resolveCheckpointRange(Object.assign({}, common, {
read: existing,
enabled: true,
sticky: process.env.OCR_STICKY_SUMMARY === 'true',
fullReview: process.env.OCR_FULL_REVIEW === 'true',
headSha: process.env.HEAD_SHA || '',
baseRef: process.env.BASE_REF || '',
mergeBase: process.env.MERGE_BASE || '',
fingerprint,
isAncestor,
}));
// Last gate, applied after the ordered twelve: the rules this run will
// apply could not be read, so no stored fingerprint can be trusted to
// mean "same rules". Widening is always safe; narrowing is not.
if (ruleUnverified && range.mode === 'checkpoint') {
range.mode = 'full';
range.reason = 'rule_unreadable';
}
// Empty RANGE_FROM means "review the full range": the review step
// expands ${RANGE_FROM:-$MERGE_BASE}, so unset and empty behave alike.
core.exportVariable('RANGE_FROM', range.mode === 'checkpoint' ? range.from : '');
core.exportVariable('OCR_CONFIG_FINGERPRINT', fingerprint);
core.exportVariable('OCR_CHECKPOINT_CARRY', existing.raw || '');
const summary = range.mode === 'checkpoint'
? `checkpoint (${range.reason}): ${range.from}..${range.to}`
: `full (${range.reason})`;
core.setOutput('range_mode', range.mode);
core.setOutput('range_summary', summary);
core.info(`[checkpoint] reviewing ${summary}`);
script: |
const fs = require('fs');
const path = require('path');
const crypto = require('crypto');
const { spawnSync } = require('child_process');
// Same helper lookup as the posting step below.
const REL = 'scripts/github-actions/post-review-comments.js';
const roots = [process.env.GITHUB_ACTION_PATH, process.env.GITHUB_WORKSPACE].filter(Boolean);
const helper = roots.map(r => path.resolve(r, REL)).find(p => fs.existsSync(p));
if (!helper) throw new Error(`Could not locate ${REL}; searched roots: ${roots.join(', ')}`);
const { resolveCheckpointRange, readCheckpointComment } = require(helper);
// `rule` names a JSON file that OCR reads off the workspace at review
// time (rules.NewResolver only touches disk when the path is non-empty;
// the default rule set is embedded in the binary and so already moves
// with OCR_VERSION_ACTUAL). Fingerprinting the *path* alone would let an
// edit to that file narrow the next range under rules the earlier
// commits were never reviewed against, so hash the contents too.
let ruleDigest = 'none';
let ruleUnverified = false;
const rulePath = process.env.OCR_RULE_PATH || '';
if (rulePath) {
try {
const abs = path.resolve(process.env.GITHUB_WORKSPACE || '.', rulePath);
ruleDigest = crypto.createHash('sha256').update(fs.readFileSync(abs)).digest('hex');
} catch (e) {
// Cannot prove the rules are unchanged -> do not narrow. OCR itself
// would normally have failed on an unreadable rule file before this
// step runs, so this is a belt-and-braces path.
ruleUnverified = true;
core.warning(`checkpoint: cannot read rule file ${rulePath} (${e.message}); forcing a full review.`);
}
}
const fingerprint = crypto.createHash('sha256')
.update(`${process.env.OCR_FINGERPRINT_INPUTS || ''}|${process.env.OCR_VERSION_ACTUAL || ''}|${ruleDigest}`)
.digest('hex')
.slice(0, 16);
// git's own ancestry verdict: 0 = ancestor, 1 = not, 128 = the object
// is not in this clone (shallow fetch, force-push, head_sha override).
// Anything else (git missing, signal) is a resolver error. Never
// treated as "ancestor" except on a literal 0.
const isAncestor = (a, b) =>
spawnSync('git', ['merge-base', '--is-ancestor', a, b], { cwd: process.env.GITHUB_WORKSPACE }).status;
const common = {
github,
owner: context.repo.owner,
repo: context.repo.repo,
prNumber: context.issue.number,
log: (m) => core.info(m),
};
// One read serves both purposes: the range decision, and the verbatim
// marker the posting step re-emits on a run that does not advance the
// checkpoint (the summary body is rewritten wholesale, which would
// otherwise erase it). Passing it into the resolver as `read` is what
// keeps this to a single listComments pagination per run.
let existing = { reason: 'resolver_error', payload: null, raw: '' };
let range = { mode: 'full', reason: 'resolver_error', from: '', to: process.env.HEAD_SHA || '' };
try {
existing = await readCheckpointComment(common);
range = await resolveCheckpointRange(Object.assign({}, common, {
read: existing,
enabled: true,
sticky: process.env.OCR_STICKY_SUMMARY === 'true',
fullReview: process.env.OCR_FULL_REVIEW === 'true',
headSha: process.env.HEAD_SHA || '',
baseRef: process.env.BASE_REF || '',
mergeBase: process.env.MERGE_BASE || '',
fingerprint,
isAncestor,
}));
} catch (e) {
// Widening is always safe; a resolver crash must not stop the review.
core.warning(`checkpoint: range resolution failed (${e.message}); reviewing the full range.`);
}
// Last gate, applied after the ordered twelve: the rules this run will
// apply could not be read, so no stored fingerprint can be trusted to
// mean "same rules". Widening is always safe; narrowing is not.
if (ruleUnverified && range.mode === 'checkpoint') {
range.mode = 'full';
range.reason = 'rule_unreadable';
}
// Empty RANGE_FROM means "review the full range": the review step
// expands ${RANGE_FROM:-$MERGE_BASE}, so unset and empty behave alike.
core.exportVariable('RANGE_FROM', range.mode === 'checkpoint' ? range.from : '');
core.exportVariable('OCR_CONFIG_FINGERPRINT', fingerprint);
core.exportVariable('OCR_CHECKPOINT_CARRY', existing.raw || '');
const summary = range.mode === 'checkpoint'
? `checkpoint (${range.reason}): ${range.from}..${range.to}`
: `full (${range.reason})`;
core.setOutput('range_mode', range.mode);
core.setOutput('range_summary', summary);
core.info(`[checkpoint] reviewing ${summary}`);
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@action.yml` around lines 324 - 413, Wrap the checkpoint-range setup and
output logic in the action script’s top-level try/catch, including helper
loading, readCheckpointComment, resolveCheckpointRange, and export/output calls.
On any unexpected error, log a warning and force the full-review fallback by
exporting an empty RANGE_FROM and setting checkpoint outputs consistently, so
the subsequent ocr review still runs.


- name: Run OpenCodeReview
env:
OCR_LLM_URL: ${{ inputs.llm_url }}
Expand All @@ -276,7 +426,7 @@ runs:
OCR_RULE: ${{ inputs.rule }}
shell: bash
run: |
ARGS=(--from "${MERGE_BASE}" --to "${HEAD_SHA}" --format json)
ARGS=(--from "${RANGE_FROM:-$MERGE_BASE}" --to "${HEAD_SHA}" --format json)
[ -n "$OCR_REVIEW_CONCURRENCY" ] && ARGS+=(--concurrency "$OCR_REVIEW_CONCURRENCY")
[ -n "$OCR_BACKGROUND" ] && ARGS+=(--background "$OCR_BACKGROUND")
[ -n "$OCR_RULE" ] && ARGS+=(--rule "$OCR_RULE")
Expand Down Expand Up @@ -316,6 +466,11 @@ runs:
OCR_REVIEW_COMMENT_BATCH_SIZE: ${{ inputs.review_comment_batch_size }}
OCR_ROUTE_SEVERITY_BELOW: ${{ inputs.route_severity_below }}
OCR_ROUTE_CATEGORIES: ${{ inputs.route_categories }}
# Set by the Resolve review range step; empty when checkpointing is off.
OCR_CHECKPOINT_CARRY: ${{ env.OCR_CHECKPOINT_CARRY }}
OCR_CONFIG_FINGERPRINT: ${{ env.OCR_CONFIG_FINGERPRINT }}
OCR_BASE_REF: ${{ env.BASE_REF }}
OCR_MERGE_BASE: ${{ env.MERGE_BASE }}
with:
github-token: ${{ inputs.github_token }}
script: |
Expand Down Expand Up @@ -346,4 +501,9 @@ runs:
reviewCommentBatchSize: parseInt(process.env.OCR_REVIEW_COMMENT_BATCH_SIZE, 10),
routeSeverityBelow: process.env.OCR_ROUTE_SEVERITY_BELOW,
routeCategories: process.env.OCR_ROUTE_CATEGORIES,
checkpointEnabled: ${{ inputs.checkpoint_range == 'true' }},
checkpointCarry: process.env.OCR_CHECKPOINT_CARRY || '',
checkpointBaseRef: process.env.OCR_BASE_REF || '',
checkpointMergeBase: process.env.OCR_MERGE_BASE || '',
checkpointFingerprint: process.env.OCR_CONFIG_FINGERPRINT || '',
});
48 changes: 48 additions & 0 deletions examples/github_actions/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,54 @@ The action posts a summary issue comment plus inline review comments. Two inputs

> `sticky_summary` and `incremental` must be quoted strings (`'true'`/`'false'`); the action compares them as strings, so an unquoted YAML boolean will not match.

### Review only what changed since the last run (checkpoints)

`incremental` filters the comments a run produces; it still reviews the whole `merge-base..head` diff every time. On a long-lived PR that means re-reading the same 40 commits on every push. `checkpoint_range` fixes the other half: a run that reviewed everything it selected records the head it covered in a hidden marker inside its sticky summary comment, and the next run reviews `<that head>..<new head>` instead.

| Input | Default | Description |
|-------|---------|-------------|
| `checkpoint_range` | `'false'` | Review only the range since the last recorded checkpoint. Requires `sticky_summary: 'true'` (the checkpoint lives in that comment). |
| `full_review` | `'false'` | Force one full review even with `checkpoint_range` enabled. The run still records a new checkpoint. |

```yaml
- uses: alibaba/open-code-review@main
with:
sticky_summary: 'true'
checkpoint_range: 'true'
```

**When in doubt, this reviews the full range.** A checkpoint is used only when every one of these holds; otherwise the run reviews `merge-base..head` exactly as it does today, and the reason is reported in the `range_summary` output and the step log:

| Reason | The run reviewed the full range because |
|--------|------------------------------------------|
| `disabled` | `checkpoint_range` is not `'true'` |
| `sticky_disabled` | `sticky_summary` is not `'true'`, so there is nowhere durable to keep a checkpoint |
| `manual_full_review` | `full_review: 'true'` was requested |
| `no_summary_comment` | the PR has no sticky summary yet (the first run) |
| `author_unverified` | the summary comment was not posted by this workflow's token |
| `corrupt_checkpoint` | the summary carries no readable checkpoint marker (absent, malformed, or two of them) |
| `schema_invalid` | the marker is for another PR, another marker version, or records a run that did not complete |
| `base_changed` | the base ref or the merge-base moved, so the diff basis is no longer the one the checkpoint was taken against |
| `config_changed` | the model, language, `llm_extra_body`, rules, routing inputs, or the resolved OCR version changed |
| `not_ancestor` | the checkpoint is not an ancestor of the new head (force-push, rebase) |
| `unknown_object` | the checkpoint commit is not in this clone, so ancestry could not be checked |
| `rule_unreadable` | a `rule` path was given but could not be read, so no stored fingerprint can be trusted to mean "same rules" |
| `resolver_error` | the comment could not be read, or `git merge-base --is-ancestor` could not run |

Three outputs report what happened: `range_mode` (`checkpoint` or `full`), `range_summary` (the mode, the reason, and the range), and `checkpoint_after` (the head recorded as the new checkpoint, or empty when the run did not advance one).

Two properties are worth knowing before you enable it:

- **Widen-only.** The start of the range only ever moves back. An older checkpoint produces a wider review, never a narrower one, and a checkpoint only advances past a run whose manifest reported `terminal_state: complete`, which failed to post nothing, and whose summary comment actually published. A run that fails halfway carries the previous checkpoint forward unchanged rather than skipping the range it did not review.
- **Same-head reruns are empty.** Re-running the workflow without pushing produces a `checkpoint` range with `from == to` — nothing new to review.
- **The sticky summary shows the latest range, not the whole PR.** The summary comment is rewritten on every run, so findings it reported for an earlier range (findings with no line information, routed findings, warnings) are replaced by the new range's. Inline review comments are separate comments and stay. If you rely on the summary as a running list for the whole PR, use `full_review: 'true'` to rebuild it, or leave `checkpoint_range` off.
Comment on lines +198 to +202

@coderabbitai coderabbitai Bot Aug 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Fix the item count and the double negative.

Line 198 says "Two properties", but three bullets follow. Line 200 contains "which failed to post nothing", which reads as a double negative.

✏️ Proposed fix 
-Two properties are worth knowing before you enable it:
+Three properties are worth knowing before you enable it:
 
-- **Widen-only.** The start of the range only ever moves back. An older checkpoint produces a wider review, never a narrower one, and a checkpoint only advances past a run whose manifest reported `terminal_state: complete`, which failed to post nothing, and whose summary comment actually published. A run that fails halfway carries the previous checkpoint forward unchanged rather than skipping the range it did not review.
+- **Widen-only.** The start of the range only ever moves back. An older checkpoint produces a wider review, never a narrower one. A checkpoint advances only past a run whose manifest reported `terminal_state: complete`, whose findings all posted, and whose summary comment actually published. A run that fails halfway carries the previous checkpoint forward unchanged rather than skipping the range it did not review.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Two properties are worth knowing before you enable it:
- **Widen-only.** The start of the range only ever moves back. An older checkpoint produces a wider review, never a narrower one, and a checkpoint only advances past a run whose manifest reported `terminal_state: complete`, which failed to post nothing, and whose summary comment actually published. A run that fails halfway carries the previous checkpoint forward unchanged rather than skipping the range it did not review.
- **Same-head reruns are empty.** Re-running the workflow without pushing produces a `checkpoint` range with `from == to` — nothing new to review.
- **The sticky summary shows the latest range, not the whole PR.** The summary comment is rewritten on every run, so findings it reported for an earlier range (findings with no line information, routed findings, warnings) are replaced by the new range's. Inline review comments are separate comments and stay. If you rely on the summary as a running list for the whole PR, use `full_review: 'true'` to rebuild it, or leave `checkpoint_range` off.
Three properties are worth knowing before you enable it:
- **Widen-only.** The start of the range only ever moves back. An older checkpoint produces a wider review, never a narrower one. A checkpoint advances only past a run whose manifest reported `terminal_state: complete`, whose findings all posted, and whose summary comment actually published. A run that fails halfway carries the previous checkpoint forward unchanged rather than skipping the range it did not review.
- **Same-head reruns are empty.** Re-running the workflow without pushing produces a `checkpoint` range with `from == to` — nothing new to review.
- **The sticky summary shows the latest range, not the whole PR.** The summary comment is rewritten on every run, so findings it reported for an earlier range (findings with no line information, routed findings, warnings) are replaced by the new range's. Inline review comments are separate comments and stay. If you rely on the summary as a running list for the whole PR, use `full_review: 'true'` to rebuild it, or leave `checkpoint_range` off.
🧰 Tools
🪛 LanguageTool

[grammar] ~200-~200: Ensure spelling is correct
Context: ..._state: complete`, which failed to post nothing, and whose summary comment actually pub...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@examples/github_actions/README.md` around lines 198 - 202, Update the README
text introducing the checkpoint behavior to say “Three properties” instead of
“Two properties,” and revise the phrase “which failed to post nothing” in the
Widen-only bullet to clearly state that the run posted no findings. Preserve the
surrounding behavior descriptions and bullet structure.

Source: Linters/SAST tools


> **Caveat — what `complete` covers.** `terminal_state: complete` means nothing in the set the run *selected* failed. Items the run waived, or excluded before selection (unsupported files, size limits), are inside that guarantee. So a checkpoint means "everything this configuration chose to review was reviewed", not "every byte of the diff was read". Changing the configuration invalidates the checkpoint (`config_changed`), which is what keeps that promise honest across runs.

> **Custom rules are fingerprinted by content.** If you pass `rule`, the checkpoint fingerprint covers that file's *contents*, not just its path — editing your rule file invalidates the checkpoint (`config_changed`) so the next run re-reviews from the merge-base under the new rules, rather than narrowing to the newest commits. The built-in rule set is embedded in the binary and moves with `ocr_version`, which is already part of the fingerprint. A `rule` path that cannot be read forces a full review (`rule_unreadable`).

> **Caveat — the trust boundary is write permission.** The checkpoint is read only from a comment authored by this workflow's own token, verified against the API rather than by matching the `github-actions[bot]` name. That proves who *posted* the comment, not that its body is unmodified: anyone with write permission on the repository can edit a bot comment and move the checkpoint forward, causing a range to be skipped. The boundary this buys is "write-permission holders are trusted" — a fork contributor, who is exactly the untrusted party under `pull_request_target`, cannot plant or alter a marker. If that is not an acceptable assumption for your repository, leave `checkpoint_range` off.

### Adjust retry and delay settings

When posting review comments individually (fallback mode), the action honors GitHub rate-limit headers (`retry-after`, `x-ratelimit-*`) with exponential backoff. The retry strategy follows GitHub's documented guidance for REST API rate limits — see [Rate limits for the REST API](https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api?apiVersion=2026-03-10) for details on primary/secondary rate limits and recommended retry behavior:
Expand Down
Loading
Loading