Skip to content

docs(examples): OCR_LLM_MODEL is required, not optional — fix the GitLab example#17

Open
chethanuk wants to merge 1 commit into
mainfrom
docs/gitlab-example-model-guard
Open

docs(examples): OCR_LLM_MODEL is required, not optional — fix the GitLab example#17
chethanuk wants to merge 1 commit into
mainfrom
docs/gitlab-example-model-guard

Conversation

@chethanuk

@chethanuk chethanuk commented Jul 21, 2026

Copy link
Copy Markdown
Owner

Description

The GitLab example documents OCR_LLM_MODEL as optional with a gpt-4o default. It is
required, and no such default exists anywhere in the codebase. A user who follows the docs
and leaves it unset gets a red pipeline and this:

Error: usage: ocr config set <key> <value>
e.g., ocr config set llm.model claude-opus-4-6

Three places make the claim:

File Line Text
examples/gitlab_ci/.gitlab-ci.yml 12 # OCR_LLM_MODEL - Model name (default: gpt-4o)
examples/gitlab_ci/README.md 41 | `OCR_LLM_MODEL` | No | No | Model name (defaults to `gpt-4o`) |
examples/gitflic_ci/README.md 42 | `OCR_LLM_MODEL` | No | Model name (e.g., `gpt-4o`) |

The code disagrees. internal/llm/resolver.go:411 treats an empty model as unresolved:

if cfg.Llm.URL == "" || cfg.Llm.AuthToken == "" || model == "" {
    return ResolvedEndpoint{}, false, nil
}

with the same guard on the env path at :169. And there is no gpt-4o fallback to fall
back to — the only occurrence in non-test Go code is a provider catalogue entry:

$ grep -rn 'gpt-4o' --include='*.go' . | grep -v _test.go
internal/llm/providers.go:292:			"openai/gpt-4o",

So the mechanism today is: the unquoted $OCR_LLM_MODEL word-splits away, leaving a 2-arg
ocr config set, which cmd/opencodereview/flags.go:262 rejects. GitLab jobs run under
set -e, so the job dies right there.

Why the obvious fix is the wrong one

The sibling GitFlic example already "handles" this at examples/gitflic_ci/gitflic-ci.yaml:51-53:

if [ -n "$OCR_LLM_MODEL" ]; then
  ocr config set llm.model "$OCR_LLM_MODEL"
fi

Copying that would make things worse, and I verified it rather than assuming. Skipping
the assignment doesn't summon a default — it just moves the failure later and hides it:

$ bash wrong.sh ; echo "EXIT=$?"
Set llm.url = https://ollama.com/v1/chat/completions
Set llm.auth_token = ****
REACHED_REVIEW_STEP_WITH_NO_MODEL_CONFIGURED
EXIT=0

$ cat ~/.opencodereview/config.json
{
    "llm": {
        "url": "https://ollama.com/v1/chat/completions",
        "auth_token": "***"
    }
}

$ ocr llm test
Error: resolve LLM endpoint: no valid LLM endpoint configured; one of
OCR_LLM_URL/OCR_LLM_TOKEN/OCR_LLM_MODEL, ~/.opencodereview/config.json, or
ANTHROPIC_BASE_URL/ANTHROPIC_AUTH_TOKEN/ANTHROPIC_MODEL must be set

A green config step, no model configured, and the review then dies with an error that
.gitlab-ci.yml:68's || true swallows — leaving the user a generic "OpenCodeReview
encountered an error"
MR comment. That is strictly worse than today's immediate failure.

Quoting alone is also not a no-op. An unset variable, quoted, becomes a 3-arg call with
an empty value:

$ ocr config set llm.url "" ; echo "EXIT=$?"
Set llm.url =
EXIT=0
$ cat ~/.opencodereview/config.json
{
    "llm": {}
}

Exit 0, a reassuring Set llm.url = , and nothing written. Quoting is only safe when paired
with an explicit required-variable check — which is why this PR does both together rather
than shipping the quoting as a tidy-up.

The fix

-      ocr config set llm.url $OCR_LLM_URL
-      ocr config set llm.auth_token $OCR_LLM_AUTH_TOKEN
-      ocr config set llm.model $OCR_LLM_MODEL
+      : "${OCR_LLM_URL:?set OCR_LLM_URL in Settings -> CI/CD -> Variables}"
+      : "${OCR_LLM_AUTH_TOKEN:?set OCR_LLM_AUTH_TOKEN in Settings -> CI/CD -> Variables}"
+      : "${OCR_LLM_MODEL:?set OCR_LLM_MODEL in Settings -> CI/CD -> Variables}"
+      ocr config set llm.url "$OCR_LLM_URL"
+      ocr config set llm.auth_token "$OCR_LLM_AUTH_TOKEN"
+      ocr config set llm.model "$OCR_LLM_MODEL"

${VAR:?msg} rather than ${VAR:-default} deliberately: there is no correct default to
supply. gpt-4o is wrong for every non-OpenAI endpoint this example targets, and llm.url
is user-specific by definition.

GitLab's own templates use ${VAR:-default} in Jobs/Build.gitlab-ci.yml:7 and
Security/DAST.gitlab-ci.yml:32 — but only where a genuine fallback exists
(${CI_APPLICATION_TAG:-$CI_COMMIT_SHA}). Where one doesn't, gitlabhq/gitlabhq
.gitlab/ci/qa-common/omnibus.gitlab-ci.yml:8 fails fast instead:
if [ -z "$RELEASE" ]; then echo "…requires…"; exit 1. That distinction is exactly the one
being applied here.

Third bug in the same five lines — a directory nothing reads

.gitlab-ci.yml:51 creates ~/.open-code-review. The real config directory is
~/.opencodereview, no hyphens (internal/session/persist.go:104). Confirmed by running
the binary: after a successful config step, ~/.opencodereview/config.json exists and the
hyphenated directory was never created. ocr config set creates the real directory itself,
so the line is dead either way — it only misleads anyone debugging the example. Removed.

Verification

The whole chain, run against a live LLM endpoint:

Case OCR_LLM_MODEL Result
Before this PR unset exit 1usage: ocr config set <key> <value>. Opaque, but immediate.
if [ -n ] guard (rejected) unset exit 0, no model configured → ocr llm test fails no valid LLM endpoint configured, swallowed by || true
This PR unset exit 1OCR_LLM_MODEL: set OCR_LLM_MODEL in Settings -> CI/CD -> Variables
This PR set exit 0, config written to ~/.opencodereview/config.json

And the fixed block end-to-end against a real endpoint, to confirm the guards are
transparent when the variables are set:

$ ocr llm test
Source: OCR config file
URL:    https://ollama.com/v1/chat/completions
Model:  gpt-oss:120b
I am an AI code-review assistant called open-code-review, ...
✓ Connection test successful

shellcheck on the extracted script block, before and after:

$ shellcheck -s bash before.sh
SC2086 x3 (llm.url, llm.auth_token, llm.model)   exit 1
$ shellcheck -s bash after.sh
                                                 exit 0

python3 -c "yaml.safe_load(...)" on the changed .gitlab-ci.yml — parses clean.

Scope

The README duplicates the entire broken snippet at examples/gitlab_ci/README.md:145-155,
so the same fix is applied there — otherwise this would fix the example and leave the
documentation of the example broken.

examples/gitflic_ci/README.md:42 makes the same false optionality claim about the same
variable, so I corrected that one line too. I did not touch
examples/gitflic_ci/gitflic-ci.yaml: its if [ -n ] guard papers over this same bug, but
changing it is a behaviour change to a different example on a platform whose CI feature set
I could not verify. Happy to follow up separately.

Deliberately not included

  • only: merge_requestsrules:. only/except is legacy and GitLab's own templates
    use rules:, but the two exercise different pipeline-selection paths, so it is
    behaviour-adjacent rather than cosmetic. It does not belong in a bugfix. Glad to send it
    as a follow-up.
  • workflow: auto_cancel: {on_new_commit: interruptible}. interruptible: true at
    :38 is inert without it, so this is a real gap — but it changes cancellation behaviour,
    and mixing that with a bugfix risks the bugfix. Also a follow-up if wanted.

Limitations

  • Not validated on a real gitlab.com pipeline. gitlab.com's free-tier shared runners now
    require credit-card identity verification, which I did not do. Everything above was proven
    locally against the real ocr binary and a live LLM endpoint, which exercises the exact
    failure path — but I have not watched this run as a GitLab job, and I am not claiming I did.
  • The GitFlic README correction is a documentation fix only; I did not exercise a GitFlic
    pipeline.
  • OCR_LLM_URL / OCR_LLM_AUTH_TOKEN were already effectively required; the guards make
    that explicit and are the reason quoting them is safe. That is a small behaviour change for
    those two — previously an unset URL also produced the same opaque usage error, so the
    failure mode improves rather than changes.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Refactoring (no functional changes)
  • Documentation update
  • CI / Build / Tooling

