diff --git a/.agents/skills/reviewing-before-pr/SKILL.md b/.agents/skills/reviewing-before-pr/SKILL.md index 106477b713fe..f3472fc56b61 100644 --- a/.agents/skills/reviewing-before-pr/SKILL.md +++ b/.agents/skills/reviewing-before-pr/SKILL.md @@ -1,10 +1,10 @@ --- name: reviewing-before-pr description: > - Run the one local Greptile review of a branch before its PR opens, with `hogli review`. - Use when instructed to open a PR or a draft PR (before `gh pr create`), or before marking - a draft ready for review when the branch never had a local review. Trigger terms: hogli review, - greptile, local review, pre-PR review, review before opening. + Run one local Greptile review of a branch before its PR opens, with `hogli review`. + Opt-in: it needs a seat on PostHog's Greptile workspace, which most people do not have yet, + so use it only when someone asks for a local Greptile review by name. Trigger terms: + hogli review, greptile, local review, pre-PR review. --- # Reviewing before opening a PR @@ -14,8 +14,8 @@ A finding handled here is an ordinary pre-push edit; the same finding after the ## When to run -- Run it **once**, right before `gh pr create`, whenever you were instructed to open a PR or a draft PR. -- Also run it once before marking a draft ready for review, if the branch never had a local review. +- Only when someone asks for it. A Greptile seat is per-person and hand-granted today, so opening a PR does not call for this review by default — the PR bot still reviews every PR. +- When asked, run it **once**, on the committed branch, right before `gh pr create`. - Do not loop it after every push, and do not re-run it after addressing findings — each run is a paid review, and the PR bot passes over the final state anyway. `hogli review` enforces this by re-printing HEAD's existing completed review instead of starting a new one. - Skip it for changes trivial enough for the `skip-agent-review` label. diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index da9f7bd5b5ad..2a6d87042690 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -72,7 +72,6 @@ - Public OSS repo: no internal customers, incidents, or operational metrics. - Stack instead of stuffing: if the diff holds two or more separable steps (migration then behavior, rename then rewrite), open a stack rather than one big PR. See AGENTS.md, "Stacked PRs" and /stacking-prs. - Simplify before opening: if your agent has a behavior-preserving cleanup pass (Claude Code: `/simplify`), run it on a non-trivial diff before final tests and preflight, since it edits the tree. Skip it for small mechanical changes. -- Review before opening: run `hogli review` once on the committed branch before `gh pr create` — the reviewing-before-pr skill carries the flow and the `no-greptile` label gate. - Draft by default: open new PRs as drafts (`gh pr create --draft`) — drafts run only a narrow CI subset and save runner credits. Fix CI and run affected tests locally before marking ready for review. - Labels: apply `skip-agent-review` for trivial/chore PRs that don't need Copilot or Greptile review. - When a human directed the work, the PR must be attributable to that person, even if agent-assisted. diff --git a/AGENTS.md b/AGENTS.md index c3cf6870a30b..715bd16d1fd0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -70,11 +70,6 @@ NEVER share sensitive information in a PR description. Users may share sensitive **Screenshots:** Upload frontend/visual changes with `hogli pr:upload-image ` and embed the printed markdown. The first run only warns and uploads nothing; re-run with `--yes` to confirm. Only PostHog employees can upload, but the public can permanently view these assets, so only upload the image if you're certain it doesn't contain customer data (including customer names), secrets, or sensitive internal info. -### Local review before opening - -When instructed to open a PR or a draft PR, run `hogli review` once on the committed branch before `gh pr create` — it runs the same Greptile reviewer that comments on every PR, so findings become pre-push edits instead of bot comments and CI re-runs. -Invoke `/reviewing-before-pr` for the full flow: auth and the no-access fallback, the run-once contract, the `no-greptile` label gate (`hogli review --check`), and recording findings in the PR description. - ### Rules - Scope is optional but encouraged when the change is specific to a feature area @@ -269,7 +264,6 @@ ALWAYS invoke the matching skill **before** writing or reviewing code in these a - `/writing-user-facing-copy` — writing or editing any text a user reads (UI labels, tooltips, empty/error states, notifications, docs, support replies), or any code change that adds or changes a visible string - `/writing-code-comments` — writing or editing a code comment in any language, or reviewing a diff that adds comments - `/writing-pr-descriptions` — writing or editing any PR body, before `gh pr create` or `gh pr edit --body` -- `/reviewing-before-pr` — when instructed to open a PR or a draft PR: one local Greptile review (`hogli review`) of the committed branch, before `gh pr create` **Invoke when in the area:** diff --git a/tools/hogli-commands/hogli_commands/ci_preflight.py b/tools/hogli-commands/hogli_commands/ci_preflight.py index f03ca2be69c2..a443b976d5dc 100644 --- a/tools/hogli-commands/hogli_commands/ci_preflight.py +++ b/tools/hogli-commands/hogli_commands/ci_preflight.py @@ -147,16 +147,6 @@ class DiffCheck: verify=None, advice="a type error costs a full CI re-run — consider `uv run mypy --cache-fine-grained .` (what CI runs)", ), - DiffCheck( - key="review", - label="review findings arriving after the PR opens (Greptile)", - triggers=["*.py", "*.ts", "*.tsx", "*.rs", "*.go"], - # A nudge, not a run: a Greptile review is a paid API call, too much to tax - # every push with. The reviewing-before-pr skill runs it once at PR-open time. - verify=None, - advice="before opening the PR (or marking it ready), consider one `hogli review` — " - "a finding caught locally avoids a bot comment round-trip and a CI re-run", - ), DiffCheck( key="markdown-format", label="markdown formatting (oxfmt)", diff --git a/tools/hogli-commands/hogli_commands/tests/test_ci_preflight.py b/tools/hogli-commands/hogli_commands/tests/test_ci_preflight.py index 292c6f96b218..460e18e1f3fc 100644 --- a/tools/hogli-commands/hogli_commands/tests/test_ci_preflight.py +++ b/tools/hogli-commands/hogli_commands/tests/test_ci_preflight.py @@ -90,7 +90,7 @@ def test_fix_without_stack_still_advises_openapi( @patch("hogli_commands.ci_preflight._fetch_master") @patch("hogli_commands.ci_preflight.subprocess.run") @patch("hogli_commands.ci_preflight.changed_files", return_value=["posthog/api/does_not_exist.py"]) - def test_nudges_name_the_command_without_running_it( + def test_nudge_names_the_command_without_running_it( self, mock_changed: MagicMock, mock_run: MagicMock, @@ -102,12 +102,9 @@ def test_nudges_name_the_command_without_running_it( assert result.exit_code == 0 assert "uv run mypy --cache-fine-grained ." in result.output - assert "hogli review" in result.output - # Giving either check a `verify` would tax every push with a repo-wide - # mypy run or a paid Greptile review. + # Giving the check a `verify` would tax every push with a repo-wide mypy run. ran = [arg for call in mock_run.call_args_list for arg in call.args[0]] assert "mypy" not in ran - assert "greptile" not in ran and "review" not in ran class TestStalenessRisks: