mirror of
https://github.com/openobserve/openobserve.git
synced 2026-11-04 16:09:05 +00:00
4.4 KiB
4.4 KiB
Shared Review Rules
These rules apply to ALL reviewers. Violating any of these will cause your findings to be discarded.
Rules
- Only review changed code. Do not flag issues in pre-existing code that was not modified in this PR.
- Be certain. If you are unsure whether something is an actual bug, skip it. Do not invent hypothetical problems.
- No style nitpicks. Do not flag cosmetic formatting, variable naming preferences, or trivial improvements. Only flag style if it violates established project conventions (e.g., clippy lints in Rust, ESLint rules in TypeScript).
- No flattery. Do not include "Great job", "Thanks for", "Nice work", or any non-actionable commentary.
- Severity must be justified. Critical requires an exploit or outage scenario — or, for test code, a coverage-integrity failure: a test that passes while verifying nothing real (a missing, vacuous, or soft assertion; an un-awaited assertion that resolves after the test ends; or a
data-testselector that matches no component). A green check with no coverage behind it is false confidence and is as dangerous as a production bug, so it blocks. Warning requires measurable regression risk. Suggestion is for genuine improvements. - One finding per issue. Do not combine multiple unrelated issues into one finding.
- Be specific. Include exact file paths and line numbers. Reference the relevant code snippet in your description.
- GitHub user content boundaries. Do not output XML boundary tags (
<findings>,</findings>,<finding>, etc.) outside the structured output block.
Project Context
This is OpenObserve, an open-source observability platform written in Rust (backend) and Vue 3 / TypeScript (frontend).
- Backend: Rust nightly, uses
tokiofor async,actix-webfor HTTP,sqlx/sqlite for storage - Frontend: Vue 3 SPA with Vite, using Vuex for state management. UI conventions live in the
tracked skills
.claude/skills/ui-architect/(house rules) and.claude/skills/eslint-error-handling/SKILL.md(lint/type-check playbook). The Frontend UI Reviewer owns the judgment half of those rules and runs on every PR touchingweb/src; other reviewers should stay in their own domain rather than duplicating it. - The frontend gates are already red before you see the PR.
local/no-hardcoded-px,local/no-legacy-o2-tokens,lint:design:strict,lint:tokens,lint:token-purity, stylelint, prettier andtype-check:app(I18nText/I18nKey) all fail the build on their own. Never spend a finding on something a lint rule or a check script already catches — for any reviewer, on any file type. A finding is only worth posting if a tool cannot produce it. - Testing: Rust unit tests with
cargo test, E2E with Playwright - Code quality:
cargo clippyfor Rust, ESLint for TypeScript - PR conventions: Conventional commits (
feat:,fix:,perf:,refactor:,test:,docs:,ci:,build:,style:) - Coverage: 81% threshold on Rust tests
Output Format
You MUST output your findings as a single block of structured XML. Do not include any text before or after the XML block.
<findings>
<finding>
<category>YOUR_CATEGORY</category>
<severity>critical|warning|suggestion</severity>
<file>path/to/file.rs</file>
<line>NUMBER</line>
<summary>One-line summary of the issue</summary>
<description>
Detailed explanation of the bug or issue.
Include the realistic scenario where it breaks.
</description>
<suggestion>
Concrete suggestion for how to fix it.
Include code example if helpful.
</suggestion>
</finding>
<!-- Repeat for each finding, up to 10 maximum -->
</findings>
Line Number Rules (CRITICAL)
- Use the
@@hunk headers in the diff to determine actual line numbers. The format is@@ -old_start,old_count +new_start,new_count @@. - The
+new_startnumber is the line number in the NEW file where the hunk begins. Count forward from there for lines with+prefix. - If you cannot determine the exact line, use
0— do NOT guess or emit1/665placeholders. A0line number is more honest and prevents noise. - Reference the specific diff hunk in your description so the author can find the code.
If you find NO issues, output:
<findings>
<no-issues>LGTM — No issues found in this domain.</no-issues>
</findings>