How Has This Been Tested?

  • Built the real binary and ran the extracted script block for every combination of
    set/unset variables — table above
  • Proved the rejected if [ -n ] variant reaches a green config step with no model, then
    fails at ocr llm test with no valid LLM endpoint configured
  • Proved quoting alone converts the hard failure into exit 0 with an empty config
  • Ran the fixed block end-to-end against a live LLM endpoint — ✓ Connection test successful
  • shellcheck -s bash — 3× SC2086 before, clean after
  • yaml.safe_load on the changed .gitlab-ci.yml
  • Confirmed ~/.opencodereview/config.json is written and ~/.open-code-review never is

Checklist

  • My code follows the project's coding style
  • I have performed a self-review of my code
  • I have added tests that prove my fix is effective — n/a, CI example + docs; no test
    harness executes examples/. Verified against the real binary instead, as above.
  • New and existing unit tests pass locally with my changes
  • I have updated the documentation accordingly
  • I have signed the CLA

AI assistance: Claude Code helped research and verify this change. I reviewed the full diff
and take responsibility for it.

Summary by CodeRabbit

  • Documentation

    • Updated GitFlic and GitLab CI documentation to require OCR_LLM_MODEL.
    • Clarified that no default model is provided and pipelines fail when the setting is missing.
    • Added guidance for validating required OCR configuration values before setup.
  • Bug Fixes

    • CI configuration now checks required OCR settings and safely applies validated values.

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@chethanuk, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 57 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 358d5970-75f7-4594-a035-bb0b6c2b915d

📥 Commits

Reviewing files that changed from the base of the PR and between 580abcd and 781df21.

📒 Files selected for processing (3)
  • examples/gitflic_ci/README.md
  • examples/gitlab_ci/.gitlab-ci.yml
  • examples/gitlab_ci/README.md
📝 Walkthrough

Walkthrough

Changes

OCR LLM configuration

Layer / File(s) Summary
Document required OCR model settings
examples/gitflic_ci/README.md, examples/gitlab_ci/.gitlab-ci.yml, examples/gitlab_ci/README.md
CI documentation now marks OCR_LLM_MODEL as required and states that OCR fails when it is unset.
Validate and write OCR configuration
examples/gitlab_ci/.gitlab-ci.yml, examples/gitlab_ci/README.md
GitLab CI setup validates required OCR variables before writing quoted values to OCR configuration.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: stay-foolish-forever

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: making OCR_LLM_MODEL required in the GitLab example and docs.
Description check ✅ Passed The description covers purpose, change type, testing, checklist, and related issues, matching the template well.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch docs/gitlab-example-model-guard

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request updates the GitFlic and GitLab CI/CD examples to mark OCR_LLM_MODEL as a required variable, as OpenCodeReview (OCR) lacks a built-in default model and fails when it is unset. It also introduces shell parameter validation checks to ensure required variables are set, quotes the variables to prevent word splitting, and removes unnecessary directory creation commands. There are no review comments, so I have no further feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@chethanuk

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@chethanuk
chethanuk force-pushed the docs/gitlab-example-model-guard branch 2 times, most recently from 6f99eff to 580abcd Compare July 21, 2026 16:14
The GitLab example documented OCR_LLM_MODEL as optional with a gpt-4o
default in three places. There is no such default: internal/llm/resolver.go:411
treats an empty model as unresolved, and the only gpt-4o in non-test Go code
is a provider catalogue entry at internal/llm/providers.go:292.

Unset, the variable word-splits away and leaves a 2-arg 'ocr config set',
so parseConfigArgs sees ["set", "llm.model"] and the len(args) < 3 guard at
cmd/opencodereview/flags.go:268 rejects it with the opaque usage error
returned on :269.

Add ${VAR:?} guards naming each missing variable, quote all three
assignments, correct the docs in both READMEs, and drop the dead
'mkdir -p ~/.open-code-review' - the real config dir is ~/.opencodereview.
@chethanuk
chethanuk force-pushed the docs/gitlab-example-model-guard branch from 580abcd to 781df21 Compare July 21, 2026 16:16
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