Open Code Review Toolkit: making repeat OCR reviews work in GitLab CI

[xeonvs]

Hi OCR community,

We actively develop Open Code Review Toolkit, an unofficial open-source toolkit for running OCR in GitLab merge request pipelines. We recently released version 0.6.0, and I wanted to share the practical problem we are trying to solve.

Running OCR once in CI is fairly straightforward: invoke the CLI with --format json and consume the structured result. The harder questions tend to appear on the second run:

• What should happen to findings from the previous review?
• How do we preserve a discussion once a person has joined it?
• How can a team give OCR useful repository context and record a tradeoff it has already accepted?
• How do we stop changes in the branch under review from declaring themselves trusted instructions?

OCR remains the review engine. The toolkit handles the surrounding workflow: preparing repository context, configuring and invoking the CLI, processing its JSON output, and publishing the result to GitLab.

A minimal run looks like this:

ocr-ci preflight
ocr-ci configure
ocr-ci review --result /tmp/ocr-result.json \
  --stderr /tmp/ocr-stderr.log \
  -- --from "$CI_MERGE_REQUEST_DIFF_BASE_SHA" \
     --to "$CI_MERGE_REQUEST_SOURCE_BRANCH_SHA" \
     --format json
ocr-ci post --result /tmp/ocr-result.json \
  --stderr /tmp/ocr-stderr.log

The OCR binary is installed separately and pinned by version and checksum. Toolkit 0.6.0 is qualified for OCR 1.9.3.

Treating a rerun as part of the same conversation

We did not want every OCR run to add another layer of stale comments.

After a successful rerun, the toolkit replaces only untouched, bot-owned OCR discussions. If someone has replied to a thread, or resolved it in GitLab, the conversation is preserved and a matching finding is not posted again.

Reviewers can also reply inside an OCR discussion with one of two commands:

• /ocr suppress keeps the discussion open but suppresses a matching finding on later runs.
• /ocr resolve suppresses the finding and resolves the discussion after the next review has been published successfully.

Findings are matched using both their diff position and a stable fingerprint. Moving the surrounding code should not bring back an issue the team has already considered, while a materially different problem can still be reported as a new finding.

The ownership rule is deliberately simple: an untouched OCR discussion can be managed by automation; once a person joins it, it belongs to the team's conversation.

Recording accepted decisions as review context

Not every warning points to a bug. Sometimes it reflects a deliberate compatibility constraint, generated code, an external API limitation, or a temporary architectural tradeoff.

Teams can record those decisions in:

.opencodereview/accepted-decisions.md

For example:

## generated-client-timeout

The generated client keeps the provider timeout so regeneration stays reproducible.

- Scope: src/client/generated/**
- Category: compatibility
- Owner: client-platform
- Review after: 2026-12-01

The toolkit gives OCR only the decisions that apply to the part of the repository being reviewed. This adds context that can help OCR distinguish a known tradeoff from an accidental change.

An accepted decision is not a blanket exemption and does not mechanically suppress a finding. OCR can still report a new or unrelated problem. For a specific published finding, reviewers continue to use /ocr suppress or /ocr resolve.

There is also an important trust boundary: decisions come only from the target branch. A merge request cannot edit accepted-decisions.md and thereby authorize its own changes.

Project instructions with repository scope

The same principle applies to root and nested AGENTS.md and CLAUDE.md files.

For each changed path, OCR receives only the applicable target-branch instructions, ordered from the repository root toward the nearest parent directory. This lets a monorepo express different guidance for different services or components without loading every instruction file into every review.

Instructions added or changed by the merge request are excluded from its trusted review context.

Repository evidence through a read-only MCP server

The toolkit collects a bounded set of facts from immutable base and head Git objects. OCR can query those facts through a built-in read-only MCP server with summary, list, and get operations.

That context includes:

• repository components and structure;
• detected frameworks and templates;
• applicable project decisions and instructions;
• the provenance of an individual fact;
• evidence changes between base and head;
• whether a particular evidence domain was covered completely or only partially.

Current static evidence covers Jinja2 and Ansible-style templates, Go web frameworks, Symfony/PHP, and React, Next.js, TypeScript, and Vite repositories.

Repository code is never imported or executed. Evidence plugins operate on bounded manifest, lockfile, and tree data, and the MCP server remains read-only. OCR still runs as a CLI process and returns structured JSON; MCP is used to give the review agent additional context while it works.

Publishing the result to GitLab

The toolkit creates inline discussions when GitLab accepts the diff position. If a line can no longer receive an inline comment, the finding is moved to a bounded fallback note rather than silently disappearing.

The review summary uses one outcome line to distinguish a clean review, findings, warnings, incomplete coverage, limits, and failures. Category and severity stay with each finding. They are plain text by default, with optional Shields badges for teams that want them; text mode avoids an external image request.

Review execution and publication are separate steps. In the default draft mode, the toolkit prepares the current set of notes first and replaces the previous bot-owned review only after the new one has been published successfully. If publication fails, the last successful review and all human-owned discussions remain intact.

After a complete successful review, the toolkit can also add a conservative GitLab approval bound to the exact source SHA that OCR reviewed. GitLab remains authoritative for approval rules, Code Owners, and protected branches.

For now, the toolkit is deliberately focused on GitLab CI. OCR is installed separately; the project owns repository context, CI orchestration, and the lifecycle of review results around it.

I would be especially interested in hearing from teams that already run OCR in merge request pipelines:

1. How do you handle reruns and stale review comments today?
2. Are commands inside a discussion a useful way to record a reviewer's decision, or would another interaction work better?
3. Which kinds of repository context make the biggest difference to review quality, and where would you draw the boundary between OCR and an external CI integration?

Thanks to the OCR maintainers for keeping the CLI and structured JSON contract usable for external integrations. That boundary is what lets us build this workflow without forking or changing the review engine.

[lizhengfeng101]

Thanks for the detailed write-up. Upfront caveat: everything below is my own current understanding, not a maintainer position — treat it as one reader's take rather than project guidance.

The boundary you draw (OCR stays the review engine, the integration owns the surrounding lifecycle) seems like the right one to me, so it's useful to see it exercised in a real GitLab setup.

Two things that may help, one concrete and one directional.

There is a GitLab CI example in the repo you may not have seen

examples/gitlab_ci/ is a GitLab-specific posting implementation, and it already covers several of the problems you describe:

• Sticky summary — find-by-marker then PUT /notes/:id, so the summary note updates in place across runs instead of stacking.
• Incremental overlap — skips comments whose line range overlaps a prior bot discussion on the same path, using an IoU threshold for multi-line spans (OCR_INCREMENTAL, OCR_INCREMENTAL_OVERLAP_THRESHOLD).
• Idempotent retry — each inline discussion carries an invisible <!-- ocr-<pipeline>-<job>-<hex> --> id tag. When POST /discussions fails with 5xx/408/network (where the request may still have landed), it queries existing discussions for that id before retrying; found means success, and if the read API is unavailable it skips the retry rather than risk a duplicate.
• 400 line-resolution fallback — on a position-related 400 it fetches GET /merge_requests/:iid/diffs, classifies the finding as valid/invalid/unknown, and drops provably-out-of-diff findings to the summary instead of blindly retrying.
• Severity/category routing — OCR_ROUTE_SEVERITY_BELOW / OCR_ROUTE_CATEGORIES move low-value findings to the summary before they ever enter the inline write path, so they can't be double-posted on retry.
• Deterministic ordering — comments sorted by path → start_line → end_line → original index, which is what makes rerun reconciliation reproducible in the first place.

Might be worth diffing against your 0.6.0 publish path. The idempotent-retry reconciliation and the 400 classification in particular are the two places I've seen external integrations most often lose or duplicate comments.

How I'd currently approach the rerun model

For repeat reviews on the same MR/PR, this is the shape that makes most sense to me right now:

1. All cross-run state lives in the integration layer. OCR consumes a diff and emits stateless structured findings; nothing about "was this reported before" belongs in the engine. Your design already works this way, which is the part I find most convincing.

2. On rerun, remove the previous unhandled bot comments before publishing the new set. Unhandled meaning no human engagement — no reply, no reaction, not resolved. Once a person touches a thread it belongs to the team and automation leaves it alone. Same ownership rule you landed on; the difference is applying it by removal rather than retention, because after several iterations an append-only history leaves reviewers to work out which findings still apply against code that has since changed.

I'd note the tradeoff honestly: removal loses the trail of what was reported and later became moot, and if publication fails mid-flight you can end up with fewer comments than before. Your draft-mode ordering (publish first, replace after success) handles that failure case better than removal does, so it may be that the two ideas want combining rather than choosing between.

3. Decide re-posting with two layers, position then semantics.

• Position — path plus line range, with relocation so code moving within a file isn't a new finding.
• Semantics — when positions don't line up, compare the substance of the finding against the previous set. Same underlying issue with different wording or a different anchor line still counts as already reported.

Only findings clearing both layers get posted. That second layer is where I'd personally go beyond a fingerprint. A fingerprint handles relocation well, but it's still a hash, so it also moves when nothing meaningful changed — the model phrasing the same finding slightly differently between runs, or a small edit shifting the context the fingerprint derives from. My impression is that phrasing drift, more than code movement, is what resurrects findings across reruns. If your fingerprint is narrow enough to be stable against it, that result would be genuinely worth writing up.

To be precise about what ships today: the incremental mode in the Action and in the GitLab example implements only the non-destructive subset of the above — position/IoU matching, history never deleted. Removing unhandled comments and the semantic layer are things an integration would have to build itself. So the above is what I'd build, not a description of existing built-in behavior.

On your three questions

Reruns and stale comments — covered above.

Commands inside a discussion — looks reasonable to me, and the /ocr suppress vs /ocr resolve split captures reviewer intent that implicit signals (a reply, a reaction) genuinely cannot. The cost is the syntax reviewers have to learn. Keeping both — implicit engagement marks a thread as handled, commands express the stronger intent — seems like the better combination.

Where to draw the boundary — your placement matches my instinct, with one open question I don't have an answer to. Semantic identity of a finding: should each integration compute it, or should OCR emit a stable semantic id in its JSON so every integration shares one notion of "same finding"? The latter would stop each GitLab/GitHub/internal integration from reinventing it, at the cost of pulling a rerun-lifecycle concept into a deliberately stateless engine. I can't see that resolving cleanly, and it's above my pay grade to call — but if you have a concrete proposal for the field's shape and stability guarantees, opening it as a separate issue seems like the right way to get maintainer input, since the JSON contract is where it would have to be settled.

Happy to go deeper on any of this — particularly the semantic-matching layer and how it interacts with your fingerprint, if you want to compare notes on where each one breaks.

[xeonvs]

Thank you for such a detailed response, and especially for clearly separating what ships today from what you would build next. That distinction made the comparison much more useful.

A small historical clarification: I started building the toolkit before examples/gitlab_ci/ appeared. When the upstream GitLab example first arrived, it was a much smaller CI example with an embedded posting script; the separate tested post_review.py and its current production hardening came later. I therefore see these as two independently developed integrations that converged on some similar decisions, rather than the toolkit being a wrapper around the current upstream example.

The original post described toolkit 0.6.0 with OCR 1.9.3. For this comparison I reviewed the current toolkit 0.6.2, OCR 1.9.5, and the current examples/gitlab_ci/ implementation.

The toolkit was created for more than turning OCR JSON into GitLab comments. Its purpose is to make OCR integration reproducible and easier to operate, while adding verifiable control layers around the review engine: compatibility and preflight checks, bounded repository evidence, protected-target policy and guidance, safe publication, and a repeated-review lifecycle. OCR remains the only review engine. The current toolkit already supplies relevant repository context through a bounded read-only MCP server; discussion and external-object enrichment are planned separately and are not claims about the current runtime.

Here is my current comparison. Bold text marks the side I think is stronger in that particular area; where both cells are bold, they provide different useful guarantees.

Area	Alibaba examples/gitlab_ci/	Toolkit 0.6.2
Integration form	Copyable YAML and Python script; convenient for quick adoption and local customization	Versioned integration package with a CLI, public contracts, preflight, and release qualification
OCR reproducibility	OCR_VERSION can be pinned, but defaults to latest	Exact OCR version, checksum verification, and versioned compatibility evidence
Repeat reviews	Sticky summary plus optional append-only overlap filtering; earlier inline history is retained	A successful complete review replaces only untouched bot-owned notes; a partial review does not remove the last complete state
Human ownership	The example does not currently give replies, resolved state, or reviewer commands a separate lifecycle	Replies, GitLab Resolve, /ocr suppress, and /ocr resolve create explicit ownership and suppression transitions bound to the actual bot author ID
Cross-run matching	Path and line-range IoU; no semantic matching today	Exact position plus an occurrence-aware fingerprint over normalized finding content and its code anchor, including compatibility with older fingerprints
Ambiguous inline creation	A unique per-write marker and positive readback can recover a discussion that was created despite a lost response; retry after a zero-match read still has an in-flight/visibility race	No blind retry and no duplicate fallback after an ambiguous create; today this fails closed rather than recovering a successful create
Publication transaction	The summary is updated around sequential inline publication	Draft-first publication keeps the previous successful review until the current one has published; definite failures use bounded rollback
Invalid positions	A diff-inventory valid / invalid / unknown classification gives a more precise fallback reason	Diff refs must match the reviewed head; typed 400/422 failures use bounded fallback, while an unknown write outcome never becomes a duplicate fallback
Rate limits and ordering	Pacing, jitter, Retry-After/reset handling, deterministic posting order, and configurable severity/category routing	Bounded GET retries and deterministic duplicate-fingerprint ordering, but no equivalent general pacing/routing layer
Context and trust boundary	Raw OCR_BACKGROUND and a repository-owned rule path can be passed directly to OCR	Bounded target-derived policy, instructions, accepted decisions, and evidence; the source branch cannot declare its own changes trusted guidance
Approval	No complete approval lifecycle	Exact-SHA approval machinery with separate completeness, finding, and confirmation gates; in 0.6.2, admitting MR metadata also blocks approval, and #100 tracks decoupling those decisions

The main architectural point is one on which we agree:

All cross-run state lives in the integration layer.

OCR consumes a diff and produces structured findings. The integration layer owns provider state, the publication transaction, discussion ownership, suppression, and the repeated-run lifecycle. I also agree with your observation:

Your draft-mode ordering (publish first, replace after success) handles that failure case better than removal does...

That is why I would not delete the previous review before publishing the new one. The toolkit first acquires a bounded complete snapshot, creates the current review, and only removes replaceable notes from the previous run after successful publication. If the OCR result is partial, it preserves the previous complete review.

Position, fingerprint, and semantic identity

Our fingerprint has a narrower role than a universal cross-run semantic identity. Untouched findings do not accumulate: a new complete review replaces them. Position and fingerprint primarily preserve a decision already made by a reviewer in a resolved, replied-to, or explicitly suppressed discussion.

The current model survives ordinary movement of a code anchor when the normalized finding and anchor remain compatible, but it has a real limit: if a finding both moves and is substantially rephrased, its position and fingerprint may both change. A fuzzy model-based comparison could match more cases, but a false match is dangerous here because it can silently hide a materially different defect.

That makes your question particularly useful:

Should OCR emit a stable semantic id in its JSON so every integration shares one notion of “same finding”?

I do not think our current fingerprint can simply be moved into OCR JSON as that common semantic ID. Such a contract would first need to define:

• what remains stable when wording or the anchor line changes;
• what happens when the model, rules, or OCR version changes;
• when a changed defect must receive a new identity;
• how identical findings at multiple locations are distinguished; and
• who versions the identity algorithm and what stability integrations may rely on.

If we can make those guarantees concrete enough, I agree that a separate upstream issue would be the right next step.

Commands and reactions

I agree with this distinction:

Implicit engagement marks a thread as handled, commands express the stronger intent.

An ordinary reply already means that a discussion has moved from automation into the team's conversation. /ocr suppress and /ocr resolve remain useful because they express an exact intent that cannot safely be inferred from free-form text.

Your mention of reactions exposed a separate gap: the toolkit does not currently consider reactions when deciding ownership. If we add that capability, the least surprising semantics seem to be:

• any non-bot reaction is human engagement, so automation no longer treats the discussion as untouched;
• the emoji name itself is not interpreted — 👍, 👎, 👀, or ✅ do not silently mean suppress, resolve, or accepted decision;
• the discussion is preserved and a matching duplicate is not posted beside it, as with an ordinary human reply;
• reactions cannot change policy, approval eligibility, posting authority, or lifecycle commands; and
• if reaction state cannot be acquired completely and reliably, automation must not claim that the thread is untouched and safe to delete.

GitLab exposes reactions through a separate paginated per-note API, rather than as a field in the discussion response we already consume. Before implementing this, we would need to define bounded acquisition, API cost, compatibility, and failure semantics. I would avoid configurable meanings for individual emoji: that would quickly become an implicit second command language. A compatibility switch should be added only if operational experience demonstrates a real need; the safe ownership rule itself can remain fixed.

Concrete follow-up

The most immediate improvement from this comparison is positive reconciliation after an ambiguous inline create. I have recorded that as toolkit issue #101.

The default toolkit path creates inline drafts with POST /draft_notes; direct mode uses POST /discussions. In either case, a complete author-bound readback with exactly one write-marker match can recover the created object without another create. However, zero matches do not prove that the first request is no longer in flight or that its write is already visible. Current GitLab API contracts expose no create idempotency key for these endpoints, so #101 deliberately does not permit retry-on-absence. Zero, multiple, incomplete, unavailable, malformed, or foreign-author results remain fail-closed.

This preserves the existing draft transaction, rollback, previous-review guarantees, and the rule that an ambiguous inline create never becomes a duplicate fallback. It is an independent publication-layer improvement; it does not change the OCR review engine or the planned context architecture.

Thank you again for the feedback. The concrete ambiguous-write pattern, the precise limits of position-only incremental matching, and the question of where semantic finding identity should live were all particularly valuable.