docs(examples): add GitFlic CI auto-review example#201
Conversation
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.
|
✅ OpenCodeReview: No comments generated. Looks good to me. |
lizhengfeng101
left a comment
lizhengfeng101
left a comment
There was a problem hiding this comment.
感谢 @revenant20 的高质量贡献！这个 PR 完全遵循了 #138 评审中的架构方向，将平台特定的发布逻辑放在 CI 层而非核心二进制中。代码结构清晰、测试覆盖充分、文档详尽，--dry-run 调试支持也很实用。
以下是一些小的改进建议，整体上这是一个很棒的贡献。
| - | | ||
| 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 |
There was a problem hiding this comment.
Bug: OCR_LLM_MODEL 在 README 中标注为 Optional，但这里无条件执行。当该变量未设置时，$OCR_LLM_MODEL 展开为空字符串，ocr config set llm.model "" 会写入空模型，可能导致后续 endpoint 解析失败。
（这个问题在 #138 的评审中也被提出过。）
建议加一个条件判断：
- |
ocr config set llm.url $OCR_LLM_URL
ocr config set llm.auth_token $OCR_LLM_AUTH_TOKEN
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"}}'There was a problem hiding this comment.
Done in 1dc279b — wrapped the model write in if [ -n "$OCR_LLM_MODEL" ] so an unset (optional) variable no longer writes an empty model.
|
|
||
| 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() |
There was a problem hiding this comment.
Nit: 文件句柄未显式关闭。虽然 CPython 引用计数会立即回收，但在其他 Python 实现中可能泄漏。
建议改为：
if path == "-":
data = sys.stdin.read()
else:
with open(path, encoding="utf-8") as f:
data = f.read()There was a problem hiding this comment.
Done in 1dc279b — switched to a with open(...) block so the handle is closed explicitly.
| --to "${CI_COMMIT_SHA}" \ | ||
| --format json \ | ||
| --audience agent \ | ||
| > /tmp/ocr-result.json || true |
There was a problem hiding this comment.
Note: || true 意味着即使 ocr review 完全失败（如 token 无效），pipeline 仍会继续执行 post_review.py，此时 JSON 文件可能为空或包含非法内容。
建议在之后加一个空文件检查：
- |
if [ ! -s /tmp/ocr-result.json ]; then
echo "OCR review produced no output, skipping post."
exit 0
fi或者如果这是有意为之（让 post_review.py 自行处理错误），可以在注释中说明意图。
There was a problem hiding this comment.
Done in 1dc279b — added if [ ! -s /tmp/ocr-result.json ] to skip posting on empty output, plus a comment explaining that the || true step can leave an empty/partial file.
| with urllib.request.urlopen(req) as resp: | ||
| resp.read() | ||
| except urllib.error.HTTPError as e: | ||
| snippet = e.read(512).decode("utf-8", "replace").strip() |
There was a problem hiding this comment.
Minor: 某些 API 在错误响应体中可能回显请求信息。如果 GitFlic 在 4xx/5xx 响应中包含了请求头片段，这里的 snippet 可能会将 token 泄漏到 CI 日志中。风险不高，但可以考虑对 snippet 做一下长度截断或脱敏。
There was a problem hiding this comment.
Done in 1dc279b — the token is now redacted from the snippet (snippet.replace(token, "***")) before it goes into the error message, so it can't reach the CI log.
| - **[github_actions/](./github_actions/)** - GitHub Actions integration example | ||
| - **[gitlab_ci/](./gitlab_ci/)** - GitLab CI integration example | ||
| - **[gitflic_ci/](./gitflic_ci/)** - GitFlic CI integration example | ||
|
|
There was a problem hiding this comment.
Nit: 这个文件缺少末尾换行符（No newline at end of file）。既然本次已修改了此文件，可以顺便修复。
There was a problem hiding this comment.
Done in 1dc279b — added the trailing newline.
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
|
Thanks for the thorough and kind review, @lizhengfeng101! All five comments are addressed in 1dc279b:
Ready for another look whenever you have time. |
2 participants
This adds a GitFlic CI integration example under
examples/gitflic_ci/, reworking #138 in line with the feedback there: the platform-specific publishing glue lives in the CI layer rather than in theocrbinary, consistent with the GitHub and GitLab examples. Implements #102; supersedes #138.The posting glue is a small, dependency-free
post_review.py(standard library only): it readsocr review --format jsonand posts the findings onto the merge request as inline discussions, a fallback note, and a summary. GitFlic's Discussions API requires an old-side line even for a comment on the new side of the diff — which the new-side-only review output doesn't carry — so the script recomputes it from the same merge-base diff the review ran on (git diff merge-base(from, to)..to), anchoring added lines to the closest preceding old line.It ships with
post_review_test.py, a standard-libraryunittestsuite (no network, git, or Go toolchain) whose line-mapping cases are ported from the original Go tests, so the example stays verifiable. I've also run it end-to-end against a live GitFlic CE instance to confirm the inline positions render correctly via the API.cc @lizhengfeng101 — thanks again for the thoughtful review on #138; happy to keep iterating here or in #102.