From b347889f2b17c9921359f4fcc8be953b66f5d7eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=A4=D0=B5=D0=B4=D0=BE=D1=80?= Date: Wed, 24 Jun 2026 09:41:32 +0300 Subject: [PATCH 1/3] docs(examples): add GitFlic CI auto-review example Add examples/gitflic_ci/, a CI-layer integration that reviews GitFlic merge requests with OpenCodeReview and posts the findings as MR discussions. Like the GitHub and GitLab examples, the posting glue lives outside the core binary. post_review.py (standard library only) reads `ocr review --format json` and posts inline discussions plus a fallback note and a summary. GitFlic's Discussions API requires an old-side line for inline comments, which the new-side-only review output lacks, so the script recomputes it from the same merge-base diff the review ran on. Ships with a stdlib unittest suite whose line-mapping cases are ported from the review's diff logic. --- examples/README.md | 1 + examples/gitflic_ci/README.md | 80 ++++ examples/gitflic_ci/gitflic-ci.yaml | 73 ++++ examples/gitflic_ci/post_review.py | 472 ++++++++++++++++++++++++ examples/gitflic_ci/post_review_test.py | 240 ++++++++++++ 5 files changed, 866 insertions(+) create mode 100644 examples/gitflic_ci/README.md create mode 100644 examples/gitflic_ci/gitflic-ci.yaml create mode 100644 examples/gitflic_ci/post_review.py create mode 100644 examples/gitflic_ci/post_review_test.py diff --git a/examples/README.md b/examples/README.md index 8a4caea1..bba03c82 100644 --- a/examples/README.md +++ b/examples/README.md @@ -6,5 +6,6 @@ This directory contains examples for integrating OpenCodeReview (OCR) into vario - **[github_actions/](./github_actions/)** - GitHub Actions integration example - **[gitlab_ci/](./gitlab_ci/)** - GitLab CI integration example +- **[gitflic_ci/](./gitflic_ci/)** - GitFlic CI integration example Each subdirectory contains its own README with detailed setup instructions. \ No newline at end of file diff --git a/examples/gitflic_ci/README.md b/examples/gitflic_ci/README.md new file mode 100644 index 00000000..9f36ce73 --- /dev/null +++ b/examples/gitflic_ci/README.md @@ -0,0 +1,80 @@ +# OpenCodeReview - GitFlic CI Demo + +This demo shows how to integrate OpenCodeReview into a [GitFlic](https://gitflic.ru) CI/CD pipeline to automatically review Merge Requests and post the findings as MR discussions — inline on the changed lines where possible. + +Like the GitHub Actions and GitLab CI examples, the posting glue lives in the CI layer rather than in the `ocr` binary. Here it is a small, dependency-free Python script — [`post_review.py`](post_review.py) — that reads `ocr review --format json` and posts to the GitFlic Discussions API. The only GitFlic-specific wrinkle it handles is the **old-side line**: GitFlic requires it even for a comment on the new side of the diff, and `ocr review` reports new-side positions only, so the script recomputes it from the same merge-base diff the review ran on. + +## How It Works + +``` +MR Created/Updated → Merge Request Pipeline → ocr review → post_review.py → Discussions on MR +``` + +1. A Merge Request Pipeline triggers the `code-review` job +2. It installs OCR via npm in a `node:20` image (which also ships `python3` and `git`) +3. Runs `ocr review --from origin/ --to $CI_COMMIT_SHA --format json --audience agent` +4. Runs `python3 post_review.py`, which reads the JSON and posts: + - **Inline discussions** on the changed lines (`POST .../discussions/create` with `newLine`/`oldLine`/`newPath`/`oldPath`) + - **A fallback note** collecting comments that could not be placed inline + - **A summary note** with the totals + +The MR context (owner, project, MR id, branch refs) is picked up automatically from the predefined GitFlic CI variables (`CI_PROJECT_NAMESPACE`, `CI_PROJECT_NAME`, `CI_MERGE_REQUEST_LOCAL_ID`, `CI_MERGE_REQUEST_TARGET_BRANCH_NAME`, `CI_COMMIT_SHA`), so `post_review.py` needs no arguments in CI. Outside CI every value can be passed via flags — run `python3 post_review.py -h`. + +## Setup + +### 1. Enable Merge Request Pipelines + +Go to **Project Settings → CI/CD Settings** and enable **Merge Request Pipeline**. New merge requests will then trigger the pipeline automatically. + +### 2. Copy the pipeline files + +Copy **both** `gitflic-ci.yaml` (GitFlic expects this exact file name at the repository root) and `post_review.py` into your repository. If you keep `post_review.py` somewhere other than the repo root, adjust the `python3 post_review.py` path in `gitflic-ci.yaml` accordingly. + +### 3. Configure CI/CD Variables + +Go to **Settings → CI/CD → Variables** and add: + +| Variable | Required | Description | +|----------|----------|-------------| +| `OCR_LLM_URL` | Yes | LLM API endpoint URL | +| `OCR_LLM_AUTH_TOKEN` | Yes | LLM API authentication token | +| `GITFLIC_TOKEN` | Yes | GitFlic access token used to post discussions | +| `OCR_LLM_MODEL` | No | Model name (e.g., `gpt-4o`) | +| `GITFLIC_API_URL` | No | REST API base URL for self-hosted GitFlic (default: `https://api.gitflic.ru`) | + +> **Note:** GitFlic CI/CD does not accept variables with values shorter than 8 characters, so `use_anthropic` cannot be set as a CI variable. The pipeline sets it to `false`; to use Anthropic Claude models, edit `gitflic-ci.yaml` directly. + +### 4. Create a GitFlic Access Token + +Create a token in **User Settings → Access Tokens** (or a dedicated service account — its name becomes the bot name shown in discussions) and store it in the `GITFLIC_TOKEN` variable. The token owner must have access to the project sufficient for commenting on merge requests. + +## Notes & Limitations + +- **Inline positioning** — GitFlic requires all four of `newLine`/`oldLine`/`newPath`/`oldPath` for a code comment; if any is missing it silently creates a general comment. `post_review.py` computes the old-side position from the same merge-base diff the review ran on (`git diff merge-base(from, to)..to`), and anchors added lines to the closest preceding old line. +- **Rate limit** — the GitFlic cloud API allows 500 requests/hour per token. One review posts `comments + 2` requests at most, which fits comfortably. +- **Self-hosted GitFlic** — set `GITFLIC_API_URL` to your instance's REST API base URL. +- **Re-reviews** — every push to the MR triggers a new pipeline and a new review. To skip already-reviewed MRs, check existing discussions for the `OpenCodeReview` marker before running the review step. + +## Tests + +`post_review.py` ships with [`post_review_test.py`](post_review_test.py) — standard-library `unittest`, no network or git required: + +```bash +cd examples/gitflic_ci +python3 post_review_test.py +``` + +The line-mapping cases are ported from the upstream Go tests so the script keeps proven parity with the binary. + +## Debugging + +Test the posting step locally without touching the MR: + +```bash +ocr review --from origin/main --to HEAD --format json > /tmp/r.json +python3 post_review.py /tmp/r.json \ + --owner --project --mr \ + --from origin/main --to HEAD --dry-run +``` + +`--dry-run` prints every discussion (with the computed positions) instead of posting, and does not require `GITFLIC_TOKEN`. diff --git a/examples/gitflic_ci/gitflic-ci.yaml b/examples/gitflic_ci/gitflic-ci.yaml new file mode 100644 index 00000000..c6c30ee5 --- /dev/null +++ b/examples/gitflic_ci/gitflic-ci.yaml @@ -0,0 +1,73 @@ +# OpenCodeReview - GitFlic CI Merge Request Auto-Review Demo +# +# Reviews Merge Requests with OpenCodeReview and posts the findings onto the +# MR as discussions (inline where possible). The posting glue lives in the CI +# layer, in post_review.py next to this file -- consistent with the GitHub and +# GitLab examples, which keep platform-specific publishing out of the `ocr` +# binary. +# +# Setup: +# - Commit BOTH this file and post_review.py into your repository (adjust the +# `python3 post_review.py` path below if you place the script elsewhere). +# - Enable "Merge Request Pipeline" in Project Settings -> CI/CD Settings. +# - Use a runner able to run the node:20 image (it ships node, python3 and git), +# or any shell runner with node 20+, python3 and git available. +# +# Required CI/CD Variables (Settings -> CI/CD -> Variables): +# OCR_LLM_URL - LLM API endpoint (e.g., https://api.openai.com/v1/chat/completions) +# OCR_LLM_AUTH_TOKEN - Authentication token for the LLM API +# GITFLIC_TOKEN - GitFlic access token used to post MR discussions +# +# Optional CI/CD Variables: +# OCR_LLM_MODEL - Model name (e.g., gpt-4o) +# GITFLIC_API_URL - GitFlic REST API base URL; only needed for self-hosted +# instances (defaults to https://api.gitflic.ru) +# +# post_review.py picks up the MR context automatically from the predefined +# GitFlic CI variables: CI_PROJECT_NAMESPACE, CI_PROJECT_NAME, +# CI_MERGE_REQUEST_LOCAL_ID, CI_MERGE_REQUEST_TARGET_BRANCH_NAME, CI_COMMIT_SHA. + +stages: + - review + +code-review: + stage: review + image: node:20 + script: + # Run only in merge request pipelines + - | + if [ -z "$CI_MERGE_REQUEST_LOCAL_ID" ]; then + echo "Not a merge request pipeline, skipping review." + exit 0 + fi + + # Install OpenCodeReview + - npm install -g @alibaba-group/open-code-review + + # Configure OCR + - | + 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 config set llm.use_anthropic false + ocr config set llm.extra_body '{"thinking": {"type": "disabled"}}' + + # Make sure the target branch and full history are available for merge-base diff + - git fetch --unshallow 2>/dev/null || true + - git fetch origin "$CI_MERGE_REQUEST_TARGET_BRANCH_NAME" + + # Run OCR review (CI_COMMIT_SHA as head supports forked MRs as well) + - | + ocr review \ + --from "origin/${CI_MERGE_REQUEST_TARGET_BRANCH_NAME}" \ + --to "${CI_COMMIT_SHA}" \ + --format json \ + --audience agent \ + > /tmp/ocr-result.json || true + echo "OCR review completed." + + # Post review comments onto the MR (inline discussions + summary note). + # post_review.py recomputes the old-side line for each comment from the + # merge-base diff, which the GitFlic Discussions API requires for inline + # (code) comments. + - python3 post_review.py /tmp/ocr-result.json diff --git a/examples/gitflic_ci/post_review.py b/examples/gitflic_ci/post_review.py new file mode 100644 index 00000000..79350ff2 --- /dev/null +++ b/examples/gitflic_ci/post_review.py @@ -0,0 +1,472 @@ +#!/usr/bin/env python3 +"""Post an OpenCodeReview result onto a GitFlic merge request. + +This is the CI-layer "glue" for GitFlic, mirroring examples/gitlab_ci: it keeps +platform-specific publishing out of the `ocr` binary and lives entirely in the +pipeline. It reads the JSON emitted by `ocr review --format json` and posts it +onto the merge request as discussions: + + - one inline discussion per comment that maps onto the diff, + - a single fallback note collecting the comments that do not, + - a final summary note. + +GitFlic's Discussions API needs an *old-side* line even for a comment on the new +side of the diff: an inline (code) discussion requires all four of +newLine/oldLine/newPath/oldPath, otherwise GitFlic silently records a plain +comment. `ocr review` only reports new-side positions, so this script computes +the old-side line itself by parsing the same merge-base diff the review ran on +(`git diff merge-base(from, to)..to`). + +Standard library only (json, urllib, subprocess) so it runs on the stock +node:20 / python image used by the pipeline. +""" + +import argparse +import json +import os +import re +import subprocess +import sys +import urllib.error +import urllib.request +from urllib.parse import quote + +# GitFlic SaaS REST API endpoint; override with --api-url / $GITFLIC_API_URL for +# self-hosted instances (e.g. http://gitflic.example/rest-api). +DEFAULT_API_URL = "https://api.gitflic.ru" + +# Context lines around each hunk; must match what `ocr review` diffs with so the +# new-side line numbers in the comments align with the hunks parsed here. +DIFF_CONTEXT_LINES = 3 + + +def log(msg): + print(msg, file=sys.stderr) + + +# --------------------------------------------------------------------------- # +# Diff parsing +# --------------------------------------------------------------------------- # + +HUNK_CONTEXT, HUNK_ADDED, HUNK_DELETED = range(3) + +_HUNK_HEADER_RE = re.compile(r"^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@") +_DIFF_HEADER_RE = re.compile(r"^diff --git a/(.+?) b/(.+)$") + + +class Hunk: + """One @@ ... @@ block of a unified diff.""" + + __slots__ = ("old_start", "old_count", "new_start", "new_count", "lines") + + def __init__(self, old_start, old_count, new_start, new_count): + self.old_start = old_start + self.old_count = old_count + self.new_start = new_start + self.new_count = new_count + self.lines = [] # list of (type, content) + + +class FileDiff: + """A single file's section of a unified diff.""" + + __slots__ = ("old_path", "new_path", "is_new", "is_deleted", "is_binary", "text") + + def __init__(self, old_path="", new_path=""): + self.old_path = old_path + self.new_path = new_path + self.is_new = False + self.is_deleted = False + self.is_binary = False + self.text = "" # raw diff body, fed to parse_hunks() on demand + + +def parse_hunks(raw): + """Parse one file's unified diff text into a list of Hunks. + + Lines before the first @@ header (diff --git, ---, +++) are ignored. + """ + hunks = [] + current = None + for line in raw.split("\n"): + m = _HUNK_HEADER_RE.match(line) + if m: + if current is not None: + hunks.append(current) + old_start = int(m.group(1)) + old_count = int(m.group(2)) if m.group(2) else 1 + new_start = int(m.group(3)) + new_count = int(m.group(4)) if m.group(4) else 1 + current = Hunk(old_start, old_count, new_start, new_count) + continue + if current is None: + continue + if line.startswith("\\ No newline at end of file"): + continue + if line.startswith("diff --git "): + break + if line.startswith("+"): + current.lines.append((HUNK_ADDED, line[1:])) + elif line.startswith("-"): + current.lines.append((HUNK_DELETED, line[1:])) + else: + content = line[1:] if line[:1] == " " else line + current.lines.append((HUNK_CONTEXT, content)) + if current is not None: + hunks.append(current) + return hunks + + +def parse_diff(diff_text): + """Split combined unified diff text into per-file FileDiff sections.""" + files = [] + current = None + buf = [] + + def flush(): + if current is not None: + current.text = "\n".join(buf) + files.append(current) + + for line in diff_text.split("\n"): + m = _DIFF_HEADER_RE.match(line) + if m: + flush() + buf = [] + current = FileDiff(old_path=m.group(1), new_path=m.group(2)) + if current is None: + continue + if line.startswith("Binary files ") or line.startswith("GIT binary patch"): + current.is_binary = True + elif line.startswith("new file mode"): + current.is_new = True + elif line.startswith("deleted file mode"): + current.is_deleted = True + elif line.startswith("--- "): + path = line[4:] + if path == "/dev/null": + current.is_new = True + current.old_path = "/dev/null" + elif path.startswith("a/"): + current.old_path = path[2:] + elif line.startswith("+++ "): + path = line[4:] + if path == "/dev/null": + current.is_deleted = True + current.new_path = "/dev/null" + elif path.startswith("b/"): + current.new_path = path[2:] + buf.append(line) + + flush() + return files + + +# --------------------------------------------------------------------------- # +# Line mapping (new file side -> old file side) +# --------------------------------------------------------------------------- # + + +def clamp_line(n): + return 1 if n < 1 else n + + +def old_line_for(hunks, new_line): + """Map a new-side line number to the corresponding old-side line. + + Lines added by the diff have no old counterpart, so they are anchored to the + closest preceding old line -- GitFlic only needs a plausible old-side + position to render the code comment next to the insertion point. The result + is always >= 1. + """ + delta = 0 # cumulative (new - old) line-count shift from preceding hunks + for h in hunks: + if new_line < h.new_start: + break + if new_line < h.new_start + h.new_count: + return _old_line_in_hunk(h, new_line) + delta += h.new_count - h.old_count + return clamp_line(new_line - delta) + + +def _old_line_in_hunk(h, new_line): + """Walk a hunk's lines tracking both counters until reaching new_line.""" + old_ln, new_ln = h.old_start, h.new_start + last_old = h.old_start - 1 # last old line seen before the current position + for line_type, _content in h.lines: + if line_type == HUNK_CONTEXT: + if new_ln == new_line: + return clamp_line(old_ln) + last_old = old_ln + old_ln += 1 + new_ln += 1 + elif line_type == HUNK_DELETED: + last_old = old_ln + old_ln += 1 + elif line_type == HUNK_ADDED: + if new_ln == new_line: + return clamp_line(last_old) + new_ln += 1 + return clamp_line(last_old) + + +# --------------------------------------------------------------------------- # +# Comment formatting +# --------------------------------------------------------------------------- # + + +def format_comment(c): + """Render an inline discussion body.""" + body = c.get("content", "") + suggestion = c.get("suggestion_code", "") + existing = c.get("existing_code", "") + if suggestion and existing: + body += "\n\n**Suggestion:**\n```\n" + suggestion + "\n```" + return body + + +def format_comment_fallback(c): + """Render a comment for the fallback (non-inline) note.""" + md = "### 📄 `%s`" % c.get("path", "") + start_line = c.get("start_line", 0) + end_line = c.get("end_line", 0) + if start_line and end_line: + md += " (L%d-L%d)" % (start_line, end_line) + md += "\n\n" + c.get("content", "") + suggestion = c.get("suggestion_code", "") + existing = c.get("existing_code", "") + if suggestion and existing: + md += "\n\n**Before:**\n```\n" + existing + "\n```\n\n**After:**\n```\n" + suggestion + "\n```" + return md + + +# --------------------------------------------------------------------------- # +# Publishing (transport-agnostic; `post` does the actual API call) +# --------------------------------------------------------------------------- # + + +def publish(result, diffs_by_path, post): + """Post the review result via the `post(discussion)` callable. + + `post` receives a discussion dict and must raise on failure. A general + comment carries only "message"; an inline comment also carries + newLine/oldLine/newPath/oldPath. Returns {"inline": int, "fallback": int}. + """ + comments = result.get("comments") or [] + if not comments: + message = result.get("message") or "No comments generated. Looks good to me." + post({"message": "✅ **OpenCodeReview**: " + message}) + return {"inline": 0, "fallback": 0} + + inline = 0 + failed = [] + hunks_cache = {} + + for c in comments: + path = c.get("path", "") + end_line = c.get("end_line", 0) or 0 + fd = diffs_by_path.get(path) + if fd is None: + log("no diff for %s; folding comment into the summary note" % path) + failed.append(c) + continue + if fd.is_binary or fd.is_deleted or end_line <= 0: + failed.append(c) + continue + + old_path = fd.old_path + old_line = 1 + if fd.is_new or old_path == "" or old_path == "/dev/null": + # GitFlic has no old side for a new file; anchor to the new path. + old_path = fd.new_path + else: + hunks = hunks_cache.get(path) + if hunks is None: + hunks = parse_hunks(fd.text) + hunks_cache[path] = hunks + old_line = old_line_for(hunks, end_line) + + discussion = { + "message": format_comment(c), + "newLine": end_line, + "oldLine": old_line, + "newPath": path, + "oldPath": old_path, + } + try: + post(discussion) + except Exception as e: # noqa: BLE001 - any transport error falls back + log("inline comment failed for %s:%d: %s" % (path, end_line, e)) + failed.append(c) + continue + inline += 1 + + if failed: + note = "🔍 **OpenCodeReview** found issues that could not be posted inline:\n\n---\n\n" + for c in failed: + note += format_comment_fallback(c) + "\n\n---\n\n" + post({"message": note}) + + summary = "🔍 **OpenCodeReview** found **%d** issue(s) in this MR." % len(comments) + summary += "\n- ✅ %d posted as inline comment(s)" % inline + summary += "\n- 📝 %d posted as summary (could not be placed inline)" % len(failed) + warnings = result.get("warnings") or [] + if warnings: + summary += "\n\n⚠️ %d warning(s) occurred during review." % len(warnings) + post({"message": summary}) + + return {"inline": inline, "fallback": len(failed)} + + +# --------------------------------------------------------------------------- # +# GitFlic REST transport +# --------------------------------------------------------------------------- # + + +def make_poster(api_url, token, owner, project, mr): + """Return a post(discussion) that POSTs to the GitFlic Discussions API.""" + endpoint = "%s/project/%s/%s/merge-request/%s/discussions/create" % ( + api_url.rstrip("/"), + quote(owner, safe=""), + quote(project, safe=""), + quote(mr, safe=""), + ) + + def post(discussion): + body = json.dumps(discussion).encode("utf-8") + req = urllib.request.Request(endpoint, data=body, method="POST") + req.add_header("Authorization", "token " + token) + req.add_header("Content-Type", "application/json") + try: + with urllib.request.urlopen(req) as resp: + resp.read() + except urllib.error.HTTPError as e: + snippet = e.read(512).decode("utf-8", "replace").strip() + raise RuntimeError("gitflic API %s %s: %s" % (e.code, e.reason, snippet)) + + return post + + +def make_dry_run_poster(): + """Return a post(discussion) that prints instead of calling the API.""" + + def post(discussion): + if discussion.get("newPath") and "newLine" in discussion and "oldLine" in discussion: + position = "%s:%d (old %s:%d)" % ( + discussion["newPath"], + discussion["newLine"], + discussion.get("oldPath", ""), + discussion["oldLine"], + ) + else: + position = "general" + print("--- dry-run discussion [%s] ---\n%s\n" % (position, discussion["message"])) + + return post + + +# --------------------------------------------------------------------------- # +# git / IO +# --------------------------------------------------------------------------- # + + +def _git(repo, *args): + return subprocess.run( + ["git", *args], cwd=repo, check=True, capture_output=True, text=True + ).stdout + + +def load_diffs_by_path(repo, from_ref, to_ref): + """Build {new_path: FileDiff} for the merge-base diff `ocr review` ran on.""" + base = _git(repo, "merge-base", from_ref, to_ref).strip() + out = _git( + repo, "diff", "--no-ext-diff", "--no-textconv", + "--src-prefix=a/", "--dst-prefix=b/", "--no-color", + "-U%d" % DIFF_CONTEXT_LINES, base, to_ref, "--", + ) + return {fd.new_path: fd for fd in parse_diff(out)} + + +def load_review_result(path): + """Read the JSON produced by `ocr review --format json` (path '-' = stdin).""" + data = sys.stdin.read() if path == "-" else open(path, encoding="utf-8").read() + return json.loads(data) + + +# --------------------------------------------------------------------------- # +# CLI +# --------------------------------------------------------------------------- # + + +def parse_args(argv): + target = os.environ.get("CI_MERGE_REQUEST_TARGET_BRANCH_NAME", "") + default_from = "origin/" + target if target else "" + + p = argparse.ArgumentParser( + description="Post `ocr review --format json` output onto a GitFlic merge request." + ) + p.add_argument("file", nargs="?", default="-", + help="review result JSON ('-' = stdin, default)") + p.add_argument("--owner", default=os.environ.get("CI_PROJECT_NAMESPACE", ""), + help="project owner alias (default: $CI_PROJECT_NAMESPACE)") + p.add_argument("--project", default=os.environ.get("CI_PROJECT_NAME", ""), + help="project alias (default: $CI_PROJECT_NAME)") + p.add_argument("--mr", default=os.environ.get("CI_MERGE_REQUEST_LOCAL_ID", ""), + help="merge request local id (default: $CI_MERGE_REQUEST_LOCAL_ID)") + p.add_argument("--api-url", default=os.environ.get("GITFLIC_API_URL", "") or DEFAULT_API_URL, + help="GitFlic REST API base URL (default: $GITFLIC_API_URL or %s)" % DEFAULT_API_URL) + p.add_argument("--from", dest="from_ref", default=default_from, + help="base ref of the reviewed range (default: origin/$CI_MERGE_REQUEST_TARGET_BRANCH_NAME)") + p.add_argument("--to", dest="to_ref", default=os.environ.get("CI_COMMIT_SHA", ""), + help="head ref of the reviewed range (default: $CI_COMMIT_SHA)") + p.add_argument("--repo", default=".", help="git repository root (default: .)") + p.add_argument("--dry-run", action="store_true", + help="print discussions instead of posting them") + return p.parse_args(argv) + + +def main(argv=None): + args = parse_args(sys.argv[1:] if argv is None else argv) + + missing = [name for name, value in ( + ("--owner", args.owner), ("--project", args.project), ("--mr", args.mr), + ("--from", args.from_ref), ("--to", args.to_ref), + ) if not value] + if missing: + log("error: %s required (set via flag or CI environment)" % ", ".join(missing)) + return 2 + + token = os.environ.get("GITFLIC_TOKEN", "") + if not token and not args.dry_run: + log("error: GITFLIC_TOKEN environment variable is required") + return 2 + + try: + result = load_review_result(args.file) + except (OSError, ValueError) as e: + log("error: cannot read review result %s: %s" % (args.file, e)) + return 1 + + try: + diffs_by_path = load_diffs_by_path(args.repo, args.from_ref, args.to_ref) + except (subprocess.CalledProcessError, OSError) as e: + # Without the diff, inline positions cannot be computed; comments still + # go out via the fallback note. + log("warning: cannot read diff %s..%s, posting all comments as fallback: %s" + % (args.from_ref, args.to_ref, e)) + diffs_by_path = {} + + if args.dry_run: + post = make_dry_run_poster() + else: + post = make_poster(args.api_url, token, args.owner, args.project, args.mr) + + stats = publish(result, diffs_by_path, post) + total = len(result.get("comments") or []) + print("Posted %d inline comment(s), %d via fallback note (%d total)." + % (stats["inline"], stats["fallback"], total)) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/examples/gitflic_ci/post_review_test.py b/examples/gitflic_ci/post_review_test.py new file mode 100644 index 00000000..be438289 --- /dev/null +++ b/examples/gitflic_ci/post_review_test.py @@ -0,0 +1,240 @@ +#!/usr/bin/env python3 +"""Tests for post_review.py. + +Standard-library unittest only (no pytest, no network, no git): run with + + python3 post_review_test.py # from examples/gitflic_ci/ + python3 -m unittest discover examples/gitflic_ci + +The line-mapping cases are ported 1:1 from the upstream Go test +internal/publish/gitflic/linemap_test.go and publisher_test.go, so the script +keeps proven parity with the binary it replaces. +""" + +import unittest + +import post_review as pr + + +# old file lines 1..10; line 3 modified, a line inserted after old line 5, +# old line 8 deleted. (from linemap_test.go) +SAMPLE_DIFF = """diff --git a/main.go b/main.go +--- a/main.go ++++ b/main.go +@@ -1,10 +1,10 @@ + line1 + line2 +-line3 old ++line3 new + line4 + line5 ++inserted after5 + line6 + line7 +-line8 + line9 + line10 +""" + +NEW_FILE_DIFF = """diff --git a/added.go b/added.go +new file mode 100644 +--- /dev/null ++++ b/added.go +@@ -0,0 +1,3 @@ ++package main ++ ++func main() {} +""" + +DELETED_FILE_DIFF = """diff --git a/gone.go b/gone.go +deleted file mode 100644 +--- a/gone.go ++++ /dev/null +@@ -1,2 +0,0 @@ +-package main +- +""" + +BINARY_DIFF = """diff --git a/logo.png b/logo.png +index 1111111..2222222 100644 +Binary files a/logo.png and b/logo.png differ +""" + + +class OldLineForTest(unittest.TestCase): + def setUp(self): + self.hunks = pr.parse_hunks(SAMPLE_DIFF) + + def test_single_hunk_positions(self): + self.assertEqual(len(self.hunks), 1) + cases = [ + ("context before changes", 1, 1), + ("modified line maps to deleted position anchor", 3, 3), + ("context after modification", 4, 4), + ("added line anchors to preceding old line", 6, 5), + ("context shifted by insertion", 7, 6), + ("context after deletion", 9, 9), + ("last context line", 10, 10), + ] + for name, new_line, want in cases: + with self.subTest(name): + self.assertEqual(pr.old_line_for(self.hunks, new_line), want) + + def test_outside_hunks(self): + # one line added and one deleted -> cumulative delta 0 + self.assertEqual(pr.old_line_for(self.hunks, 42), 42) + + def test_multiple_hunks(self): + multi = ( + "@@ -1,2 +1,4 @@\n" + " line1\n" + "+added2\n" + "+added3\n" + " line2\n" + "@@ -10,3 +12,3 @@\n" + " line10\n" + "-line11 old\n" + "+line11 new\n" + " line12\n" + ) + hunks = pr.parse_hunks(multi) + self.assertEqual(len(hunks), 2) + # between hunks: new 8 = old 6 (two lines added by hunk 1) + self.assertEqual(pr.old_line_for(hunks, 8), 6) + # inside second hunk: modified new 13 anchors to old 11 + self.assertEqual(pr.old_line_for(hunks, 13), 11) + + def test_pure_addition_at_top(self): + hunks = pr.parse_hunks("@@ -0,0 +1,2 @@\n+first\n+second\n") + self.assertEqual(pr.old_line_for(hunks, 1), 1) + + +class ParseDiffTest(unittest.TestCase): + def test_modified_file(self): + fd = pr.parse_diff(SAMPLE_DIFF)[0] + self.assertEqual((fd.old_path, fd.new_path), ("main.go", "main.go")) + self.assertFalse(fd.is_new or fd.is_deleted or fd.is_binary) + + def test_new_file(self): + fd = pr.parse_diff(NEW_FILE_DIFF)[0] + self.assertTrue(fd.is_new) + self.assertEqual(fd.new_path, "added.go") + + def test_deleted_file(self): + fd = pr.parse_diff(DELETED_FILE_DIFF)[0] + self.assertTrue(fd.is_deleted) + self.assertEqual(fd.new_path, "/dev/null") + + def test_binary_file(self): + fd = pr.parse_diff(BINARY_DIFF)[0] + self.assertTrue(fd.is_binary) + + def test_multiple_files(self): + files = pr.parse_diff(SAMPLE_DIFF + NEW_FILE_DIFF) + self.assertEqual([f.new_path for f in files], ["main.go", "added.go"]) + + +class Recorder: + """A post() that records discussions; optionally fails the first inline.""" + + def __init__(self, fail_first_inline=False): + self.calls = [] + self.fail_first_inline = fail_first_inline + self._inline_seen = 0 + + def __call__(self, discussion): + if self.fail_first_inline and "newPath" in discussion: + self._inline_seen += 1 + if self._inline_seen == 1: + raise RuntimeError("simulated 403") + self.calls.append(discussion) + + +def diffs_from(diff_text): + return {fd.new_path: fd for fd in pr.parse_diff(diff_text)} + + +class PublishTest(unittest.TestCase): + def test_inline_and_summary(self): + result = { + "comments": [{ + "path": "main.go", "content": "possible nil dereference", + "start_line": 6, "end_line": 6, + "existing_code": "x := y.Field", + "suggestion_code": "if y != nil { x = y.Field }", + }], + } + rec = Recorder() + stats = pr.publish(result, diffs_from(SAMPLE_DIFF), rec) + + self.assertEqual(stats, {"inline": 1, "fallback": 0}) + self.assertEqual(len(rec.calls), 2) # inline + summary + + inline = rec.calls[0] + self.assertEqual(inline["newLine"], 6) + self.assertEqual(inline["oldLine"], 5) + self.assertEqual((inline["newPath"], inline["oldPath"]), ("main.go", "main.go")) + self.assertIn("possible nil dereference", inline["message"]) + self.assertIn("**Suggestion:**", inline["message"]) + + summary = rec.calls[1] + self.assertNotIn("newPath", summary) + self.assertIn("**1** issue(s)", summary["message"]) + + def test_fallback_for_unmapped_comment(self): + result = { + "comments": [{ + "path": "missing.go", "content": "issue in file absent from diff", + "start_line": 1, "end_line": 1, + }], + "warnings": [{"file": "a.go", "message": "skipped", "type": "subtask_error"}], + } + rec = Recorder() + stats = pr.publish(result, {}, rec) + + self.assertEqual(stats, {"inline": 0, "fallback": 1}) + self.assertEqual(len(rec.calls), 2) # fallback + summary + self.assertIn("could not be posted inline", rec.calls[0]["message"]) + self.assertIn("`missing.go`", rec.calls[0]["message"]) + self.assertIn("1 warning(s)", rec.calls[1]["message"]) + + def test_inline_error_falls_back(self): + result = { + "comments": [{ + "path": "main.go", "content": "finding", + "start_line": 1, "end_line": 1, + }], + } + rec = Recorder(fail_first_inline=True) + stats = pr.publish(result, diffs_from(SAMPLE_DIFF), rec) + + self.assertEqual(stats, {"inline": 0, "fallback": 1}) + self.assertEqual(len(rec.calls), 2) # fallback + summary after inline failure + + def test_no_comments(self): + rec = Recorder() + stats = pr.publish({"message": "No comments generated. Looks good to me."}, {}, rec) + + self.assertEqual(stats, {"inline": 0, "fallback": 0}) + self.assertEqual(len(rec.calls), 1) + self.assertIn("Looks good to me", rec.calls[0]["message"]) + + def test_new_file_anchors_to_new_path(self): + result = { + "comments": [{ + "path": "added.go", "content": "empty main", + "start_line": 3, "end_line": 3, + }], + } + rec = Recorder() + stats = pr.publish(result, diffs_from(NEW_FILE_DIFF), rec) + + self.assertEqual(stats["inline"], 1) + inline = rec.calls[0] + self.assertEqual(inline["oldPath"], "added.go") + self.assertEqual(inline["oldLine"], 1) + self.assertEqual(inline["newLine"], 3) + + +if __name__ == "__main__": + unittest.main() From 10119d70044b526259aff3aa2df46ec97a2047de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=A4=D0=B5=D0=B4=D0=BE=D1=80?= Date: Wed, 24 Jun 2026 10:11:16 +0300 Subject: [PATCH 2/3] docs(readme): list the GitFlic CI example in the localized READMEs --- README.ja-JP.md | 1 + README.ko-KR.md | 1 + README.md | 1 + README.ru-RU.md | 1 + README.zh-CN.md | 1 + 5 files changed, 5 insertions(+) diff --git a/README.ja-JP.md b/README.ja-JP.md index d2cc1a54..fd4dafd4 100644 --- a/README.ja-JP.md +++ b/README.ja-JP.md @@ -331,6 +331,7 @@ ocr review \ - [`github_actions/`](./examples/github_actions/) — GitHub Actions統合の例 - [`gitlab_ci/`](./examples/gitlab_ci/) — GitLab CI統合の例 +- [`gitflic_ci/`](./examples/gitflic_ci/) — GitFlic CI統合の例 ## コマンド diff --git a/README.ko-KR.md b/README.ko-KR.md index 28e8323d..99535e85 100644 --- a/README.ko-KR.md +++ b/README.ko-KR.md @@ -331,6 +331,7 @@ ocr review \ - [`github_actions/`](./examples/github_actions/): GitHub Actions 통합 예시 - [`gitlab_ci/`](./examples/gitlab_ci/): GitLab CI 통합 예시 +- [`gitflic_ci/`](./examples/gitflic_ci/): GitFlic CI 통합 예시 ## Commands diff --git a/README.md b/README.md index 574def5b..9ac7c2b4 100644 --- a/README.md +++ b/README.md @@ -333,6 +333,7 @@ See the [`examples/`](./examples/) directory for integration examples: - [`github_actions/`](./examples/github_actions/) — GitHub Actions integration example - [`gitlab_ci/`](./examples/gitlab_ci/) — GitLab CI integration example +- [`gitflic_ci/`](./examples/gitflic_ci/) — GitFlic CI integration example ## Commands diff --git a/README.ru-RU.md b/README.ru-RU.md index 2c27f0ea..54a8d1ff 100644 --- a/README.ru-RU.md +++ b/README.ru-RU.md @@ -333,6 +333,7 @@ ocr review \ - [`github_actions/`](./examples/github_actions/) — пример интеграции с GitHub Actions - [`gitlab_ci/`](./examples/gitlab_ci/) — пример интеграции с GitLab CI +- [`gitflic_ci/`](./examples/gitflic_ci/) — пример интеграции с GitFlic CI ## Команды diff --git a/README.zh-CN.md b/README.zh-CN.md index 6a430ef8..1b30f472 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -331,6 +331,7 @@ ocr review \ - [`github_actions/`](./examples/github_actions/) — GitHub Actions 集成示例 - [`gitlab_ci/`](./examples/gitlab_ci/) — GitLab CI 集成示例 +- [`gitflic_ci/`](./examples/gitflic_ci/) — GitFlic CI 集成示例 ## 命令 From 1dc279b1b9b4e9787e0d0482265a2d0571ca14b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=A4=D0=B5=D0=B4=D0=BE=D1=80?= Date: Wed, 24 Jun 2026 15:13:58 +0300 Subject: [PATCH 3/3] fix(examples): address GitFlic CI review feedback from PR #201 Apply the five review comments left on the PR: - gitflic-ci.yaml: guard `ocr config set llm.model` behind a non-empty check so the documented-optional OCR_LLM_MODEL no longer breaks the config step when it is unset - gitflic-ci.yaml: skip posting when `ocr review` produced no output (the step ends with `|| true`) instead of feeding empty/partial JSON to post_review.py - post_review.py: redact the token from HTTP error snippets so it cannot leak into CI logs if GitFlic echoes the request back in an error body - post_review.py: read the review-result file via a `with` block so the handle is closed explicitly - examples/README.md: add the missing trailing newline --- examples/README.md | 2 +- examples/gitflic_ci/gitflic-ci.yaml | 15 +++++++++++++-- examples/gitflic_ci/post_review.py | 10 +++++++++- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/examples/README.md b/examples/README.md index bba03c82..2ffc3ffe 100644 --- a/examples/README.md +++ b/examples/README.md @@ -8,4 +8,4 @@ This directory contains examples for integrating OpenCodeReview (OCR) into vario - **[gitlab_ci/](./gitlab_ci/)** - GitLab CI integration example - **[gitflic_ci/](./gitflic_ci/)** - GitFlic CI integration example -Each subdirectory contains its own README with detailed setup instructions. \ No newline at end of file +Each subdirectory contains its own README with detailed setup instructions. diff --git a/examples/gitflic_ci/gitflic-ci.yaml b/examples/gitflic_ci/gitflic-ci.yaml index c6c30ee5..2a2b9bd1 100644 --- a/examples/gitflic_ci/gitflic-ci.yaml +++ b/examples/gitflic_ci/gitflic-ci.yaml @@ -48,7 +48,9 @@ code-review: - | 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 + if [ -n "$OCR_LLM_MODEL" ]; then + ocr config set llm.model "$OCR_LLM_MODEL" + fi ocr config set llm.use_anthropic false ocr config set llm.extra_body '{"thinking": {"type": "disabled"}}' @@ -70,4 +72,13 @@ code-review: # post_review.py recomputes the old-side line for each comment from the # merge-base diff, which the GitFlic Discussions API requires for inline # (code) comments. - - python3 post_review.py /tmp/ocr-result.json + # + # The review step above ends with `|| true`, so a failed `ocr review` (bad + # token, network error) leaves an empty or partial file. Skip posting in + # that case instead of feeding invalid JSON to post_review.py. + - | + if [ ! -s /tmp/ocr-result.json ]; then + echo "OCR review produced no output, skipping post." + exit 0 + fi + python3 post_review.py /tmp/ocr-result.json diff --git a/examples/gitflic_ci/post_review.py b/examples/gitflic_ci/post_review.py index 79350ff2..e8b05556 100644 --- a/examples/gitflic_ci/post_review.py +++ b/examples/gitflic_ci/post_review.py @@ -342,6 +342,10 @@ def post(discussion): resp.read() except urllib.error.HTTPError as e: snippet = e.read(512).decode("utf-8", "replace").strip() + # Some APIs echo request details back in error bodies; never let the + # token reach the CI log if GitFlic does that. + if token: + snippet = snippet.replace(token, "***") raise RuntimeError("gitflic API %s %s: %s" % (e.code, e.reason, snippet)) return post @@ -389,7 +393,11 @@ def load_diffs_by_path(repo, from_ref, to_ref): def load_review_result(path): """Read the JSON produced by `ocr review --format json` (path '-' = stdin).""" - data = sys.stdin.read() if path == "-" else open(path, encoding="utf-8").read() + if path == "-": + data = sys.stdin.read() + else: + with open(path, encoding="utf-8") as f: + data = f.read() return json.loads(data)