alibaba / alibaba/open-code-review

docs(review-rules): document merge_system_rule in the review rules guide

Open
#1,133 0 comments 0 reactions 0 assignees View on GitHub
documentation good first issue i18n
Dominant language
Go
Stars
24.4k
Forks
1.8k
Avg merge
2d 6h
Merged PRs (30d)
105

Description

## Description

`merge_system_rule` is an existing boolean field on a `rule.json` entry. When set, the matched user rule is **combined with** the system rule for that file's language instead of **replacing** it:

```json
{
"rules": [
{
"path": "**/*",
"rule": "Security review: flag hardcoded secrets, unvalidated redirects, and missing authz checks.",
"merge_system_rule": true
}
]
}
```

```
$ ocr rules check src/main/java/com/example/UserService.java
Source: Project (.opencodereview/rule.json)
Pattern: **/*
Rule:
────────────────────────────────────────
## System-Specific Rules (Mandatory)

…contents of java.md…

---

## User-Specific Rules (Mandatory)

Security review: flag hardcoded secrets, unvalidated redirects, and missing authz checks.
────────────────────────────────────────
```

The field has been implemented for a while (`ProjectRuleEntry.MergeSystemRule` → `mergeWithSystemRule`, `internal/config/rules/system_rules.go`), but **the review rules guide never mentions it** — not in any of the five locales. The only place it appears in the whole repo is `skills/open-code-review/SKILL.md`, which most users never read.

The cost of this gap is real: #1132 was filed asking for exactly this capability as a new feature, because the docs give every indication that a catch-all user rule unavoidably discards the per-language system rules.

## Scope

- File(s):
- `pages/src/content/docs/en/review-rules.md`
- `pages/src/content/docs/zh/review-rules.md`
- `pages/src/content/docs/ja/review-rules.md`
- `pages/src/content/docs/ru/review-rules.md`
- `pages/src/content/docs/ko/review-rules.md`
- Reference implementation: `internal/config/rules/system_rules.go` — `ProjectRuleEntry` (field declaration) and `mergeWithSystemRule` (merge format and empty-string edge cases)
- Existing wording to borrow from: `skills/open-code-review/SKILL.md`
- Docs-only change; no Go code should need to be touched.

## Acceptance Criteria

- [ ] The "Rule file format (layers 1–3)" section lists `merge_system_rule` alongside `path` and `rule`, stating that it is optional and defaults to `false` (replace).
- [ ] A worked example shows the JSON plus the resulting `ocr rules check` output, so the `## System-Specific Rules (Mandatory)` / `## User-Specific Rules (Mandatory)` structure is visible.
- [ ] The docs state that the system half is resolved **per file**, so one catch-all entry yields `java.md` for `.java`, `python.md` for `.py`, and `default.md` for unrecognized extensions.
- [ ] The docs state that it works in all three user layers: `--rule`, `/.opencodereview/rule.json`, and `~/.opencodereview/rule.json`.
- [ ] The docs state the limitation: it merges the **system** layer only. Multiple *user* entries matching the same file still resolve first-match-wins (see discussion #633), and a matching layer still shadows lower user layers.
- [ ] A recipe is added under "Recipes" for the common case — global security rules on top of the built-in per-language rules — since that is what people search for.
- [ ] All five locales updated with equivalent content.
- [ ] `make test` passes.
- [ ] `make check` passes.

## Context

Discovered while triaging #1132 ("Support adding global review rules"), whose entire request is already satisfied by this flag.

Behaviour verified on `v1.11.0` with `ocr rules check`: `a.java`, `a.py`, `a.go`, and an unrecognized extension each produced a different system half, confirming per-file language resolution rather than a fixed `default.md`; and dropping `merge_system_rule` reduced the output to the user rule alone.

Good first issue: no Go changes required, the behaviour is easy to observe locally with `ocr rules check`, and the surrounding doc already has the right structure — a field list to extend and a "Recipes" section to append to. Note that `pages/src/content/docs/**` is translated prose and is exempt from the English-only scan (see `AGENTS.md`), so the non-English locales should be written in their own language.

Contributor guide

Open the contributing guide

Research direction

Start with pages/src/content/docs/en/review-rules.md, the four locale files, and the wording in skills/open-code-review/SKILL.md; consult internal/config/rules/system_rules.go for merge behavior and edge cases. Update the field list, example, per-file resolution notes, layer limitations, and Recipes section in all locales, then run make test and make check.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
cli, documentation
Issue type
Documentation
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.